Add a reader for NWC SAF GEO HRW data #3070

pnuu · 2025-02-21T13:26:32Z

This PR adds a reader for the High Resolution Winds data from NWC SAF GEO.

The data structure is very complex, and due to the unsupported compound data type can't be opened with xr.open_dataset(). Because there are 259 datasets, I've made the dataset definitions dynamic instead of putting them into the reader YAML. The code is in a separate file because the internal structure is completely different to the other NWC SAF GEO products (see the linked issue).

By default the file handler reads the datasets separately for each imaging channel. That is, the datasets are named wind_vis06_air_pressure, wind_hrvis_wind_speed, and so on. The prefix is the name of the channel within the files.

The user can also supply reader_kwargs={"merge_channels": True} to collect all the data together. In this case the datasets are named without the prefix, such as air_pressure, wind_speed, etc.

Closes Reader for NWC SAF HRW (high resolution winds) data #3052
Tests added
Fully documented

codecov · 2025-02-21T13:34:26Z

Codecov Report

Attention: Patch coverage is 97.73756% with 5 lines in your changes missing coverage. Please review.

Project coverage is 96.15%. Comparing base (128bb9e) to head (6e690d9).
Report is 47 commits behind head on main.

Files with missing lines	Patch %	Lines
satpy/readers/nwcsaf_hrw_nc.py	96.00%	5 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##             main    #3070    +/-   ##
========================================
  Coverage   96.14%   96.15%            
========================================
  Files         383      385     +2     
  Lines       55798    56021   +223     
========================================
+ Hits        53649    53867   +218     
- Misses       2149     2154     +5

