8000 Updated the grid size to read claas3 data by jequierz · Pull Request #3084 · pytroll/satpy · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Updated the grid size to read claas3 data #3084

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
May 16, 2025

Conversation

jequierz
Copy link
I have updated the cmsaf_claas2 reader so that it handles now the larger grid of the claas3 (changed from 3636x3636 to 3712x3712).
  • Closes #xxxx
  • Tests added
  • Fully documented
  • Add your name to AUTHORS.md if not there already

@sfinkens
Copy link
Member

Tests fail, because the dummy file we use for testing doesn't have coordinates. That's actually a valuable feedback, because we can make the check a little simpler:

self.grid_size = self.file_content["/dimension/y"]

Then the tests will fail again, because the area definition from the (2x2) test file doesn't match the (3636x3636) expectation any more. Let me know when you are at this point, then we can continue together.

@sfinkens
Copy link
Member

Here's a proposal for an updated test that runs for two time stamps and two grid sizes

class TestAreaDef:
    @pytest.fixture(params=[3636, 3712])
    def grid_size(self, request):
        return request.param

    @pytest.fixture
    def fake_file(self, fake_dataset, encoding, tmp_path, start_time, grid_size):
        """Write a fake dataset to file.

        Only add y dimension, so that the file is small but we can still check the
        real-life area definition.
        """
        filename = tmp_path / "CPPin20140101001500305SVMSG01MD.nc"
        ds = netCDF4.Dataset(filename, mode="w")
        ds.createDimension("y", grid_size)
        ds.setncattr("time_coverage_start", start_time.strftime("%Y%m%dT%H%MZ"))
        ds.close()
        return filename

    @pytest.fixture
    def file_handler(self, fake_file):
        """Return a CLAAS-2 file handler."""
        from satpy.readers.cmsaf_claas2 import CLAAS2
        return CLAAS2(fake_file, {}, {})

    @pytest.fixture
    def area_extent_exp(self, start_time, grid_size):
        """Get expected area extent."""
        if start_time < datetime.datetime(2017, 12, 6):
            extents = {
                3636: (-5454733.160460291, -5454733.160460292, 5454733.160460292, 5454733.160460291),
                3712: (-5568748.485046371, -5568748.485046371, 5568748.485046371, 5568748.485046371)
            }
        else:
            extents = {
                3636: (-5456233.362099582, -5453232.958821001, 5453232.958821001, 5456233.362099582),
                3712: (-5570248.686685662, -5567248.28340708, 5567248.28340708, 5570248.686685662)
            }
        return extents[grid_size]

    @pytest.fixture
    def area_exp(self, area_extent_exp, grid_size):
        """Get expected area definition."""
        proj_dict = {
            "a": 6378169.0,
            "b": 6356583.8,
            "h": 35785831.0,
            "lon_0": 0.0,
            "proj": "geos",
            "units": "m",
        }
        return AreaDefinition(
            area_id="msg_seviri_fes_3km",
            description="MSG SEVIRI Full Earth Scanning service area definition with 3 km resolution",
            proj_id="geos",
            projection=proj_dict,
            area_extent=area_extent_exp,
            width=grid_size,
            height=grid_size,
        )

    def test_get_area_def(self, file_handler, area_exp):
        """Test area definition."""
        area = file_handler.get_area_def(make_dataid(name="foo"))
        assert area == area_exp

"""Test end time property."""
assert file_handler.end_time == datetime.datetime(2085, 8, 13, 13, 15)

class TestAreaDef:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you just need to add a docstring for this class to make ruff happy, see

https://results.pre-commit.ci/run/github/51397392/1744803645.EteACOctTJiZK_OgSV2njA

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, more docstring problems reported by ruff. Try installing pre-commit hooks in your working copy

pip install ruff pre-commit
pre-commit install
ruff check test_cmsaf_claas.py

Copy link
codecov bot commented Apr 22, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.22%. Comparing base (dc6987c) to head (5faf374).
Report is 25 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3084   +/-   ##
=======================================
  Coverage   96.22%   96.22%           
=======================================
  Files         398      398           
  Lines       57330    57350   +20     
=======================================
+ Hits        55165    55185   +20     
  Misses       2165     2165           
Flag Coverage Δ
behaviourtests 3.87% <0.00%> (-0.01%) ⬇️
unittests 96.31% <100.00%> (+<0.01%) ⬆️

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.

🚀 New feature 8000 s to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jequierz jequierz force-pushed the update_cmsaf_reader branch from 4d18462 to c6dc906 Compare May 15, 2025 10:02
@pnuu pnuu added enhancement code enhancements, features, improvements component:readers labels May 15, 2025
@pnuu pnuu moved this to Ready for review in PCW Spring 2025 May 15, 2025
@pnuu pnuu marked this pull request as ready for review May 15, 2025 16:27
@pnuu pnuu requested review from djhoese and mraspaud as code owners May 15, 2025 16:27
@pnuu
Copy link
Member
pnuu commented May 15, 2025

pre-commit.ci autofix

Copy link
Member
@pnuu pnuu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These updates seem to be fine, just needs the one docstring that I added a suggestion to.

The tests are failing though: https://github.com/pytroll/satpy/actions/runs/15050403317/job/42303560714?pr=3084#step:9:201

We can have a debugging session tomorrow to see what's going on.

class TestAreaDef:
"""Test the area of both CLAAS2 and CLAAS3 products"""
@pytest.fixture(params=[3636, 3712])
def grid_size(self, request):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def grid_size(self, request):
def grid_size(self, request):
"""Return the grid size."""

Docstring needed.

filename = tmp_path / "CPPin20140101001500305SVMSG01MD.nc"
ds = netCDF4.Dataset(filename, mode="w")
ds.createDimension("y", grid_size)
ds.setncattr("time_coverage_start", start_time.strftime("%Y%m%dT%H%MZ"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used the wrong time format here, this should fix the problem:

Suggested change
ds.setncattr("time_coverage_start", start_time.strftime("%Y%m%dT%H%MZ"))
ds.setncattr("time_coverage_start", start_time.strftime(""%Y-%m-%dT%H:%M:%SZ""))

(fromisoformat doesn't support all iso formats in Python-3.10; and this is the format in the CLAAS files)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah! We found out that independently on our debugging session with @jequierz 😁

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was also weird that the test passed with Python 3.12 locally. I guess the NetCDF4 library has added some datetime handling to newer versions to make it work even with the "wrong" time string.

@coveralls
Copy link
coveralls commented May 16, 2025

Pull Request Test Coverage Report for Build 15064229552

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.

Details

  • 35 of 35 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.001%) to 96.327%

Totals Coverage Status
Change from base Build 15005538775: 0.001%
Covered Lines: 55445
Relevant Lines: 57559

💛 - Coveralls

@pnuu pnuu merged commit ee03d8c into pytroll:main May 16, 2025
17 of 18 checks passed
@github-project-automation github-project-automation bot moved this from Ready for review to Done in PCW Spring 2025 May 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:readers enhancement code enhancements, features, improvements
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 3810 participants
0