Fix VIIRS EDR using the wrong geolocation arrays

djhoese commented 3 months ago

The VIIRS EDR CloudHeight files includes not only normal lon/lat arrays but a parallax corrected lon/lat. These parallax lon/lats don't include proper metadata (ex. _FillValue) so loading them causes fill values to remain like -999.9 instead of NaN. My previous code in this reader was rather naive and basically said if a variable name contained "longitude" or "latitude" then we should use it as the geolocation for our data arrays. This code was picking up the parallax corrected lon/lats instead of the "standard" ones. This PR is an attempt at making the reader smarter at choosing the right lon/lat arrays.

CC @kathys

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

codecov[bot] commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 95.94%. Comparing base (834f45d) to head (fc95ee1). Report is 332 commits behind head on main.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #2842 +/- ## ======================================= Coverage 95.94% 95.94% ======================================= Files 366 366 Lines 53561 53584 +23 ======================================= + Hits 51389 51412 +23 Misses 2172 2172 ``` | [Flag](https://app.codecov.io/gh/pytroll/satpy/pull/2842/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/2842/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pytroll) | `4.04% <0.00%> (-0.01%)` | :arrow_down: | | [unittests](https://app.codecov.io/gh/pytroll/satpy/pull/2842/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pytroll) | `96.04% <100.00%> (+<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 3 months ago

Pull Request Test Coverage Report for Build 9700068663

Details

27 of 27 (100.0%) changed or added relevant lines in 2 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.008%) to 96.045%

Totals
Change from base Build 9687731633:	0.008%
Covered Lines:	51634
Relevant Lines:	53760

💛 - Coveralls

simonrp84 commented 3 months ago

Is there a way for the user to select which one is used? I can think of many circumstances in which the parallax corrected versions would be preferred (despite the metadata issues).

djhoese commented 3 months ago

@simonrp84 At the moment, no. It would make sense, but maybe we save that for another PR if someone really wants it. Right now, besides this cloud height file, there are no other lon/lats available in the EDR files that I'm familiar with. We could either do a kwarg-style choice for users or there is (technically) the ability to customize the YAML right now and describe all the products you want and their specific lon/lats.

djhoese commented 3 months ago

I got the "good enough" signal from Panu on slack. Merging...thanks for the reviews.

coveralls commented 3 months ago

Pull Request Test Coverage Report for Build 9702660698

Details

45 of 45 (100.0%) changed or added relevant lines in 2 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.009%) to 96.046%

Totals
Change from base Build 9687731633:	0.009%
Covered Lines:	51641
Relevant Lines:	53767

pytroll / satpy