Flag	Coverage Δ
behaviourtests	`3.86% <0.00%> (-0.02%)`	⬇️
unittests	`96.24% <97.73%> (+<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 features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

pnuu · 2025-02-24T06:46:46Z

I reduced the number of data rows that were written to the test data. With the original 1234 data points it took 19 seconds to run the tests on my laptop. Increasing to 12345 the tests took 170 seconds and with the current 123 only 2 seconds.

pnuu · 2025-02-24T08:30:59Z

I'll have a look at adding a kwarg to merge the different channel observations, so the user could do something like

scn = Scene(reader="nwcsaf-geo", filenames=filenames, reader_kwargs={"merge_channels": True})
scn.load(["wind_speed", "wind_from_direction"])

instead of loading each of the channels (wind_vis06_wind_speed, wind_vis08_wind_speed, etc.) separately.

pnuu · 2025-02-24T12:55:23Z

Also some documentation added.

coveralls · 2025-02-27T10:53:20Z

Pull Request Test Coverage Report for Build 13971092899

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.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

218 of 223 (97.76%) changed or added relevant lines in 2 files are covered.
23 unchanged lines in 1 file lost coverage.
Overall coverage increased (+0.006%) to 96.259%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
satpy/readers/nwcsaf_hrw_nc.py	121	126	96.03%

Files with Coverage Reduction	New Missed Lines	%
satpy/writers/init.py	23	90.91%

Totals
Change from base Build 13624409281:	0.006%
Covered Lines:	54116
Relevant Lines:	56219

💛 - Coveralls

djhoese · 2025-03-17T18:13:08Z

satpy/tests/reader_tests/test_nwcsaf_hrw_nc.py

+FILETYPE_INFO = {"file_type": "nc_nwcsaf_geo_hrw"}
+
+
+@pytest.fixture


Suggested change

@pytest.fixture

@pytest.fixture(scope="module")

Unless I misunderstand the fixture, this should make it so it is only created once for all of this module's tests.

I agree with the scope, but this will break tmp_path, you need to use tmp_path_factory instead https://docs.pytest.org/en/stable/how-to/tmp_path.html#the-tmp-path-factory-fixture

Adjusted in ba584f3

djhoese

I had a suggestion or two, but I really don't think I should have final say on this as I have no experience with nwcsaf readers. I'm marking my review as approve even though I requested a few things. Not doing my suggestions does not mean this file handler is broken or that it can't be merged, but it isn't as good as it could be 😉

djhoese · 2025-03-17T18:15:02Z

satpy/readers/nwcsaf_hrw_nc.py

+        with suppress(OSError):
+            self.h5f.close()
+
+    def available_datasets(self, configured_datasets=None):


The configured_datasets are not "forwarded on" as suggested in the base file handler:

satpy/satpy/readers/file_handlers.py

Lines 256 to 263 in f915074

for is_avail, ds_info in (configured_datasets or []):

if is_avail is not None:

# some other file handler said it has this dataset

# we don't know any more information than the previous

# file handler so let's yield early

yield is_avail, ds_info

continue

yield self.file_type_matches(ds_info["file_type"]), ds_info

Without this users will not be able to statically define datasets in the YAML.

So what to change?

mraspaud

Just a couple of small things, but otherwise LGTM

mraspaud · 2025-03-18T12:16:08Z

satpy/readers/nwcsaf_hrw_nc.py

+    scn = Scene(reader="nwcsaf-geo", filenames=[filename])
+    pprint.pprint(scn.available_dataset_names())
+
+This print all the available datasets. The truncated output of this is::


Suggested change

This print all the available datasets. The truncated output of this is::

This prints all the available datasets. The truncated output of this is::

Fixed in 6e690d9

mraspaud · 2025-03-18T12:20:30Z

satpy/tests/reader_tests/test_nwcsaf_hrw_nc.py

+FILETYPE_INFO = {"file_type": "nc_nwcsaf_geo_hrw"}
+
+
+@pytest.fixture


I agree with the scope, but this will break tmp_path, you need to use tmp_path_factory instead https://docs.pytest.org/en/stable/how-to/tmp_path.html#the-tmp-path-factory-fixture

mraspaud · 2025-03-18T12:24:29Z

satpy/readers/nwcsaf_hrw_nc.py

+            except ValueError:
+                logger.warning("Reading %s is not supported.", dataset_name)


I would have thought this raises a KeyError...

It raises ValueError when the data are there but the compound datatype is unreadable.

mraspaud

LGTM

Add a reader for NWC SAF GEO HRW data

9108d3a

pnuu added enhancement code enhancements, features, improvements component:readers labels Feb 21, 2025

pnuu self-assigned this Feb 21, 2025

pnuu requested review from adybbroe, djhoese and mraspaud as code owners February 21, 2025 13:26

lahtinep added 4 commits February 21, 2025 20:41

Add HRW test file creation and file handler init test

5f4f381

Make read_dataset() private method

8008472

Add more HRW reader tests

c074c5f

Reduce data amount, check scn start/end times

44811e3

lahtinep added 2 commits February 24, 2025 11:39

Add dataset merging and refactor

1b16523

Add module documentation

b1fc1db

lahtinep added 5 commits February 24, 2025 15:46

Add units to datasets

8a57410

Refactor available_datasets()

32a0554

Refactor channel prefix determination

86cb81d

Refactor _create_xarray()

7348a34

Merge branch 'main' into feature-nwc-geo-hrw

b2c4dc9

Merge branch 'main' into feature-nwc-geo-hrw

9a9c0a7

djhoese reviewed Mar 17, 2025

View reviewed changes

djhoese approved these changes Mar 17, 2025

View reviewed changes

mraspaud reviewed Mar 18, 2025

View reviewed changes

lahtinep added 2 commits March 20, 2025 15:53

Make hrw_file fixture module-scoped

ba584f3

Fix a typo

6e690d9

mraspaud approved these changes Mar 21, 2025

View reviewed changes

mraspaud merged commit 1ded1dd into pytroll:main Mar 21, 2025
18 checks passed

pnuu deleted the feature-nwc-geo-hrw branch March 21, 2025 12:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a reader for NWC SAF GEO HRW data #3070

Add a reader for NWC SAF GEO HRW data #3070

pnuu commented Feb 21, 2025 •

edited

Loading

codecov bot commented Feb 21, 2025 •

edited

Loading

pnuu commented Feb 24, 2025

pnuu commented Feb 24, 2025

pnuu commented Feb 24, 2025

coveralls commented Feb 27, 2025 •

edited

Loading

djhoese Mar 17, 2025

mraspaud Mar 18, 2025

pnuu Mar 20, 2025

djhoese left a comment

djhoese Mar 17, 2025

pnuu Mar 20, 2025

mraspaud left a comment

mraspaud Mar 18, 2025

pnuu Mar 20, 2025

mraspaud Mar 18, 2025

mraspaud Mar 18, 2025

pnuu Mar 20, 2025

mraspaud left a comment

		FILETYPE_INFO = {"file_type": "nc_nwcsaf_geo_hrw"}


		@pytest.fixture

	for is_avail, ds_info in (configured_datasets or []):
	if is_avail is not None:
	# some other file handler said it has this dataset
	# we don't know any more information than the previous
	# file handler so let's yield early
	yield is_avail, ds_info
	continue
	yield self.file_type_matches(ds_info["file_type"]), ds_info

	This print all the available datasets. The truncated output of this is::
	This prints all the available datasets. The truncated output of this is::

		except ValueError:
		logger.warning("Reading %s is not supported.", dataset_name)

Add a reader for NWC SAF GEO HRW data #3070

Add a reader for NWC SAF GEO HRW data #3070

Conversation

pnuu commented Feb 21, 2025 • edited Loading

codecov bot commented Feb 21, 2025 • edited Loading

Codecov Report

pnuu commented Feb 24, 2025

pnuu commented Feb 24, 2025

pnuu commented Feb 24, 2025

coveralls commented Feb 27, 2025 • edited Loading

Pull Request Test Coverage Report for Build 13971092899

Warning: This coverage report may be inaccurate.

Details

💛 - Coveralls

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

djhoese left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mraspaud left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mraspaud left a comment

Choose a reason for hiding this comment

pnuu commented Feb 21, 2025 •

edited

Loading

codecov bot commented Feb 21, 2025 •

edited

Loading

coveralls commented Feb 27, 2025 •

edited

Loading