[syedhamidali]

• Tests added
• Changes are documented in history.md

The NEXRAD Level 2 reader had two gaps that downstream processing (e.g. velocity dealiasing, QC) had to work around by reading the raw headers:

1. Nyquist velocity / unambiguous range were not exposed. Both are in the MSG_31 radial data block (RAD, ICD 2620002 Table XVII-H, scaled by 0.01 m s-1 / 0.1 km) and in the MSG_1 header for legacy files.
   Each sweep now carries scalar nyquist_velocity (m s-1, get_nyquist_velocity_attrs, same as the ODIM/GAMIC readers from add nyquist_velocity as coordinate in ODIM H5/GAMIC reader #291) and unambiguous_range (meters, new get_unambiguous_range_attrs in xradar.model). Both names are already listed in optional_sweep_metadata_vars (CfRadial2/FM301). Surveillance-only MSG_1 cuts store zero, which is returned as NaN.
   Example (KATX20130717_195021_V06): sweep_0 (CS) 8.81 m s-1 / 466 km, sweep_1 (CD) 35.09 m s-1 / 117 km.

2. Special moment codes were decoded as data. Per ICD Table XVII-I, raw 0 = below threshold and 1 = range folded for all moments, so they showed up as e.g. DBZH −33/−32.5 dBZ, VRADH −64.5/−64 m s-1, ZDR −8 dB, RHOHV 0.20.
   The moments now get _FillValue = 0. Because xarray can only round-trip a single fill value cleanly (several values in missing_value warn on every open and conflict with _FillValue on encode), the array wrapper maps range folded (1) to below threshold (0) when masking is active, so both decode to NaN while the data stay lazy. Padding of shorter rays also uses the fill code.
   With mask_and_scale=False (globally or per variable, e.g. mask_and_scale={"VRADH": False}) the raw codes are returned unchanged, so range folded gates stay distinguishable (raw.VRADH == 1). This is documented in the docstring and in docs/importers.md.

Export: the new _FillValue encoding round-trips through to_cfradial2 and to_cfradial1 (NaN is written as 0, values identical after reopening).

Open points for review

• Range folded masked by default. I merged range folded into the mask so that the default output has no spurious values. An alternative would be to keep range folded as a separate flag (e.g. an extra variable or ODIM-like _Undetect handling), which needs a second fill value. Happy to change this if you prefer that.
• CCORH (CFP, clutter filter power removed) has more special codes (2–7 in the ICD). Only 0 and 1 are masked here, as for all other moments.
• to_cfradial2 currently does not write nyquist_velocity / unambiguous_range to the sweep groups. Not changed here.
• Sweep data variable counts go up by two (nyquist_velocity, unambiguous_range); the two existing tests checking the count are updated.

Tests

• New tests on KATX20130717_195021_V06 (MSG_31) and KLIX20050828_180149 (MSG_1) from open-radar-data: Nyquist/unambiguous range values (checked against the raw RAD block), masking for DBZH/VRADH/WRADH (8 bit) and ZDR/RHOHV/PHIDP (16 bit), raw codes with mask_and_scale=False, per-variable mask_and_scale mapping.
• pytest tests: 686 passed. Pre-commit: all hooks pass except zizmor, which reports the same findings on main.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.03%. Comparing base (6394632) to head (8638b0e).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #457      +/-   ##
==========================================
+ Coverage   96.02%   96.03%   +0.01%     
==========================================
  Files          30       30              
  Lines        7472     7494      +22     
==========================================
+ Hits         7175     7197      +22     
  Misses        297      297              

Flag	Coverage Δ
notebooktests	82.81% <96.66%> (+0.01%)	⬆️
unittests	95.87% <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 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.

[kmuehlbauer]

Great, my brain failed me. So I always was under the impression that in the appending changelog case this will go without creating merge conflicts, but alas... 🤷 Nevermind @syedhamidali, but maybe adding to the end is still preferable.

[syedhamidali]

@kmuehlbauer yes, git sees two PRs adding a line at the same place as a conflict, wherever that place is. I tried the options in a small repo:

approach	two PRs adding an entry
append at the end of the section	conflict
.gitattributes: docs/history.md merge=union	merges cleanly with local git (keeps both lines), but GitHub ignores merge drivers when checking/merging PRs, so the PR still shows as conflicting (GitLab has the same limitation)
one fragment file per PR	merges cleanly, also on GitHub

So what actually works is changelog fragments, e.g. with towncrier (used by pip, pytest, attrs, twisted; GitLab moved to per-MR changelog files and reported ~90% fewer merge conflicts):

• each PR adds docs/changes/<PR>.<type>.md with its one-line entry, e.g. docs/changes/457.enh.md, with types matching our prefixes (add, enh, fix, mnt, doc, tst)
• history.md is only touched at release time: towncrier build --version 0.13.0 collects the fragments into a new section (with the {pull} links and author) and deletes them
• a small CI check can ask for a fragment on each PR (skippable with a label for trivial PRs)

Setup is a [tool.towncrier] table in pyproject.toml plus a template, and the release checklist gets one command. If that sounds good, I can open a PR for it and convert the open PRs' entries as they get updated.

[aladinor]

Thanks @syedhamidali, this works well. One suggestion on CFP (inline), not a blocker. Approving.

What I ran, at c20fa00 merged locally with main (a8f2dac): NEXRAD, CfRadial1/2 and model tests, 278 passed, black/ruff clean. The 11 new or changed NEXRAD tests fail on main. nyquist_velocity matches the RAD block on every sweep of KATX 2013, KILX 2023 and KLBB 2016 (e.g. 8.81/35.09/29.97/32.33 m s-1 on KATX), and the KLIX MSG_1 surveillance cut gives NaN. The moments stay lazy, and mask_and_scale={"VRADH": False} and False work through xr.open_dataset, xr.open_datatree, xd.open_datatree and open_nexradlevel2_datatree. to_cfradial1 writes both values as (time,) and they read back; the moments are the same after both round trips.

Before merging: main needs to be merged in once more. The conflict in NexradLevel2Store.__init__ is only main's _ManagedStoreMixin base (from #433) next to the new mask_and_scale argument, and the history entry moves from 0.13.0 to Development (unreleased). The closing fixture in the test suggestion comes with that merge.

Nit: the new tests don't close what they open; main's NEXRAD tests use the closing fixture now.

On your first open point, I'm fine with range folded masked by default: the IRIS reader also returns NaN for its no-data codes (#465), and the raw codes stay available with mask_and_scale=False. @kmuehlbauer may see it differently.

[aladinor]

Thanks for applying the CFP suggestions. The new test uses the closing fixture from main, so it errors until main is merged in, and CI doesn't run while the PR has conflicts. With main merged locally (conflict in NexradLevel2Store.__init__ and history.md only): NEXRAD tests 184 passed, black/ruff clean.

[kmuehlbauer]

On your first open point, I'm fine with range folded masked by default: the IRIS reader also returns NaN for its no-data codes (#465), and the raw codes stay available with mask_and_scale=False. @kmuehlbauer may see it differently.

Either way is fine. Finally, the user will tell us.

[kmuehlbauer]

Thanks all!

[syedhamidali]

Thank you!