Add FCI LSA SAF FRP csv reader support - #3434
Conversation
… enhancement files.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3434 +/- ##
==========================================
- Coverage 96.33% 96.32% -0.02%
==========================================
Files 467 467
Lines 59142 59274 +132
==========================================
+ Hits 56977 57096 +119
- Misses 2165 2178 +13
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
ameraner
left a comment
ameraner
left a comment
There was a problem hiding this comment.
Nice job, thank you for this! Just some minor comments/questions/edits, and a requried change for the pixel coordinates.
| data = data.assign_coords({ | ||
| "longitude": ("y", lons), | ||
| "latitude": ("y", lats), | ||
| }) |
There was a problem hiding this comment.
maybe I'm missing something, but what is the point of adding the lon-lat as coordinates on y here, if we then immediately after remap the data on the 2-d array?
There was a problem hiding this comment.
You are right. Dimensions are not needed here.
Removed assign_cords()
There was a problem hiding this comment.
could it be that the change was not pushed? The assign_coords is still there
ameraner
added
enhancement
|
The CI run still fails in the two TestParallaxCorrectionModifier.test_modifier_interface_cloud_moves_to_observer cases for foroyar. The failure is caused by an unevaluated Dask boolean array: This behavior surprises me, as the CI tests passed 2 days ago. The changes made, suggested by Andreas Review, should not impact the testing success in my opinion. My branch is not behind origin/main (HEAD...origin/main = 8 0). Could you please confirm whether this is a known Dask/Pyresample compatibility issue and whether these two tests should be adjusted or marked as expected failures? |
ameraner
left a comment
ameraner
left a comment
There was a problem hiding this comment.
Thank you for the updates! I noticed a couple of things, see below.
The failing tests are not due to your changes, they're probably due to some library update that broke the parallax tests.. they will need to be fixed separately (feel free to give it a go in another PR if you have time).
| nc_fci_frp: | ||
| file_reader: !!python/name:satpy.readers.fci_lsasaf_frp_l2_nc.FRPFileHandler | ||
| file_patterns: | ||
| - 'W_PT-{facility_or_tool},SATELLITE,LSA-509_{platform_name}_MTFRPPIXEL_MTG-{coverage}_{disposition_mode}_LPMG_{start_time:%Y%m%d%H%M%S}.nc' |
There was a problem hiding this comment.
until we really support nc, this should be removed
There was a problem hiding this comment.
I thought one step in advance, as I just need to finish the utests for the nc-reader. But I agree with you. Will be removed here, and added later.
| data = data.assign_coords({ | ||
| "longitude": ("y", lons), | ||
| "latitude": ("y", lats), | ||
| }) |
There was a problem hiding this comment.
could it be that the change was not pushed? The assign_coords is still there
| grid = np.full(FRP_GRID_SHAPE, np.nan, dtype=np.float32) | ||
| grid[rows_int, cols_int] = values | ||
| xarr = xr.DataArray( | ||
| grid, | ||
| dims=("y", "x"), | ||
| attrs=attrs, | ||
| ) |
There was a problem hiding this comment.
The grid array here should be daskified to adhere to the normal format of reader outputs, and to optimise memory usage. You can take inspiration from an old version of the LI L2 reader to do that: https://github.com/pytroll/satpy/blame/618fc0e498c7f1e1bfce011f1ed1b041895ed8ce/satpy/readers/li_l2_nc.py#L141-L148
(in that reader, later on we needed an np.add.at functionality - so if you look at the current reader it is much more complex. However you don't need that here, since we have only one value per pixel, hence my link to an older blame version).
There was a problem hiding this comment.
Thanks this was helpful. The new version is daskified now.
|
I confirm that tests are failing in satpy main after |
|
The test failures are unrelated to this PR. Satpy main is also failing. See #3441 . |
3 participants
Summary
This PR adds tests for the FCI LSA SAF FRP L2 CSV reader.
Including yaml-, utest-, composite-, and enhancement-files
Work finished, based on PWC Spring 2026 Issue - Closes #3350
Testing