-
Notifications
You must be signed in to change notification settings - Fork 285
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
Fix instrument name reading from attributes for VIIRS L2 data #2760
Conversation
Any ideas what would be the best way to adapt the tests to check for both attribute versions? Dupplicating the fake filehandler class and the test class doesn't seem very attractive... |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2760 +/- ##
==========================================
- Coverage 95.94% 95.92% -0.02%
==========================================
Files 375 377 +2
Lines 53293 53502 +209
==========================================
+ Hits 51130 51321 +191
- Misses 2163 2181 +18
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Ok, duplicated the minimal amount of test code I thought was necessary. |
Pull Request Test Coverage Report for Build 8264934533Details
💛 - Coveralls |
CC @wjsharpe, Does this look good to you? |
Posting the realization from slack here for the record: @pnuu was using the old "viirs_l2.py" reader that he had created a while ago. A couple months ago I reworked all the VIIRS EDR (note EDR, not L2) to be one |
Looks good to me |
I should've remembered that |
Yeah, this PR was a mixup with the old |
The some of the attributes in the CSPP/ASCI files have been renamed between v3r0 and v3r2. The only on I've seen that affects Satpy is the instrument name. The original reader version was based on older data and works at least until v3r0 files, but the current one seems to handle only the newer files. With this PR both can be read.
AUTHORS.md
if not there already