Handle valid range and valid_range correctly for two types of HY2 SCAT l2b hdf5 data #2211

TAlonglong · 2022-09-28T19:45:24Z

There was a mixture of where attributes were taken from, both data and self[key]
Adding keep_attrs when using xarray.where let us keep the attributes so we can use attributes from that instead of self[key]

Closes hy2_scat_l2b_h5 reader does not work any more due to space in valid range attribute #2208
Tests added
Fully documented
Add your name to AUTHORS.md if not there already

Add a corrected green composite for FCI and use this in the true color RGB. The problem is the same as for AHI, in that the "green" channel misses the chlorophyl peak making our green planet look rather brown as if all the forests have burnt :(

Move the GreenCorrector to the new module satpy.composites.spectral, as it applies to FCI as well as to AHI. Adapt AHI and FCI composites using this corrector. Improve documentation for the corrector.

Add the new file that was missing in the previous commit and that was causing test failures.

Some small fixes for the green corrector based on feedback from Dave.

…butes

… in xarray.where

TAlonglong · 2022-09-29T05:45:43Z

Hm, I don't understand why the tests are failing. When I run the tests locally, they pass.

pnuu · 2022-09-29T05:59:56Z

Often that would mean missing dependencies on CI, but can't see how that would be the case in this one if the tests have been passing before the PR 🤔

TAlonglong · 2022-09-29T14:29:45Z

Ah, ok. Setting up python 3.8 I get the test to fail. Now I have something to debug.

TAlonglong · 2022-09-29T14:55:15Z

No, it is not the python version but xarray version. upgrading to xarray 2022.6.0 pyhd8ed1ab_1 conda-forge
gives the error. Looks like the keeps_attrs is not respected in xarray.where https://docs.xarray.dev/en/stable/generated/xarray.where.html

Hm, do I use it wrong or is there a bug in xarray?

djhoese · 2022-09-29T15:27:07Z

In the docs it says it will take the attrs of the first array argument x. In both cases of your usage that DataArray doesn't have any .attrs I think:

data = xr.where(data > 180, data - 360., data, keep_attrs=True)

I think data - 360.0 resets the attrs, but I could be wrong.

In your other case:

data = xr.where(data < valid_range[0], np.nan, data, keep_attrs=True)

np.nan won't have any attrs either.

TAlonglong · 2022-09-29T15:47:31Z

Ah, thanks.

TAlonglong · 2022-09-29T16:11:48Z

Now the test for this change pass as far as I can see. But there is another test failing.

Hm what changed now?

djhoese · 2022-09-29T18:07:08Z

This is a known failure that I need to fix. Someone already added an atol in one of the other active PRs. It seems numpy or other precision has changed in recent versions and now one of the tests is being too strict.

TAlonglong · 2022-09-29T18:30:56Z

Yeah, that was my guess also. Should I add this in this PR? Or does it need to be in a separate PR?

Then I can at least make an issue?

djhoese · 2022-09-29T19:42:24Z

If this PR is straight forward and ready to be merged besides the failing test then I'm OK with you adding it here. I think 1e-4 is good enough, but I'd have to search for the other contributor who already fixed it. In another PR I saw yet another test failing, but I don't see that here so we'll have to see what happens.

TAlonglong · 2022-09-30T05:36:55Z

OK, I ran the reader tests locally, and a lot is failing, mainly the same issue:

============================================================================== short test summary info ==============================================================================
FAILED satpy/tests/reader_tests/test_seviri_l1b_calibration.py::TestSEVIRICalibrationAlgorithm::test_vis_calibrate - AssertionError: Left and right DataArray objects are not close
FAILED satpy/tests/reader_tests/test_seviri_l1b_hrit.py::TestHRITMSGCalibration::test_calibrate[VIS006-reflectance-NOMINAL-False] - AssertionError: Left and right DataArray objec...
FAILED satpy/tests/reader_tests/test_seviri_l1b_hrit.py::TestHRITMSGCalibration::test_calibrate[VIS006-reflectance-NOMINAL-True] - AssertionError: Left and right DataArray object...
FAILED satpy/tests/reader_tests/test_seviri_l1b_hrit.py::TestHRITMSGCalibration::test_calibrate[HRV-reflectance-NOMINAL-False] - AssertionError: Left and right DataArray objects ...
FAILED satpy/tests/reader_tests/test_seviri_l1b_hrit.py::TestHRITMSGCalibration::test_calibrate[HRV-reflectance-NOMINAL-True] - AssertionError: Left and right DataArray objects a...
FAILED satpy/tests/reader_tests/test_seviri_l1b_native.py::TestNativeMSGCalibration::test_calibrate[VIS006-reflectance-NOMINAL-False] - AssertionError: Left and right DataArray o...
FAILED satpy/tests/reader_tests/test_seviri_l1b_native.py::TestNativeMSGCalibration::test_calibrate[VIS006-reflectance-NOMINAL-True] - AssertionError: Left and right DataArray ob...
FAILED satpy/tests/reader_tests/test_seviri_l1b_native.py::TestNativeMSGCalibration::test_calibrate[HRV-reflectance-NOMINAL-False] - AssertionError: Left and right DataArray obje...
FAILED satpy/tests/reader_tests/test_seviri_l1b_native.py::TestNativeMSGCalibration::test_calibrate[HRV-reflectance-NOMINAL-True] - AssertionError: Left and right DataArray objec...
FAILED satpy/tests/reader_tests/test_seviri_l1b_nc.py::TestNCSEVIRIFileHandler::test_calibrate[VIS006-reflectance-False] - AssertionError: Left and right DataArray objects are no...
FAILED satpy/tests/reader_tests/test_seviri_l1b_nc.py::TestNCSEVIRIFileHandler::test_calibrate[VIS006-reflectance-True] - AssertionError: Left and right DataArray objects are not...
FAILED satpy/tests/reader_tests/test_seviri_l1b_nc.py::TestNCSEVIRIFileHandler::test_get_dataset[VIS006-reflectance] - AssertionError: Left and right DataArray objects are not close
FAILED satpy/tests/reader_tests/test_utils.py::TestSunEarthDistanceCorrection::test_apply_sunearth_corr - AssertionError: 
FAILED satpy/tests/reader_tests/test_utils.py::TestSunEarthDistanceCorrection::test_remove_sunearth_corr - AssertionError: 
FAILED satpy/tests/reader_tests/test_vii_l1b_nc.py::TestViiL1bNCFileHandler::test_calibration_functions - ValueError: ('The number of tie points in the along-route dimension must...
FAILED satpy/tests/reader_tests/test_vii_l1b_nc.py::TestViiL1bNCFileHandler::test_functions - ValueError: ('The number of tie points in the along-route dimension must be a multip...
============================================================== 16 failed, 874 passed, 269 warnings in 96.76s (0:01:36) ==============================================================

I think this is better cleaned up separately

mraspaud

Thanks for fixing this. Could you add a test for this?

Fix ici after mws merge

Update seadas_l2 reader to handle alternative NetCDF file format

…rods Fix CLAVR-x configuration in 'awips_tiled' writer to be backwards compatible

updates: - [github.com/pre-commit/mirrors-mypy: v0.981 → v0.982](pre-commit/mirrors-mypy@v0.981...v0.982)

TAlonglong · 2022-10-11T08:34:16Z

So, I have come up with an idea to, what to call it, override __getitem__ for the class FakeHDF5FileHandler2 in test_hy2_scat_l2b_h5.py like this:

class FakeHDF5FileHandler2(FakeHDF5FileHandler):
    """Swap-in HDF5 File Handler."""

    def __getitem__(self, key):
        # https://stackoverflow.com/questions/2390827/how-to-properly-subclass-dict-and-override-getitem-setitem
        val = self.file_content.__getitem__(key)
        if isinstance(val, xr.core.dataarray.DataArray):
            val = val.copy()
        return val

By doing this the original test fails for none NSOAS data as they are expected to do.

I hope I can do this and adding my fix to the code as already in the PR.
Any comments on this?

[pre-commit.ci] pre-commit autoupdate

djhoese · 2022-10-11T13:53:59Z

I'm still not sure what the original issue is but if you think this helps then try it out and let's see how it goes. FYI val = self.file_content.__getitem__(key) is the same as val = self.file_content[key].

… in xarray.where

…longlong/satpy into issue2208-hy2-scat-l2b-h5-valid-range

codecov · 2022-10-11T14:23:40Z

Codecov Report

Merging #2211 (4f269d7) into main (8c1ccdd) will increase coverage by 0.01%.
The diff coverage is 98.41%.

@@            Coverage Diff             @@
##             main    #2211      +/-   ##
==========================================
+ Coverage   94.13%   94.15%   +0.01%     
==========================================
  Files         293      294       +1     
  Lines       45079    45203     +124     
==========================================
+ Hits        42437    42559     +122     
- Misses       2642     2644       +2

Flag	Coverage Δ
behaviourtests	`4.67% <0.00%> (-0.02%)`	⬇️
unittests	`94.80% <98.41%> (+<0.01%)`	⬆️

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

Impacted Files	Coverage Δ
satpy/tests/test_utils.py	`100.00% <ø> (ø)`
satpy/readers/viirs_edr_active_fires.py	`91.93% <50.00%> (ø)`
satpy/readers/seadas_l2.py	`96.84% <96.66%> (-1.28%)`	⬇️
satpy/composites/ahi.py	`100.00% <100.00%> (ø)`
satpy/composites/spectral.py	`100.00% <100.00%> (ø)`
satpy/enhancements/viirs.py	`100.00% <100.00%> (ø)`
satpy/readers/abi_base.py	`94.52% <100.00%> (+0.07%)`	⬆️
satpy/readers/hy2_scat_l2b_h5.py	`98.33% <100.00%> (+0.11%)`	⬆️
satpy/scene.py	`93.37% <100.00%> (+0.01%)`	⬆️
satpy/tests/reader_tests/test_abi_l2_nc.py	`100.00% <100.00%> (ø)`
... and 5 more

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

coveralls · 2022-10-11T14:37:53Z

Coverage increased (+0.01%) to 94.75% when pulling 4f269d7 on TAlonglong:issue2208-hy2-scat-l2b-h5-valid-range into 8c1ccdd on pytroll:main.

TAlonglong · 2022-10-11T15:42:15Z

pre-commit.ci autofix

for more information, see https://pre-commit.ci

mraspaud · 2022-11-09T15:29:38Z

@TAlonglong could you try to clean up the history here? there are a lot of old commits hanging around, and that makes is particularly difficult to review

TAlonglong · 2022-11-10T07:11:29Z

yeah, I think I did some merge from the main or rebase that messed this up. I never get these things right.

I don't understand how I could clean this up. I will need to have a look around to see if I can figure something out.

djhoese · 2022-11-10T15:40:23Z

I would suggest maybe creating a new branch off of current main, then doing git cherry-pick <commit SHA> for each one of your actual commits in this PR. If you do them in order then it might not be too bad.

TAlonglong · 2022-11-11T06:52:29Z

Okay, so I partly did what David suggested. But there was mess of try and fail code, so cherry picking became difficult. So I ended up to create a diff patch and apply that to a new branch #2268

Please feel free to close this PR if that work out.

mraspaud · 2022-11-11T07:52:32Z

I merged #2268, so closing this

gerritholl and others added 12 commits August 16, 2022 17:05

First attempt at true color FCI RGB

5b3ad0b

Move green corrector to new module

06231e9

Move the GreenCorrector to the new module satpy.composites.spectral, as it applies to FCI as well as to AHI. Adapt AHI and FCI composites using this corrector. Improve documentation for the corrector.

Improve docs for FCI true color composite

4cf470e

Add new module that was missing in previous commit

cd132f9

Add the new file that was missing in the previous commit and that was causing test failures.

try corrected green raw?

be84ed4

Update URL to rasterio repository in CI

a34de8d

Small fixes based on Dave feedback

d053f22

Some small fixes for the green corrector based on feedback from Dave.

Update seadas_l2 reader to handle alternative NetCDF file format

43d6bba

Fix fine/coarse area checks for 1D SwathDefinitions

6e53c6d

Fix VIIRS EDR Active Fires reader using bad sensor and platform attri…

3b1c58f

…butes

Use the attributes of data when this has changed. Use keep_attrs=True…

6ec5b8c

… in xarray.where

TAlonglong requested review from djhoese and mraspaud as code owners September 28, 2022 19:45

Use keep_attrs=True in xarray.where

8e667e6

pnuu assigned TAlonglong Sep 29, 2022

pnuu added the cleanup Code cleanup but otherwise no change in functionality label Sep 29, 2022

need to store attrs from data and update data.attrs after xarray.where

2fe859b

mraspaud reviewed Sep 30, 2022

View reviewed changes

mraspaud and others added 4 commits October 6, 2022 13:21

Merge pull request pytroll#2221 from adybbroe/fix-ici-after-mws-merge

dbf9929

Fix ici after mws merge

Merge pull request pytroll#2209 from djhoese/feature-seadas-nc

6d24db1

Update seadas_l2 reader to handle alternative NetCDF file format

Merge pull request pytroll#2220 from djhoese/bugfix-clavrx-awips-catp…

15be5c1

…rods Fix CLAVR-x configuration in 'awips_tiled' writer to be backwards compatible

[pre-commit.ci] pre-commit autoupdate

25740d7

updates: - [github.com/pre-commit/mirrors-mypy: v0.981 → v0.982](pre-commit/mirrors-mypy@v0.981...v0.982)

Merge pull request pytroll#2231 from pytroll/pre-commit-ci-update-config

5cc09e0

[pre-commit.ci] pre-commit autoupdate

Trygve Aspenes added 5 commits October 11, 2022 16:00

Use the attributes of data when this has changed. Use keep_attrs=True…

2d95747

… in xarray.where

Use keep_attrs=True in xarray.where

f9fd9cd

need to store attrs from data and update data.attrs after xarray.where

7f49a6d

Merge branch 'issue2208-hy2-scat-l2b-h5-valid-range' of github.com:TA…

2169af9

…longlong/satpy into issue2208-hy2-scat-l2b-h5-valid-range

test with copy of xarray datasets

1461f20

TAlonglong requested a review from adybbroe as a code owner October 11, 2022 14:04

Trygve Aspenes added 2 commits October 11, 2022 16:08

docstring

df70a54

docstring

2218d68

add properties test

a3e4f48

[pre-commit.ci] auto fixes from pre-commit.com hooks

4f269d7

for more information, see https://pre-commit.ci

TAlonglong requested review from mraspaud and removed request for adybbroe and djhoese October 12, 2022 06:14

TAlonglong mentioned this pull request Nov 10, 2022

Cleanup hy2 reader #2268

Merged

3 tasks

mraspaud closed this Nov 11, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handle valid range and valid_range correctly for two types of HY2 SCAT l2b hdf5 data #2211

Handle valid range and valid_range correctly for two types of HY2 SCAT l2b hdf5 data #2211

TAlonglong commented Sep 28, 2022 •

edited

Loading

TAlonglong commented Sep 29, 2022

pnuu commented Sep 29, 2022

TAlonglong commented Sep 29, 2022

TAlonglong commented Sep 29, 2022

djhoese commented Sep 29, 2022

TAlonglong commented Sep 29, 2022

TAlonglong commented Sep 29, 2022

djhoese commented Sep 29, 2022

TAlonglong commented Sep 29, 2022

djhoese commented Sep 29, 2022

TAlonglong commented Sep 30, 2022

mraspaud left a comment

TAlonglong commented Oct 11, 2022 •

edited

Loading

djhoese commented Oct 11, 2022

codecov bot commented Oct 11, 2022 •

edited

Loading

coveralls commented Oct 11, 2022 •

edited

Loading

TAlonglong commented Oct 11, 2022

mraspaud commented Nov 9, 2022

TAlonglong commented Nov 10, 2022

djhoese commented Nov 10, 2022

TAlonglong commented Nov 11, 2022

mraspaud commented Nov 11, 2022

Handle valid range and valid_range correctly for two types of HY2 SCAT l2b hdf5 data #2211

Handle valid range and valid_range correctly for two types of HY2 SCAT l2b hdf5 data #2211

Conversation

TAlonglong commented Sep 28, 2022 • edited Loading

TAlonglong commented Sep 29, 2022

pnuu commented Sep 29, 2022

TAlonglong commented Sep 29, 2022

TAlonglong commented Sep 29, 2022

djhoese commented Sep 29, 2022

TAlonglong commented Sep 29, 2022

TAlonglong commented Sep 29, 2022

djhoese commented Sep 29, 2022

TAlonglong commented Sep 29, 2022

djhoese commented Sep 29, 2022

TAlonglong commented Sep 30, 2022

mraspaud left a comment

Choose a reason for hiding this comment

TAlonglong commented Oct 11, 2022 • edited Loading

djhoese commented Oct 11, 2022

codecov bot commented Oct 11, 2022 • edited Loading

Codecov Report

coveralls commented Oct 11, 2022 • edited Loading

TAlonglong commented Oct 11, 2022

mraspaud commented Nov 9, 2022

TAlonglong commented Nov 10, 2022

djhoese commented Nov 10, 2022

TAlonglong commented Nov 11, 2022

mraspaud commented Nov 11, 2022

TAlonglong commented Sep 28, 2022 •

edited

Loading

TAlonglong commented Oct 11, 2022 •

edited

Loading

codecov bot commented Oct 11, 2022 •

edited

Loading

coveralls commented Oct 11, 2022 •

edited

Loading