Adapt the SEVIRI native format reader in Satpy to support remote reading #2863

pkhalaj · 2024-07-26T11:46:33Z

This PR adds a feature to Satpy to allow for reading SEVIRI native format files remotely. It relies on the fsspec package.

Closes #xxxx
Tests added
Fully documented
Add your name to AUTHORS.md if not there already

codecov · 2024-07-26T12:00:18Z

Codecov Report

Attention: Patch coverage is 96.72131% with 4 lines in your changes missing coverage. Please review.

Project coverage is 96.03%. Comparing base (b25107d) to head (d22d2b6).
Report is 1 commits behind head on main.

Files	Patch %	Lines
satpy/readers/seviri_l1b_native.py	81.81%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2863      +/-   ##
==========================================
+ Coverage   95.97%   96.03%   +0.05%     
==========================================
  Files         368      368              
  Lines       53973    54057      +84     
==========================================
+ Hits        51801    51914     +113     
+ Misses       2172     2143      -29

Flag	Coverage Δ
behaviourtests	`4.01% <0.00%> (-0.01%)`	⬇️
unittests	`96.13% <96.72%> (+0.05%)`	⬆️

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 · 2024-07-26T12:13:24Z

Pull Request Test Coverage Report for Build 10382915666

Details

118 of 122 (96.72%) changed or added relevant lines in 3 files are covered.
8 unchanged lines in 3 files lost coverage.
Overall coverage increased (+0.05%) to 96.132%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
satpy/readers/seviri_l1b_native.py	18	22	81.82%

Files with Coverage Reduction	New Missed Lines	%
satpy/tests/utils.py	2	93.16%
satpy/tests/reader_tests/gms/test_gms5_vissr_l1b.py	3	98.67%
satpy/tests/reader_tests/gms/test_gms5_vissr_navigation.py	3	97.18%

Totals
Change from base Build 10303695673:	0.05%
Covered Lines:	52143
Relevant Lines:	54241

💛 - Coveralls

mraspaud

Very nice so far, it's really good to have an actual file to test the reader on now.

AUTHORS.md

satpy/readers/seviri_l1b_native.py

satpy/tests/reader_tests/test_seviri_l1b_native.py

sfinkens · 2024-07-29T12:47:54Z

Nice work, thanks a lot! Can you please update the fsspec support column for this reader in the reader table?

pkhalaj · 2024-07-29T13:19:05Z

Nice work, thanks a lot! Can you please update the fsspec support column for this reader in the reader table?

Thanks. Yes, I just did that!

ameraner

LGTM, just a question on the lazyness when switching from memmap to frombuffer.

satpy/readers/seviri_l1b_native.py

This function uses `readers.utils.generic_open()` and `np.frombuffer()` to achieve this.

In particular, the following functions/methods have been modified: - `has_archive_header()` now uses `readers.utils.generic_open()` instead of `open()`. - `read_header()` now uses `readers.utils.fromfile()` instead of `np.formfile()`. - `NativeMSGFileHandler._read_trailer()` now uses `readers.utils.fromfile()` instead of `np.formfile()`. - `NativeMSGFileHandler._get_memmap()` has been renamed to `NativeMSGFileHandler._get_array()` and now uses `readers.utils.fromfile()` instead of `np.memmap()`.

Reason: since `NativeMSGFileHandler._get_memmap()` has been renamed to `NativeMSGFileHandler._get_array()`.

…n `seviri_l1b_native`

This includes generating an actual file on disk and attempt to read it.

… well as plain files

