From bf035679f8f3479e1cd1152c910b17d5a50580c9 Mon Sep 17 00:00:00 2001 From: Mark Harfouche Date: Thu, 6 Aug 2026 06:51:28 -0400 Subject: [PATCH] Preserve list attributes when reading with h5netcdf
Claude's draft h5netcdf collapses every length-1 attribute to a scalar, mirroring netcdf4-python, so `{"a": ["one"]}` read back as `{"a": "one"}` and `[]` as `array([], dtype=float64)` (GH10275). That collapse hides two things the file does record: - netCDF stores a single string as NC_CHAR, whose HDF5 dataspace is scalar, and a sequence of strings as NC_STRING, whose dataspace is not; - a zero length attribute cannot be a scalar whatever its dataspace, whether it is the null dataspace netcdf-c writes or the (0,) h5netcdf writes. `_read_attributes` now reads the dataspace off the underlying h5py attribute and restores those two cases. Numeric attributes are deliberately left alone: netcdf-c stores a scalar number as a length-1 vector, so 1 and [1] are the same bytes on disk, and guessing "sequence" would turn the scale_factor of every existing file into a list. Nothing about how xarray writes files changed -- verified by dumping dtype/shape/dataspace/value for every attribute written by both engines, which is byte-for-byte identical to main. The netCDF4 engine is untouched, since netcdf4-python collapses length-1 attributes before xarray sees them and exposes no way to ask for the length. Resume this Claude session: ``` cd /Users/mark/git/xarray/xarray claude --resume 8659c07e-0bfb-4785-8161-4b65723eeb66 ```
Co-authored-by: Claude --- doc/whats-new.rst | 16 ++++++++++ xarray/backends/h5netcdf_.py | 58 +++++++++++++++++++++++++++++++++++ xarray/tests/test_backends.py | 44 ++++++++++++++++++++++++++ 3 files changed, 118 insertions(+) diff --git a/doc/whats-new.rst b/doc/whats-new.rst index 70aef4acf3e..a796d4c320e 100644 --- a/doc/whats-new.rst +++ b/doc/whats-new.rst @@ -49,6 +49,22 @@ Deprecations Bug Fixes ~~~~~~~~~ +- The ``h5netcdf`` engine no longer collapses list-valued attributes when it + reads them. A ``["one"]`` written by ``engine="h5netcdf"`` came back as + ``"one"``, and an empty list as ``array([], dtype=float64)``; both now come + back as the lists they were written as (:issue:`10275`). h5netcdf collapses + every length-1 attribute to a scalar to mirror netcdf4-python, which hides two + things the file does record: netCDF stores one string as ``NC_CHAR`` in a + scalar HDF5 dataspace and a sequence of strings as ``NC_STRING`` in a + non-scalar one, and a zero length attribute cannot be a scalar at all. Nothing + about how xarray writes files changed, so existing files are unaffected. + + Numeric attributes are deliberately left alone: netcdf-c stores a scalar + number as a length-1 vector, so ``1`` and ``[1]`` are the same bytes on disk + and cannot be told apart. The ``netcdf4`` engine is also unchanged, since + netcdf4-python collapses length-1 attributes before xarray sees them and + offers no way to ask for the length. + By `Mark Harfouche `_. - Fix async zarr tests using ``wraps`` with ``autospec=True`` on async methods, which caused ``AsyncMock`` objects to leak through instead of real array data (:pull:`11232`). diff --git a/xarray/backends/h5netcdf_.py b/xarray/backends/h5netcdf_.py index 445c694c1fb..b289a410c37 100644 --- a/xarray/backends/h5netcdf_.py +++ b/xarray/backends/h5netcdf_.py @@ -74,11 +74,68 @@ def _getitem(self, key): return array[key] +#: Sentinel for an attribute whose dataspace we cannot inspect. +_UNKNOWN_DATASPACE = object() + + +def _attribute_dataspace_shape(h5attrs, key): + """Shape of the HDF5 dataspace backing attribute ``key``. + + ``()`` is a scalar dataspace, holding exactly one value. ``(n,)`` is a + simple dataspace, holding a sequence of ``n``. ``None`` is a null dataspace, + which netcdf-c writes for a zero length attribute. Returns + :py:data:`_UNKNOWN_DATASPACE` when the dataspace cannot be inspected. + + h5netcdf hands back an already-decoded value that hides this distinction, + because it collapses any length-1 attribute to a scalar to mirror + netcdf4-python (:issue:`10275`), so go to the underlying h5py attribute. + """ + if h5attrs is None: + return _UNKNOWN_DATASPACE + try: + return h5attrs.get_id(key).shape + except (AttributeError, KeyError, TypeError): + # ``get_id`` is part of h5py's low level API, which the pyfive backend + # does not provide. + return _UNKNOWN_DATASPACE + + +def _restore_sequence_attribute(value, shape): + """Undo h5netcdf's collapse of a length-1 or zero-length attribute. + + Only the cases the file unambiguously records are restored: + + * a sequence of strings, which netCDF stores as NC_STRING in a non-scalar + dataspace, as opposed to the single NC_CHAR string it stores in a scalar + one; + * a zero length attribute, which cannot be a scalar whatever its dataspace. + + Numeric attributes are left alone. netcdf-c stores a scalar number as a + length-1 vector, so ``1`` and ``[1]`` are the same bytes on disk and there + is nothing to tell them apart -- guessing would turn every ``scale_factor`` + in every existing file into a list. + """ + if shape is _UNKNOWN_DATASPACE: + return value + if isinstance(value, str | bytes): + # a scalar dataspace holds one string; anything else holds a sequence. + # ``shape is None`` is the zero length _FillValue h5netcdf returns as + # b"", which is not a sequence of strings. + if shape is not None and len(shape) > 0: + return [value] + elif isinstance(value, np.ndarray) and value.size == 0: + return [] + return value + + def _read_attributes(h5netcdf_var): # GH451 # to ensure conventions decoding works properly on Python 3, decode all # bytes attributes to strings attrs = {} + # ``Attributes`` wraps the h5py attributes it decodes; we need the raw ones + # to tell a single value from a sequence of one -- see GH10275. + h5attrs = getattr(h5netcdf_var.attrs, "_h5attrs", None) for k, v in h5netcdf_var.attrs.items(): if k not in ["_FillValue", "missing_value"] and isinstance(v, bytes): try: @@ -90,6 +147,7 @@ def _read_attributes(h5netcdf_var): f"returning bytes undecoded.", UnicodeWarning, ) + v = _restore_sequence_attribute(v, _attribute_dataspace_shape(h5attrs, k)) attrs[k] = v return attrs diff --git a/xarray/tests/test_backends.py b/xarray/tests/test_backends.py index 96430c85ea7..7480ab5461d 100644 --- a/xarray/tests/test_backends.py +++ b/xarray/tests/test_backends.py @@ -4850,6 +4850,50 @@ def test_cross_engine_read_write_netcdf4(self) -> None: with open_dataset(tmp_file, engine=read_engine) as actual: assert_identical(data, actual) + def test_roundtrip_string_sequence_attrs(self) -> None: + # GH10275: netCDF stores a single string as NC_CHAR and a sequence of + # strings as NC_STRING, so a length-1 sequence should not come back as + # a bare string. + original = Dataset( + {"x": ("t", [1.0], {"attr": ["one_item_only"]})}, + attrs={ + "one": ["one_item_only"], + "two": ["one_item", "two_items"], + "scalar": "plain", + }, + ) + with self.roundtrip(original) as actual: + assert actual.attrs["one"] == ["one_item_only"] + assert actual.attrs["two"] == ["one_item", "two_items"] + # a scalar string is not a sequence and must stay a scalar + assert actual.attrs["scalar"] == "plain" + assert actual["x"].attrs["attr"] == ["one_item_only"] + + def test_roundtrip_empty_list_attr(self) -> None: + # GH10275: a zero length attribute cannot be a scalar whatever its + # dataspace, so it is restored as an empty list rather than handed back + # as the empty float64 array h5netcdf types it as. This holds for files + # written by the netCDF4 engine too, which stores it as a null dataspace. + original = Dataset({"x": ("t", [1.0], {"attr": []})}, attrs={"empty": []}) + with self.roundtrip(original) as actual: + assert actual.attrs["empty"] == [] + assert actual["x"].attrs["attr"] == [] + + with create_tmp_file() as tmp_file: + original.to_netcdf(tmp_file, engine="netcdf4") + with open_dataset(tmp_file, engine="h5netcdf") as actual: + assert actual.attrs["empty"] == [] + + def test_numeric_attrs_are_not_turned_into_lists(self) -> None: + # netcdf-c stores a scalar number as a length-1 vector, so 1 and [1] are + # the same bytes on disk. Guessing "sequence" would turn the scale_factor + # of every existing file into a list, so numbers are left alone (GH10275). + original = Dataset(attrs={"scale_factor": 0.01, "flags": [1, 2]}) + with self.roundtrip(original, open_kwargs={"decode_cf": False}) as actual: + assert actual.attrs["scale_factor"] == 0.01 + assert not isinstance(actual.attrs["scale_factor"], list) + assert_array_equal(actual.attrs["flags"], [1, 2]) + def test_read_byte_attrs_as_unicode(self) -> None: with create_tmp_file() as tmp_file: with nc4.Dataset(tmp_file, "w") as nc: