Skip to content
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 OSI SAF L3 products #2632

Merged
merged 19 commits into from
Nov 28, 2023
Merged

Add reader for OSI SAF L3 products #2632

merged 19 commits into from
Nov 28, 2023

Conversation

simonrp84
Copy link
Member

@simonrp84 simonrp84 commented Nov 8, 2023

This PR adds a reader for the OSI SAD level 3 products on an EASE or polar stereographic grid in netCDF format. It's been tested with the following products:

  • Ice concentration

  • Ice type

  • Ice edge

  • Ice emissivity

  • Tests added

Copy link

codecov bot commented Nov 8, 2023

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (ed220be) 95.21% compared to head (98061ec) 95.21%.

Files Patch % Lines
satpy/readers/osisaf_l3_nc.py 97.90% 3 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main    #2632    +/-   ##
========================================
  Coverage   95.21%   95.21%            
========================================
  Files         356      358     +2     
  Lines       51605    51956   +351     
========================================
+ Hits        49134    49471   +337     
- Misses       2471     2485    +14     
Flag Coverage Δ
behaviourtests 4.19% <0.00%> (-0.04%) ⬇️
unittests 95.83% <99.14%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@coveralls
Copy link

coveralls commented Nov 8, 2023

Pull Request Test Coverage Report for Build 7020440556

  • 348 of 351 (99.15%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.02%) to 95.807%

Changes Missing Coverage Covered Lines Changed/Added Lines %
satpy/readers/osisaf_l3_nc.py 140 143 97.9%
Totals Coverage Status
Change from base Build 7019457197: 0.02%
Covered Lines: 49608
Relevant Lines: 51779

💛 - Coveralls

@djhoese djhoese added enhancement code enhancements, features, improvements component:readers labels Nov 13, 2023
return self["/attr/platform_name"]
except KeyError:
return self["/attr/platform"]

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think I inadvertently fixed this while sorting out another suggestion.

Comment on lines 137 to 142
def _get_ds_attr(self, a_name):
"""Get a dataset attribute and check it's valid."""
try:
return self[a_name]
except KeyError:
return None
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this the same thing:

def get(self, item, default=None):
"""Get item."""
if item in self:
return self[item]
else:
return default

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Err, apparently yes! Removed.

Copy link
Collaborator

@gerritholl gerritholl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Some minor comments.

It can't read the files we get from EUMETCAST, which have the format S-OSI_-DMI_-MULT-GL_NH_CONCn__-202304251200Z.nc. But then again, nor can we (our internal reader gives us an HDF error since a while).

satpy/readers/osisaf_l3_nc.py Outdated Show resolved Hide resolved
satpy/readers/osisaf_l3_nc.py Outdated Show resolved Hide resolved
Comment on lines 139 to 142
try:
return self[a_name]
except KeyError:
return None
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why swallow the KeyErrors? Wouldn't that hide bugs elsewhere?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because various versions of the files have different key names, so I have to try multiple attempts. Some of them don't have the attr at all - hence hiding the error. Would you prefer if I added a log warning or something?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the main way this is used is to try multiple possible attribute/variable names, then maybe a different version of this with a for loop would be better?

attr_val = self._get_first_attr(("/attr/X", "/attr/Y", "/attr/Z"))


def _get_first_attr(self, possible_names):
    for attr_name in possible_names:
        try:
            return self[attr_name]
        except KeyError:
            continue
    raise KeyError("...")

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hopefully my newest change is suitable. If not, I'll come back to it some time in the new year.

satpy/tests/reader_tests/test_osisaf_l3.py Outdated Show resolved Hide resolved
satpy/readers/osisaf_l3_nc.py Outdated Show resolved Hide resolved
satpy/readers/osisaf_l3_nc.py Outdated Show resolved Hide resolved
satpy/readers/osisaf_l3_nc.py Show resolved Hide resolved
satpy/readers/osisaf_l3_nc.py Outdated Show resolved Hide resolved
@simonrp84
Copy link
Member Author

I think I've addressed all your comments, thanks for the reviews!

@gerritholl
Copy link
Collaborator

gerritholl commented Nov 14, 2023

Thanks! It works with the files we get on EUMETCAST now.

@gerritholl
Copy link
Collaborator

[x] Closes #1955

(does that work automatically in a comment?)

@mraspaud mraspaud merged commit df66a1c into pytroll:main Nov 28, 2023
18 of 19 checks passed
@simonrp84 simonrp84 deleted the osi_saf branch November 28, 2023 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:readers enhancement code enhancements, features, improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants