Closed mraspaud closed 5 months ago
Attention: Patch coverage is 98.65772%
with 2 lines
in your changes are missing coverage. Please review.
Project coverage is 77.04%. Comparing base (
cbf2d7c
) to head (2753057
). Report is 1 commits behind head on main.
Files | Patch % | Lines |
---|---|---|
pygac/pod_reader.py | 88.88% | 2 Missing :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Nice work! I had a couple inline refactoring ideas and two thoughts:
1. I wonder what happens if `PODReader.can_read` (inherited from `Reader`) calls `PODReader.read_header` without the `eosip_header` argument. Would that work with EOSIP files?
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...
2. 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
That would be really nice, however how will satpy know what pygac header to use?
how will satpy know what pygac header to use?
Good point... I tried, but I'm not sure this is better...
pygac:
EOSIP_HEADER = header3.copy()
satpy:
from pygac import EOSIP_HEADER
reader_kwargs["header"] = EOSIP_HEADER
I just thought while cycling home that we could instead pass the header timstamp? eg
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?
how will satpy know what pygac header to use?
Good point... I tried, but I'm not sure this is better...
pygac: EOSIP_HEADER = header3.copy()
satpy: from pygac import EOSIP_HEADER reader_kwargs["header"] = EOSIP_HEADER
I like that!
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.