…le()`

This concerns `test_read_physical_seviri_nat_file()`.

This is to ensure that the remote reading tests can also run on Windows. Note: `fsspec` expects a POSIX path.

…unknown marks We do not need these marks anymore, as we decreased the size of the generated seviri native file. As a result, the tests now run fast enough.

This warning is totally benign. It is caused as a result of the seviri native file that we create which is essentially filled with zeros.

The issue was an unexpected indentation on the last line of the docstring!

…readers table

This includes: - Extracting the `_get_array()` method so that it is now a function in the module and not a class method. - Introduction of `NativeMSGFileHandler_make_dask_array_with_map_blocks()` method to utilize the dask `map_blocks()`. - Introduction of a new method, namely `NativeMSGFileHandler._number_of_visir_channels` to facilitate testing and mock patching. - Adapting the mock patches in tests accordingly.

mraspaud · 2024-08-12T07:28:18Z

@ameraner @djhoese do you have time to have a quick look at this and tell us if you agree with the new map_blocks usage?

satpy/readers/seviri_l1b_native.py

…array

…rint?)

ameraner

LGTM, thanks for the updates!

djhoese

As much as I understand of this, it looks good enough to me. Thanks for putting all this work into it.

djhoese · 2024-08-14T01:14:48Z

Oh CodeScene doesn't like how long the test module is. Any chance things can be refactored?

satpy/tests/reader_tests/test_seviri_l1b_native.py

mraspaud · 2024-08-14T09:12:58Z

Oh CodeScene doesn't like how long the test module is. Any chance things can be refactored?

Yes, we talked about this, and decided it was a job for another PR, as what would make the module smaller it so redo the mocking tests into tests using the actual file.
So I'm merging this.

mraspaud reviewed Jul 29, 2024

View reviewed changes

AUTHORS.md Show resolved Hide resolved

satpy/readers/seviri_l1b_native.py Outdated Show resolved Hide resolved

satpy/tests/reader_tests/test_seviri_l1b_native.py Outdated Show resolved Hide resolved

satpy/tests/reader_tests/test_seviri_l1b_native.py Outdated Show resolved Hide resolved

pkhalaj added a commit to pkhalaj/satpy that referenced this pull request Jul 29, 2024

Address @mraspaud comments on PR pytroll#2863

ea8524e

pkhalaj added a commit to pkhalaj/satpy that referenced this pull request Jul 29, 2024

Address @mraspaud comments on PR pytroll#2863

455d4a8

pkhalaj force-pushed the feature/read_remote_seviri_native branch from ea8524e to 455d4a8 Compare July 29, 2024 08:43

pkhalaj marked this pull request as ready for review July 29, 2024 08:44

pkhalaj requested review from sjoro, sfinkens and djhoese as code owners July 29, 2024 08:44

pkhalaj requested a review from mraspaud July 29, 2024 08:44

mraspaud requested a review from ameraner July 29, 2024 10:48

mraspaud reviewed Jul 29, 2024

View reviewed changes

satpy/tests/reader_tests/test_seviri_l1b_native.py Outdated Show resolved Hide resolved

pkhalaj marked this pull request as draft July 29, 2024 11:22

pkhalaj marked this pull request as ready for review July 29, 2024 11:33

ameraner reviewed Jul 29, 2024

View reviewed changes

satpy/readers/seviri_l1b_native.py Outdated Show resolved Hide resolved

pkhalaj force-pushed the feature/read_remote_seviri_native branch from 2b9eca8 to 9fc2a05 Compare July 31, 2024 14:06

pkhalaj requested review from ameraner and mraspaud July 31, 2024 14:17

pkhalaj added 9 commits August 12, 2024 09:21

Add readers.utils.fromfile() for remote reading

5401d28

This function uses `readers.utils.generic_open()` and `np.frombuffer()` to achieve this.

Update AUTHORS.md

7142a68

Reformat readers.utils to make it PEP8 compliant

8051d50

Update test_seviri_l1b_native with _get_array

1f10690

Reason: since `NativeMSGFileHandler._get_memmap()` has been renamed to `NativeMSGFileHandler._get_array()`.

Update mock.patch args in test_seviri_l1b_native to match changes i…

7d516d8

…n `seviri_l1b_native`

Update test_seviri_l1b_native with tests for remote reading

d01a519

This includes generating an actual file on disk and attempt to read it.

Parametrize test_read_physical_seviri_nat_file to test zip files as…

0e5f3e7

… well as plain files

Reformat test_seviri_l1b_native to make it PEP8 compliant

15d17d6

pkhalaj added 9 commits August 12, 2024 09:21

Address @mraspaud comments on PR pytroll#2863

df4a6db

Simplify fixtures and parameters of `test_read_physical_seviri_nat_fi…

43b846b

