pytroll / satpy

Python package for earth-observing satellite data processing
http://satpy.readthedocs.org/en/latest/
GNU General Public License v3.0
1.08k stars 298 forks source link

Update pyhdf-based arrs to be manually tokenized #2886

Closed djhoese closed 3 months ago

djhoese commented 3 months ago

This avoids a bug in dask or cloudpickle that alters the state of the pyhdf SDS object in some way making it unusable. The PR in dask that started triggering this was in https://github.com/dask/dask/pull/11320. My guess is that the repeated pickling/serialization is losing some hidden state in the pyhdf SDS object and then pyhdf or HDF-C no longer knows how to work with it.

We could register SDS with normalize_token in dask or we could just do what I do in this PR and come up with the token/name ourselves. Note this is the name for the dask array/task. This is similar to work done in the past by @mraspaud for the HDF5 utility package to make sure things are consistent across file variable loads.

One alternative to the PR as it is right now would be to copy from_sds as a method on the HDF utility class so it knows how to use self.filename automatically for the tokenizing. Also I'm realizing maybe the src_path shouldn't be required as it is only used if the name kwarg isn't provided. Thoughts @mraspaud @gerritholl ?

codecov[bot] commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 96.05%. Comparing base (6860030) to head (5e27be4). Report is 400 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #2886 +/- ## ========================================== + Coverage 95.99% 96.05% +0.06% ========================================== Files 368 370 +2 Lines 53985 54320 +335 ========================================== + Hits 51821 52177 +356 + Misses 2164 2143 -21 ``` | [Flag](https://app.codecov.io/gh/pytroll/satpy/pull/2886/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pytroll) | Coverage Δ | | |---|---|---| | [behaviourtests](https://app.codecov.io/gh/pytroll/satpy/pull/2886/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pytroll) | `3.99% <0.00%> (-0.03%)` | :arrow_down: | | [unittests](https://app.codecov.io/gh/pytroll/satpy/pull/2886/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pytroll) | `96.15% <100.00%> (+0.06%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pytroll#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.


🚨 Try these New Features:

coveralls commented 3 months ago

Pull Request Test Coverage Report for Build 10527846171

Details


Totals Coverage Status
Change from base Build 10404388488: 0.001%
Covered Lines: 52408
Relevant Lines: 54506

💛 - Coveralls