-
Notifications
You must be signed in to change notification settings - Fork 27
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
Support reading EO-SIP LAC data #125
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #125 +/- ##
==========================================
+ Coverage 74.39% 77.04% +2.64%
==========================================
Files 32 32
Lines 2882 3001 +119
==========================================
+ Hits 2144 2312 +168
+ Misses 738 689 -49 ☔ View full report in Codecov by Sentry. |
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.
Nice work! I had a couple inline refactoring ideas and two thoughts:
- I wonder what happens if
PODReader.can_read
(inherited fromReader
) callsPODReader.read_header
without theeosip_header
argument. Would that work with EOSIP files? - Would it make sense to make the signature format agnostic?
read_header(..., header="auto") # determine based on timestamp
read_header(..., header=header3) # use exactly this header
Yes, that is not a problem since the header isn't used for anything in that function, just check that is doesn't raise any errors...
That would be really nice, however how will satpy know what pygac header to use? |
Good point... I tried, but I'm not sure this is better...
|
I just thought while cycling home that we could instead pass the header timstamp? read_header(..., header_timestamp="auto") # for automatic, current behaviour, default
read_header(..., header_timestamp=datetime(2000, 1, 1)) # to choose a specific one or is this too obscure?
|
I like 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.
Awesome, thanks for adding a test with a real file! There are some codefactor warnings (E241), otherwise looks good to me. 👍
This PR adds support for the EO-SIP LAC formats.
In particular, the EBCDIC decoding for dataset names is implemented, and the fixed header type for POD data is added as an option.