-
Notifications
You must be signed in to change notification settings - Fork 283
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add reader for LSA-SAF data #2529
base: main
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2529 +/- ##
==========================================
- Coverage 94.89% 94.69% -0.20%
==========================================
Files 349 350 +1
Lines 50859 50986 +127
==========================================
+ Hits 48263 48283 +20
- Misses 2596 2703 +107
Flags with carried forward coverage won't be shown. Click here to find out more.
|
pre-commit.ci autofix |
for more information, see https://pre-commit.ci
Kinda curious that there pre-commit.ci shows failures, I ran the pre-commit checks locally before pushing! |
Pull Request Test Coverage Report for Build 5574764301
💛 - Coveralls |
@@ -0,0 +1,44 @@ | |||
reader: | |||
name: lsasaf-geo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the name of the software "LSASAF GEO"? If not, then I'd say an underscore in the reader name would be more consistent. Or even "lsasaf" if there is no LEO version.
@staticmethod | ||
def _ensure_crs_extents_in_meters(crs, area_extent): | ||
"""Fix units in Earth shape, satellite altitude and 'units' attribute.""" | ||
if 'kilo' in crs.axis_info[0].unit_name: | ||
proj_dict = crs.to_dict() | ||
proj_dict["units"] = "m" | ||
if "a" in proj_dict: | ||
proj_dict["a"] *= 1000. | ||
if "b" in proj_dict: | ||
proj_dict["b"] *= 1000. | ||
if "R" in proj_dict: | ||
proj_dict["R"] *= 1000. | ||
proj_dict["h"] *= 1000. | ||
area_extent = tuple([val * 1000. for val in area_extent]) | ||
crs = CRS.from_dict(proj_dict) | ||
return crs, area_extent |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused method? If not, I'm tempted to say we should just deal with the projection in kilometers if possible.
if dsid_name in self.cache: | ||
logger.debug('Get the data set from cache: %s.', dsid_name) | ||
return self.cache[dsid_name] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Is the cache ever actually written to?
- If it was, would it really provide a performance improvement?
- In what cases is the same file handler asked for the same DataArray multiple times?
land_surface_temperature: | ||
compositor: !!python/name:satpy.composites.SingleBandCompositor | ||
prerequisites: | ||
- name: 'lst' | ||
standard_name: land_surface_temperature |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm tempted to say this shouldn't be in visir
yet. I'm also tempted to say this compositor should not exist...could you add the standard_name in the reader YAML and/or reader python code? Then this composite doesn't conflict with other readers who already have a product called "land_surface_temperature", but your enhancement based on standard_name
would still be found.
This PR adds a reader for data produced by EUMETSAT's LSA-SAF. Right now, it only supports land surface temperature files but I plan to add additional ones soon.
Tests need to be added as well, but I wanted to make a PR so gain some feedback on the way I've coded this before I do the testing for it.