Skip to content

Fix silent byte-swapping of non-native-endian attribute values in the netcdf4 backend - #11543

Open
glaziermag wants to merge 3 commits into
pydata:mainfrom
glaziermag:fix-netcdf4-nonnative-endian-attrs
Open

Fix silent byte-swapping of non-native-endian attribute values in the netcdf4 backend#11543
glaziermag wants to merge 3 commits into
pydata:mainfrom
glaziermag:fix-netcdf4-nonnative-endian-attrs

Conversation

@glaziermag

@glaziermag glaziermag commented Aug 24, 2026

Copy link
Copy Markdown

Description

Writing a dataset whose attributes hold non-native-endian numpy arrays silently stores byte-swapped values with the netcdf4 engine. The easiest way to get such attributes is reading a netCDF-3 file with the scipy engine, which returns attribute arrays big-endian — so this fully-default round-trip corrupts metadata on disk:

import xarray as xr
ds = xr.open_dataset("xarray/tests/data/example_1.nc.gz")  # scipy engine guessed
ds.load().to_netcdf("out.nc")                              # netcdf4 engine guessed
xr.open_dataset("out.nc")["rh"].attrs["valid_range"]
# array([0.00000e+000, 3.03865e-319])  — was [0., 1.]; 3.03865e-319 is byte-swapped 1.0

The same happens for xarray/tests/data/bears.nc (acf [-2., 1., 0.][2.69e-43, 4.60e-41, 0.0]) and for integer attributes (>i4 [1, 2][16777216, 33554432]), for both variable and global attributes, and in NETCDF4 as well as NETCDF3 formats via this engine. No warning or error is raised; inspecting the written file with h5py shows the byte-swapped values are committed to disk at write time.

Cause: netCDF4-python's setncattr/setncatts do not byte-swap. The backend already converts variable data to native endianness for exactly this reason (_force_native_endianness, whose comment notes this "is not supported by the netCDF4 python library"), but attribute values reach setncatts/setncattr unconverted. This PR applies the same native-endianness conversion to attribute values at both write sites (prepare_variable and set_attribute), with a regression test. The h5netcdf writer already handles identical input correctly and is unchanged.

Checklist

  • Tests added
  • User visible changes (including notable bug fixes) are documented in whats-new.rst

AI Disclosure

  • This PR contains AI-generated content.
    • I have tested any AI-generated content in my PR.
    • I take responsibility for any AI-generated content in my PR.
      Tools: Claude (Claude Code). The defect was found by round-tripping the committed files in xarray/tests/data/ through default open_dataset/to_netcdf. The repro above was confirmed twice in a clean environment at 551ced5 (netCDF4 1.7.4, scipy 1.18.1, numpy 2.5.2), the fix verified to clear it, and the TestNetCDF4Data/TestScipy/TestGenericNetCDFData suites run against the patch (427 passed). Prompt: a standing instruction to find, verify, and fix one upstream defect.

[This is Claude Code on behalf of glaziermag]

netCDF4-python does not byte-swap non-native-endian attribute arrays on
write, so attributes read from netCDF-3 files with the scipy engine
(which returns big-endian arrays) were silently corrupted when written
back with the netcdf4 engine (e.g. valid_range [0., 1.] became
[0., 3.03865e-319], the byte-swapped bit pattern of 1.0). Convert
attribute values to native endianness before writing, matching what
_force_native_endianness already does for variable data.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
pre-commit-ci Bot and others added 2 commits August 24, 2026 21:19
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@dcherian
dcherian requested a review from kmuehlbauer August 26, 2026 03:07
@CAOShurong

Copy link
Copy Markdown

Independent verification of this PR at exact head 121c0595 against base 551ced5e (Windows x64, Python 3.12.10, numpy 2.5.2, netCDF4 1.7.4, scipy 1.18.1, h5netcdf + h5py installed, dask 2026.8.0). All runs below were executed locally; module path was asserted (xarray.__file__) in every process to guarantee which tree was imported. AI assistance used for this review, per the project's AI policy.

Bug reproduced on pristine base — exactly as reported. Reading example_1.nc.gz with the guessed scipy engine and writing back with the default engine stores valid_range = [0.0, 3.03865e-319] (byte-swapped 1.0) for variable rh; direct big-endian attributes corrupt too: >f8 [0.0, 1.0] -> [0.0, 3.03865e-319], >i4 [1, 2] -> [16777216, 33554432]. No warning is raised; the swapped values are what an independent reader sees on disk.

All green at PR head, same harness, same interpreter:

  • Case A — PR-body repro (committed example_1.nc.gz, both engines auto-guessed): [0.0, 1.0] round-trips correctly.
  • Case B — explicit scipy read of committed example_1.nc, then to_netcdf(engine="netcdf4", format="NETCDF4"): correct.
  • Case C — direct >f8 variable attribute + >i4 global attribute through the netcdf4 engine: correct.
  • Case D — h5netcdf writer on identical input still returns [0.0, 1.0] (untouched codepath confirmed).
  • Case E — format="NETCDF3_CLASSIC" written through the netcdf4 engine, read back with scipy: correct, so the new conversion sits cleanly before encode_nc3_attr_value and does not break classic-format attribute encoding.
  • Edge probes at head: 0-d big-endian scalar attribute, a read-only (write=False) big-endian array attribute, and plain string attributes all behave correctly.

The new regression test is load-bearing: run standalone (it is added by this PR) it fails on unpatched base with exactly the reported corruption and passes at head.

No behavioral drift on existing suites: test_backends.py -k "TestNetCDF4Data or TestScipy or TestGenericNetCDFData" gives 431 passed / 26 skipped / 16 xfailed / 4 xpassed at head — byte-identical outcome to the same selection on pristine base (431/26/16/4).

One non-blocking factual note on the description: bears.nc does not actually demonstrate the bug — its numeric attributes (acf, etc.) are stored little-endian and come back native ('<f4'), so nothing there gets byte-swapped. example_1.nc(.gz) carries the real big-endian valid_range and is the right repro file. Doesn't affect the fix; only the body text overstates that one file.

Diff is minimal and lands the guard at both write sites (set_attribute and prepare_variable); verified mergeable, full CI green at this head, no overlapping changes. This is ready for maintainer review from my side.

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.

2 participants