Implement py public decomp wt hrit base

Graenni commented 2 months ago

[ ] Closes #2900
[ ] Tests added
[ ] Fully documented
[x] Add your name to AUTHORS.md if not there already

mraspaud commented 2 months ago

Also the tests are failing because pyPublicDecompWT is not included in the environment file for github actions: https://github.com/pytroll/satpy/blob/main/continuous_integration/environment.yaml

mraspaud commented 2 months ago

@sfinkens @ameraner @pnuu do you have any opinion on this PR?

mraspaud commented 2 months ago

@Graenni CI is failing, is the decompress function function working as it should with compressed data for you? In which case it might be the mocking messing things up.

Graenni commented 2 months ago

@mraspaud yes, I am able to successfully read and process compressed HRIT files.

pnuu commented 2 months ago

Which Python version are you using while running Satpy locally?

The error is

FAILED satpy/tests/reader_tests/test_hrit_base.py::TestHRITFileHandlerCompressed::test_read_band_filepath - TypeError: a bytes-like object is required, not 'str'

I'll see what I get running the tests, but first lunch :grin:

Graenni commented 2 months ago

I run Python3.11. I can successfully load seviri l1b hrit with compression.

pnuu commented 2 months ago

I get the same error while running the tests locally. Do they pass for you?

pnuu commented 2 months ago

That is, does pytest satpy/tests/reader_tests/test_hrit_base.py::TestHRITFileHandlerCompressed pass?

pnuu commented 2 months ago

I added a debugger break-point in the decompress() function and it is never accessed by the tests.

pnuu commented 2 months ago

No wonder the function is not called as the first mock just overrides it:

        with mock.patch("satpy.readers.hrit_base.decompress", side_effect=fake_decompress) as mock_decompress:

Graenni commented 2 months ago

I did not run the tests locally so far, sorry. I just used satpy with the changes of this pr as I would normally and then the decompression worked as expected. Seems to be an issue with the way tests are configured as you already figured out.

mraspaud commented 2 months ago

@Graenni I think I fixed the tests, can you check my changes and tell me if the fake decompression is inline with what you expect?

pnuu commented 2 months ago

I got TypeError: fake_decompress() takes 0 positional arguments but 1 was given.

codecov[bot] commented 2 months ago

Codecov Report

Attention: Patch coverage is 77.27273% with 5 lines in your changes missing coverage. Please review.

Project coverage is 96.07%. Comparing base (99697f7) to head (862c558). Report is 23 commits behind head on main.

Files with missing lines	Patch %	Lines
satpy/readers/hrit_base.py	44.44%	5 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #2903 +/- ## ======================================= Coverage 96.06% 96.07% ======================================= Files 370 371 +1 Lines 54320 54288 -32 ======================================= - Hits 52185 52158 -27 + Misses 2135 2130 -5 ``` | [Flag](https://app.codecov.io/gh/pytroll/satpy/pull/2903/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/2903/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pytroll) | `4.00% <0.00%> (+<0.01%)` | :arrow_up: | | [unittests](https://app.codecov.io/gh/pytroll/satpy/pull/2903/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pytroll) | `96.17% <77.27%> (+<0.01%)` | :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.

coveralls commented 2 months ago

Pull Request Test Coverage Report for Build 10922986535

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

17 of 22 (77.27%) changed or added relevant lines in 2 files are covered.
3 unchanged lines in 1 file lost coverage.
Overall coverage increased (+0.007%) to 96.173%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
satpy/readers/hrit_base.py	4	9	44.44%
<!--	Total:	17	22	77.27%	-->

Files with Coverage Reduction	New Missed Lines	%
satpy/readers/seadas_l2.py	3	96.97%
<!--	Total:	3		-->

Totals
Change from base Build 10814739875:	0.007%
Covered Lines:	52390
Relevant Lines:	54475

💛 - Coveralls

sfinkens commented 2 months ago

@sfinkens @ameraner @pnuu do you have any opinion on this PR?

Awesome, thanks a lot for adding this @Graenni !

Graenni commented 2 months ago

@Graenni I think I fixed the tests, can you check my changes and tell me if the fake decompression is inline with what you expect?

@mraspaud yes, the return value of fake_decompress is in agreement with what is returned by the original decompress function. the success of the test might be a bit misleading, since decompression is not really tested. But because we do not have the possibility to compress some data (do we?), I don't see a better way to at least test the workflow when the compression flag is set in the header.

mraspaud commented 2 months ago

I'm merging this, thanks for the contribution @Graenni !

pytroll / satpy