satpy Share FCI L1c metadata between segments

Some of the metadata are identical in every FCI L1c segment, so reading those only once is possible. This will save a lot of time in Scene creation when the data are in S3 storage:

main - 37.0 s
PR - 23.0 s

There will be conflicts with https://github.com/pytroll/satpy/pull/2686, but maybe adding the pickle would benefit also this feature.

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

Jun 14 '24 12:06 pnuu

Codecov Report

Attention: Patch coverage is 96.87500% with 2 lines in your changes missing coverage. Please review.

Project coverage is 95.94%. Comparing base (ee2273a) to head (f46df94). Report is 10 commits behind head on main.

Files	Patch %	Lines
satpy/readers/fci_l1c_nc.py	91.30%	2 Missing :warning:

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2828   +/-   ##
=======================================
  Coverage   95.94%   95.94%           
=======================================
  Files         366      366           
  Lines       53515    53580   +65     
=======================================
+ Hits        51343    51409   +66     
+ Misses       2172     2171    -1

Flag	Coverage Δ
behaviourtests	`4.04% <0.00%> (-0.01%)`	:arrow_down:
unittests	`96.04% <96.87%> (+<0.01%)`	:arrow_up:

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

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

Jun 14 '24 12:06 codecov[bot]

Pull Request Test Coverage Report for Build 9515844065

Details

9 of 28 (32.14%) changed or added relevant lines in 2 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-0.03%) to 96.006%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
satpy/readers/fci_l1c_nc.py	4	23	17.39%
<!--	Total:	9	28

Totals
Change from base Build 9513521871:	-0.03%
Covered Lines:	51566
Relevant Lines:	53711

💛 - Coveralls

Jun 14 '24 12:06 coveralls

I'll see what I can do for testing. It's not easy, because the file handler is never used in the tests.

Jun 14 '24 12:06 pnuu

I've now added two tests that hopefully show that the storing and reusing of common items between the FCI L1c segments work.

Jun 19 '24 11:06 pnuu

Pull Request Test Coverage Report for Build 9581505593

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

48 of 51 (94.12%) changed or added relevant lines in 3 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-0.002%) to 96.038%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
satpy/readers/fci_l1c_nc.py	20	23	86.96%
<!--	Total:	48	51

Totals
Change from base Build 9513521871:	-0.002%
Covered Lines:	51605
Relevant Lines:	53734

💛 - Coveralls

Jun 19 '24 12:06 coveralls

Pull Request Test Coverage Report for Build 9592333785

Details

48 of 51 (94.12%) changed or added relevant lines in 3 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-0.002%) to 96.039%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
satpy/readers/fci_l1c_nc.py	20	23	86.96%
<!--	Total:	48	51

Totals
Change from base Build 9588363290:	-0.002%
Covered Lines:	51621
Relevant Lines:	53750

💛 - Coveralls

Jun 20 '24 06:06 coveralls

Pull Request Test Coverage Report for Build 9592773461

Details

49 of 52 (94.23%) changed or added relevant lines in 3 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-0.002%) to 96.039%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
satpy/readers/fci_l1c_nc.py	20	23	86.96%
<!--	Total:	49	52

Totals
Change from base Build 9588363290:	-0.002%
Covered Lines:	51622
Relevant Lines:	53751

💛 - Coveralls

Jun 20 '24 06:06 coveralls

Pull Request Test Coverage Report for Build 9593191408

Details

62 of 64 (96.88%) changed or added relevant lines in 3 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.006%) to 96.047%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
satpy/readers/fci_l1c_nc.py	21	23	91.3%
<!--	Total:	62	64

Totals
Change from base Build 9588363290:	0.006%
Covered Lines:	51638
Relevant Lines:	53763

💛 - Coveralls

Jun 20 '24 08:06 coveralls

Interesting! In the code I can see some metadata listed as non-shareable. Do you have a list of the shareable metadata? i.e: Which ones don't change between segments and are hence affected by this PR?

Jun 20 '24 08:06 simonrp84

Anything in the list shown here that don't end in the strings listed in NONSHAREABLE_VARIABLE_ENDINGS can be shared between the segments. Note that the {chan_name} is replaced with the 16 channels FCI has, so the list is quite long.

Jun 20 '24 08:06 pnuu

Interesting! In the code I can see some metadata listed as non-shareable. Do you have a list of the shareable metadata? i.e: Which ones don't change between segments and are hence affected by this PR?

In https://github.com/pytroll/satpy/pull/2686/files#diff-2170f8edf16088150763d5f3a6cbd69d62600d238c0ba80a41afcb4832fb7b5d I explicitly list what metadata can be shared between segments.

Jun 20 '24 09:06 gerritholl

In addition, could more implementation be in netcdf_utils so it can be used by other readers where relevant?

I think so. As I said in another comment above this snowballed from a quick test at PCW so I touched as little other parts of the code as possible. The FCI L1c data format is the most demanding in this regard, and most important to me, so started here.

I'll see about generalizing things later and converting this as a draft for the summer period.

Jun 20 '24 10:06 pnuu

Pull Request Test Coverage Report for Build 9596154108

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

62 of 64 (96.88%) changed or added relevant lines in 3 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.006%) to 96.047%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
satpy/readers/fci_l1c_nc.py	21	23	91.3%
<!--	Total:	62	64

Totals
Change from base Build 9588363290:	0.006%
Covered Lines:	51638
Relevant Lines:	53763

💛 - Coveralls

Jun 20 '24 11:06 coveralls