-
Notifications
You must be signed in to change notification settings - Fork 287
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
Update the HRPT reader to latest satpy api #1531
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1531 +/- ##
==========================================
+ Coverage 91.65% 92.02% +0.36%
==========================================
Files 248 249 +1
Lines 36172 36309 +137
==========================================
+ Hits 33155 33413 +258
+ Misses 3017 2896 -121
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
DeepCode failed to analyze this pull requestSomething went wrong despite trying multiple times, sorry about that. |
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.
Thanks very much for the quick PR!
But I think in hrpt.py get_dataset is treating key as a dict but it's a tuple so instead of key['name'] you need to use key.name (and same for key.calibration).
@howff the key is in principle an instance of a DataID. The recommended way to access its fields now is with |
The tuple problem was probably me patching hrpt.py into a version of satpy older than master. But when I use this branch I can confirm that the data loads ok and the built-in composites also work, so thank you very much :) |
You are very welcome, it was high time to update this reader! So thank you for poking us. |
@howff I think I'm happy with this PR now. Any further comments? |
I'm getting an error message with our files. from satpy import Scene
sc = Scene(["/media/nas/x21308/scratch/20210205141010_NOAA_19.hmf"], reader="avhrr_l1b_hrpt")
sc.load(["4"])
sc.show("4") gives:
|
Do you need https://github.com/pytroll/python-geotiepoints for that? |
Same problem after installing the latest
from
|
@TAlonglong yes, and pyorbital and pygac |
@gerritholl do you have python-geotiepoints installed? |
@gerritholl fun! I guess some consistancy check is in order... |
Some misplaced scanlines is to be expected if the timestamps have been corrupted due to noisy data. |
@howff, yes, corrupted timestamps are probably the issue. I was thinking of trying to fit the timestamps to a line to correct the outliers, but how would you do it? |
I agree that it's not a bug in Satpy, rather a missing feature — but as long as the data quality reading directly is worse than when pre-processing it with AAPP, users are going to keep AAPP in their pipelines despite the additional overhead. |
Doesn't pygac have a lot of experience in filtering the dozens of things that can go wrong with data? Could that help here? There's already a pygac dependency anyway... |
ah no that won't work as HRPT doesn't have line numbers |
I would be happy to have this PR merged, to get the new API working, and leave the handling of dirty data to a different PR. |
ok, great, I will merge this one then. @howff do you want to take a shot at that new PR for filter or fixing erroneous timestamps? |
This PR adapts the HRPT reader to dask, xarray and latest pygac
flake8 satpy