Implement py public decomp wt hrit base

[Graenni]

• Closes Eliminate dependency on external binaries of PublicDecompWT (xRITDecompress) by using pyPublicDecompWT #2900
• Tests added
• Fully documented
• Add your name to AUTHORS.md if not there already

[mraspaud]

Thanks a lot for looking into replacing the xrit decompression with this nice library!
That removes quite some code, which is really nice :)

I have a couple of comments inline, but also a few questions:
Do we have tests that cover this new functionality? I would suspect we don't, because we don't have a test compressed hrit file in the repo. If I'm right, I was wondering if pyPulbicDecompWT also provides a function to compress a file? Because we are creating stub hrit files for testsing, so if we could compress one, we could actually test the decompression.

[mraspaud]

I think this should be in the optional dependencies for seviri_l1b_hrit rather than here, right?

[Graenni]

Yes, this is correct. Note that the seviri_l1b_hrit reader will no longer be available without explicitly installing the necessary dependencies. Therefore, the default functionalities after a plain pip install satpy will change slightly. Even for reading uncompressed HRIT files, the pyPublicDecompWT package will be required. From my perspective, this is not an issue.

[pnuu]

This is an issue for at least us, as we want to keep the containers used in production as minimal as possible. Our file transfer system (Trollmoves) already decompresses the HRIT files so we don't need the decompression library installed. So pyPublicDeompWT should be an optional extra, as it is also simple to make it such.

[mraspaud]

Ok, so I think we need to move that dependency to the optional dependency part.
But also, in order to keep backwards compatibility, we should do the import inside the decompress function. Otherwise, as you say, the library needs to be installed even when reading non-compressed data.

[Graenni]

ok, I have moved it to the optional dependencies

[mraspaud]

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]

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

[pnuu]

Simple changes to make the decompression library optional.

[mraspaud]

@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]

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

[pnuu]

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 😁

[Graenni]

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

[pnuu]

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

[pnuu]

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

[pnuu]

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

[pnuu]

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]

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]

@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]

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

[codecov]

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 ⚠️

Additional details and impacted files

@@           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	Coverage Δ
behaviourtests	4.00% <0.00%> (+<0.01%)	⬆️
unittests	96.17% <77.27%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[coveralls]

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%

Files with Coverage Reduction	New Missed Lines	%
satpy/readers/seadas_l2.py	3	96.97%

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

---

💛 - Coveralls

[sfinkens]

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

Awesome, thanks a lot for adding this @Graenni !

[Graenni]

@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]

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