…le()`

Use a header which leads to a smaller seviri nat file on disk

816a2c7

This concerns `test_read_physical_seviri_nat_file()`.

Make the returned path posix compliant in compress_seviri_native_file

d4976c3

This is to ensure that the remote reading tests can also run on Windows. Note: `fsspec` expects a POSIX path.

Resolve warnings raised as a result of slow & order being pytest …

db3ddb8

…unknown marks We do not need these marks anymore, as we decreased the size of the generated seviri native file. As a result, the tests now run fast enough.

Resolve warnings raised as a failure in the orbit polynomial

fbd02b5

This warning is totally benign. It is caused as a result of the seviri native file that we create which is essentially filled with zeros.

Fix the issue with the docstring in amend_seviri_native_null_header()

f002b00

The issue was an unexpected indentation on the last line of the docstring!

Update fsspec support column for seviri_l1b_native reader in the …

a7596d3

…readers table

pkhalaj force-pushed the feature/read_remote_seviri_native branch from 9fc2a05 to 80f3925 Compare August 12, 2024 07:21

djhoese added enhancement code enhancements, features, improvements component:readers labels Aug 12, 2024

djhoese reviewed Aug 12, 2024

View reviewed changes

satpy/readers/seviri_l1b_native.py Outdated Show resolved Hide resolved

satpy/readers/seviri_l1b_native.py Outdated Show resolved Hide resolved

satpy/readers/seviri_l1b_native.py Show resolved Hide resolved

pkhalaj added 3 commits August 13, 2024 10:49

Replace np.zeros with np.empty in the meta parameter of the dask …

42dfc05

…array

Making dask_array private by prefixing it with _

1c00de7

Replace np.empty with np.array for performance (less memory footp…

b58823c

…rint?)

ameraner approved these changes Aug 13, 2024

View reviewed changes

djhoese approved these changes Aug 14, 2024

View reviewed changes

djhoese reviewed Aug 14, 2024

View reviewed changes

satpy/tests/reader_tests/test_seviri_l1b_native.py Outdated Show resolved Hide resolved

Change the first line of docstrings to imperative mood.

d22d2b6

mraspaud merged commit 82d8ef3 into pytroll:main Aug 14, 2024
16 of 18 checks passed

mraspaud assigned pkhalaj Aug 14, 2024

pkhalaj deleted the feature/read_remote_seviri_native branch August 14, 2024 09:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adapt the SEVIRI native format reader in Satpy to support remote reading #2863

Adapt the SEVIRI native format reader in Satpy to support remote reading #2863

pkhalaj commented Jul 26, 2024 •

edited

Loading

codecov bot commented Jul 26, 2024 •

edited

Loading

coveralls commented Jul 26, 2024 •

edited

Loading

mraspaud left a comment

sfinkens commented Jul 29, 2024

pkhalaj commented Jul 29, 2024

ameraner left a comment

mraspaud commented Aug 12, 2024

ameraner left a comment

djhoese left a comment

djhoese commented Aug 14, 2024

mraspaud commented Aug 14, 2024

Adapt the SEVIRI native format reader in Satpy to support remote reading #2863

Adapt the SEVIRI native format reader in Satpy to support remote reading #2863

Conversation

pkhalaj commented Jul 26, 2024 • edited Loading

codecov bot commented Jul 26, 2024 • edited Loading

Codecov Report

coveralls commented Jul 26, 2024 • edited Loading

Pull Request Test Coverage Report for Build 10382915666

Details

💛 - Coveralls

mraspaud left a comment

Choose a reason for hiding this comment

sfinkens commented Jul 29, 2024

pkhalaj commented Jul 29, 2024

ameraner left a comment

Choose a reason for hiding this comment

mraspaud commented Aug 12, 2024

ameraner left a comment

Choose a reason for hiding this comment

djhoese left a comment

Choose a reason for hiding this comment

djhoese commented Aug 14, 2024

mraspaud commented Aug 14, 2024

pkhalaj commented Jul 26, 2024 •

edited

Loading

codecov bot commented Jul 26, 2024 •

edited

Loading

coveralls commented Jul 26, 2024 •

edited

Loading