Skip to content

Fix mws_l1b_nc reader not scaling and masking properly - #3449

Merged
djhoese merged 1 commit into
pytroll:mainfrom
djhoese:fix-mws-l1b-calibration
Aug 31, 2026
Merged

Fix mws_l1b_nc reader not scaling and masking properly#3449
djhoese merged 1 commit into
pytroll:mainfrom
djhoese:fix-mws-l1b-calibration

Conversation

@djhoese

@djhoese djhoese commented Aug 24, 2026

Copy link
Copy Markdown
Member

Also lowercases sensor name to 'mws'.

Claude did almost all the coding on this one, but all code reviewed by me.

  • Closes #xxxx
  • Tests added
  • Fully documented
  • Add your name to AUTHORS.md if not there already

Also lowercases sensor name to 'mws'
@codecov

codecov Bot commented Aug 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.21429% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 96.34%. Comparing base (d2ee98f) to head (cc06db5).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
satpy/readers/mws_l1b.py 97.36% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3449      +/-   ##
==========================================
- Coverage   96.34%   96.34%   -0.01%     
==========================================
  Files         465      465              
  Lines       59151    59177      +26     
==========================================
+ Hits        56990    57015      +25     
- Misses       2161     2162       +1     
Flag Coverage Δ
behaviourtests 3.58% <0.00%> (-0.01%) ⬇️
unittests 96.43% <98.21%> (-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.

@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.

Verified locally (head cc06db5, base ce37614)

Regression-proven: the PR's rewritten test file (with PACKING_ATTRIBUTES import removed for base compatibility) yields 4 failures on basetest_sensor (MWS vs mws), test_get_dataset_get_channeldata_counts, test_get_dataset_get_channeldata_bts, test_get_global_attributes — and 24/24 pass on head. All 4 base failures trace to the intended behavior changes, not unrelated breakage.

Core bug confirmed in the old code:

  • _get_dataset_channel (old) applied only valid-range masking and NEVER applied scale_factor/add_offset — brightness temperatures came back as raw packed counts. New mask_and_scale() masks fill + valid range BEFORE scaling, casts to float32, applies scale/offset, and strips the packing attrs (they no longer describe the scaled array). Correct netCDF packing semantics.
  • _get_dataset_aux_data (old) only scaled when BOTH scale_factor AND add_offset existed, and did variable.attrs["missing_value"] — a hard KeyError when the attr is absent. New path is robust to missing attrs (falls back to netCDF default_fillvals and ±inf range).
  • YAML: mws_lon file_key pointed at data/navigation/mws_lat (copy-paste) — fixed to mws_lon. Real data bug.
  • sensor now lowercases /attr/instrument, matching the reader config sensors: [mws] and satpy's case-sensitive supports_sensor set intersection.

Edge-checked: counts path keeps integer dtype + _FillValue attr (dtype-typed fill); float path NaN-fills; fill-vs-range precedence (FillValue → _FillValue → missing_value) sensible; xr.set_options(keep_attrs=True) preserves attrs through where/astype.

No blocking issues. Clean, well-tested fix.

@djhoese

djhoese commented Aug 30, 2026

Copy link
Copy Markdown
Member Author

@Manny7717 Who are you? I'm sorry if I missed some conversation about this. Did someone ask you for these AI reviews?

@mraspaud mraspaud left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks an lot for the simplifications!

@djhoese
djhoese merged commit 7e3862b into pytroll:main Aug 31, 2026
18 checks passed
@djhoese
djhoese deleted the fix-mws-l1b-calibration branch August 31, 2026 15:42
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.

3 participants