-
Notifications
You must be signed in to change notification settings - Fork 312
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
Conversation
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:
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. |
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: |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
Codecov ReportAll modified and coverable lines are covered by tests ✅
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
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New feature 8000 s to boost your workflow:
|
4d18462
to
c6dc906
Compare
pre-commit.ci autofix |
for more information, see https://pre-commit.ci
There was a problem hiding this 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): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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")) |
There was a problem hiding this comment.
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:
ds.setncattr("time_coverage_start", start_time.strftime("%Y%m%dT%H%MZ")) | |
ds.setncattr("time_coverage_start", start_time.strftime(""%Y-%m-%dT% |
(fromisoformat
doesn't support all iso formats in Python-3.10; and this is the format in the CLAAS files)
There was a problem hiding this comment.
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 😁
There was a problem hiding this comment.
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.
Pull Request Test Coverage Report for Build 15064229552Warning: 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
💛 - Coveralls |
AUTHORS.md
if not there already