Skip to content

Fix conversion script for v10 of METimage SRF - #295

Open
ninahakansson wants to merge 1 commit into
pytroll:mainfrom
ninahakansson:metimage_srf_v10
Open

ninahakansson wants to merge 1 commit into
pytroll:mainfrom
ninahakansson:metimage_srf_v10

Conversation

@ninahakansson

Copy link
Copy Markdown
Contributor

Fix conversion script for v10 of METimage SRF, EPS-SGA1-METIMAGE_SRF_V10.nc

  • Closes #xxxx
  • Tests added
  • Tests passed: Passes pytest pyspectral
  • Passes flake8 pyspectral
  • Fully documented
  • Add your name to AUTHORS.md if not there already

@codecov

codecov Bot commented Aug 31, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.17%. Comparing base (7a61fdf) to head (0c26413).
⚠️ Report is 16 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #295   +/-   ##
=======================================
  Coverage   91.17%   91.17%           
=======================================
  Files          25       25           
  Lines        2673     2674    +1     
=======================================
+ Hits         2437     2438    +1     
  Misses        236      236           
Flag Coverage Δ
unittests 91.17% <ø> (+<0.01%) ⬆️

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

☔ View full report in Codecov by Harness.
📢 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.

@djhoese

djhoese commented Aug 31, 2026

Copy link
Copy Markdown
Member

Are the values in the SRFs the same and this is just a group name change?

@Manny7717 Manny7717 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Quick verification notes on head 0c26413:

Verified locally: band-name parsing is sound - the script's band keys (e.g. vii_443, vii_865) parse via int(band.split("_")[-1]) to wavelengths in nm, and the three thresholds (wv>6000 -> LVWIR, >1000 -> SMWIR, else VISNIR) map every METimage band to the right family (probed all current BANDNAMES entries). The Instrument/ISRF/<family> paths are consistent with the v10 file's nested-group layout.

One compatibility question: the group-name change is unconditional, but the module docstring (lines 1-9) and rsr_convert_scripts/README.rst:201 still reference the plain EPS-SGA1_METIMAGE_SRF.nc (non-v10) file. If any users are still converting the older file (flat LVWIR/SMWIR/VISNIR groups), this change would break their conversion. Options: bump the docstring/README to point at the v10 file explicitly, or make the lookup tolerant (try nested path first, fall back to flat). Could not fetch the EUMETSAT file to double-check the exact v10 group names (anonymous sftp 404), so flagging rather than assuming.

Non-blocking - the fix itself looks correct for v10.

@adybbroe

adybbroe commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

@djhoese SRFs have been improved. Especially the IR channels differ. Cannot really see any changes in the VIS/NIR bands. I faked @ninahakansson 's new SRFs as Metop-SG-A4 for the convenience of plotting them via:
python bin/composite_rsr_plot.py --platform_name Metop-SG-A1 Metop-SG-A4 --sensor metimage -r 8 14.0
rsr_band_0800_1400

@adybbroe
adybbroe self-requested a review September 1, 2026 10:08

@adybbroe adybbroe left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for updating the SRFs to the latest from EUMETSAT!
LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants