Add masking of data with line quality flags to seviri nc reader

[BENR0]

This PR adds masking of data with the line quality flags to the Seviri nc reader. In the Seviri hrit reader this masking is already implemented and I think this behavior should be consistent between the readers.

I refactored the main logic of the masking out of the hrit reader to reuse it in the nc reader. There is no test for the masking itself yet (by the way I noticed that it isn't tested in the hrit reader either). Will add the test if this get's a go.

Just saw #1482 which is related and would be nice to have.

• [ ] Tests added
• [ ] Fully documented

[codecov[bot]]

Codecov Report

Merging #1693 (3000ef1) into main (3a8a139) will increase coverage by 0.02%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1693      +/-   ##
==========================================
+ Coverage   94.33%   94.36%   +0.02%     
==========================================
  Files         310      310              
  Lines       46575    46641      +66     
==========================================
+ Hits        43938    44013      +75     
+ Misses       2637     2628       -9     

Flag	Coverage Δ
behaviourtests	4.59% <0.00%> (-0.01%)	:arrow_down:
unittests	95.00% <100.00%> (+0.02%)	:arrow_up:

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

Impacted Files	Coverage Δ
...y/tests/reader_tests/test_seviri_l1b_hrit_setup.py	100.00% <ø> (ø)
satpy/readers/seviri_base.py	100.00% <100.00%> (ø)
satpy/readers/seviri_l1b_hrit.py	90.45% <100.00%> (+0.03%)	:arrow_up:
satpy/readers/seviri_l1b_nc.py	70.34% <100.00%> (+1.83%)	:arrow_up:
satpy/tests/reader_tests/test_seviri_l1b_hrit.py	100.00% <100.00%> (ø)
satpy/tests/reader_tests/test_seviri_l1b_nc.py	100.00% <100.00%> (ø)
satpy/tests/test_config.py	97.18% <0.00%> (+0.40%)	:arrow_up:
satpy/tests/reader_tests/test_vii_base_nc.py	100.00% <0.00%> (+1.24%)	:arrow_up:
satpy/tests/reader_tests/test_nwcsaf_msg.py	100.00% <0.00%> (+1.80%)	:arrow_up:
satpy/tests/reader_tests/test_vii_l1b_nc.py	100.00% <0.00%> (+2.81%)	:arrow_up:
... and 1 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[coveralls]

Coverage increased (+0.007%) to 94.955% when pulling 3000ef1c1eff760417be72ed9c1cf359ea658037 on BENR0:feat_line_quality_flags_seviri_nc_reader into 3a8a13935247d82b8b4f10c40ef522d792d99625 on pytroll:main.

[BENR0]

@sfinkens @djhoese @mraspaud @adybbroe what's the status on this? Is this still wanted? I am still not convinced on the refactoring suggested by @sfinkens since it adds unnecessary complexity from my point of view.

[BENR0]

So it seems there are no other opinions. So I guess it's ready to be merged ;-).

[mraspaud]

Just some conflicts with main to solve...

[BENR0]

@mraspaud @djhoese I think this is ready to go then.

[simonrp84]

Just to check I'm reading the code correctly: Am I right in thinking that this masking is optional and is disabled by default?

[BENR0]

@simonrp84 no this is implemented the same as in the hrit reader. I think the only way to not mask the data is to request counts: https://github.com/pytroll/satpy/blob/de969f554f3f69a5561c9159868979e27489c680/satpy/readers/seviri_l1b_nc.py#L140 https://github.com/pytroll/satpy/blob/de969f554f3f69a5561c9159868979e27489c680/satpy/readers/seviri_l1b_hrit.py#L680

[djhoese]

pre-commit.ci autofix

[djhoese]

pre-commit was complaining about the import order @BENR0 so I had it automatically fix it. You'll need to git pull before doing anything more to this PR to avoid rebase/merge issues. As far as whether or not this PR is good to go, I'm not sure I should be in charge of any SEVIRI-specific decisions.

[simonrp84]

I think it's important to have the masking as optional (vaguely remember this discussion in the past with the HRIT reader). But as both HRIT and NC don't do that then it's something for another issue / PR.

[BENR0]

@djhoese @mraspaud can this also be merged?

[mraspaud]

I think there are a couple of unresolved issue left @BENR0 , right?

[djhoese]

Looks like @sfinkens had some style concerns that he wanted fixed and @simonrp84 had some concerns about default behavior versus a kwarg.

[BENR0]

Since there were no other opinions raised after https://github.com/pytroll/satpy/pull/1693#discussion_r871303238 my take was that this is good to go code wise. I agree to leave the concern regarding the optional masking raised by @simonrp84 for another PR. This PR just makes sure that the hrit and nc reader do the same processing which I think is important from a user perspective.

[djhoese]

Sounds good with the Simon comment. The refactor comment: I said don't refactor it, but Martin agreed with Stephan so I got overruled. I think they would both like if you refactored that bit of code before they're OK with merging this.

[BENR0]

:-D Ok I see. I will have a look at it then and do some refactoring.

[BENR0]

pre-commit.ci autofix

[BENR0]

Just to check I'm reading the code correctly: Am I right in thinking that this masking is optional and is disabled by default?

@simonrp84 @sfinkens I also added the masking to be optional now.