diff --git a/docs/about/releases.md b/docs/about/releases.md index a5dc0736..33774b93 100644 --- a/docs/about/releases.md +++ b/docs/about/releases.md @@ -16,6 +16,8 @@ Adds a `nrefs` accessor for counting virtual chunk references, a `mode` paramete ### Bug fixes - Fix parsing kerchunk references that use a **structured (record) dtype**, as produced for a FITS `BinTableHDU` (e.g. an SDSS spectrum). Such references round-trip the dtype through JSON as a list of `[name, format]` lists and encode the `fill_value` as base64 raw bytes; `from_kerchunk_refs` now coerces the field specs back to tuples before `np.dtype` and decodes the base64 `fill_value` into a structured scalar, instead of raising `TypeError`. By [David Stuebe](https://github.com/emfdavid). +- Fix the bytes codec recording the wrong `endian` for a **big-endian structured dtype**, which made virtual references to big-endian record data (e.g. a FITS `BinTableHDU`) decode to silently wrong values. The byte order was read from `dtype.byteorder`, which numpy reports as `"|"` for any structured dtype whatever its fields hold, so such arrays fell through to the little-endian default; it is now read from the fields, looking through subarray and nested-struct fields and ignoring fields that carry no byte order (single-byte numbers, strings). A structured dtype whose fields mix byte orders now raises, since a Zarr V3 array has a single bytes codec and cannot express it. + By [David Stuebe](https://github.com/emfdavid). - Writing a virtual `DataTree` with `region` or `append_dim` (forwarded to each node) previously always failed with `ContainsGroupError`, because every group was unconditionally created; the existing groups are now opened instead. By [Aaron Spring](https://github.com/aaronspring). diff --git a/virtualizarr/codecs.py b/virtualizarr/codecs.py index d60f2fc1..623d83f5 100644 --- a/virtualizarr/codecs.py +++ b/virtualizarr/codecs.py @@ -77,6 +77,43 @@ def extract_codecs( return (arrayarray_codecs, arraybytes_codec, bytesbytes_codecs) +def _byte_orders(dtype: np.dtype) -> set[str]: + """ + Collect the byte-order characters of a dtype's multi-byte values. + + ``dtype.byteorder`` describes only the dtype itself, and numpy reports ``"|"`` for every + structured dtype regardless of what its fields hold, so the fields are walked instead. A + subarray field (``(" bool: + """ + Whether ``dtype``'s multi-byte values are stored big-endian. + + Raises + ------ + ValueError + If the dtype's fields mix byte orders, which a Zarr V3 array cannot express. + """ + orders = _byte_orders(dtype) + if ">" not in orders: + return False + if len(orders) > 1: + raise ValueError( + f"Cannot represent {dtype} as a Zarr V3 array: its fields mix byte orders " + f"({sorted(orders)}). A Zarr V3 array has one bytes codec, so every multi-byte " + f"field must share a single byte order." + ) + return True + + def convert_to_codec_pipeline( dtype: np.dtype, codecs: list[dict] | None = [], @@ -106,7 +143,7 @@ def convert_to_codec_pipeline( arrayarray_codecs, arraybytes_codec, bytesbytes_codecs = extract_codecs(zarr_codecs) if arraybytes_codec is None: - if dtype.byteorder == ">": + if _is_big_endian(dtype): arraybytes_codec = BytesCodec(endian="big") else: arraybytes_codec = BytesCodec() diff --git a/virtualizarr/tests/test_codecs.py b/virtualizarr/tests/test_codecs.py index f87a2c3e..6c2fef71 100644 --- a/virtualizarr/tests/test_codecs.py +++ b/virtualizarr/tests/test_codecs.py @@ -122,6 +122,43 @@ def test_convert_to_codec_pipeline_scenarios(self, input_codecs, expected_pipeli result = convert_to_codec_pipeline(dtype=dtype, codecs=input_codecs) assert result == expected_pipeline + @pytest.mark.parametrize( + "dtype,expected_endian", + [ + # Plain dtypes: numpy reports the byte order on the dtype itself. + (np.dtype(">i4"), "big"), + (np.dtype("f4"), ("ivar", ">f4")]), "big"), + (np.dtype([("flux", "f4"), ("name", "S8")]), "big"), + (np.dtype([("flux", ">f4"), ("flag", "i1")]), "big"), + (np.dtype([("flag", "i1"), ("name", "S8")]), "little"), + # A subarray field hides its byte order behind ``.base``. + (np.dtype([("flux", ">f4", (3,))]), "big"), + # A nested struct hides it another level down. + (np.dtype([("coadd", [("flux", ">f4")])]), "big"), + ], + ) + def test_byte_order_is_read_from_struct_fields(self, dtype, expected_endian): + """The bytes codec's endian must describe the *stored* byte order. + + A FITS binary table arrives as a big-endian structured dtype; picking the codec from + ``dtype.byteorder`` (which is "|" for any struct) silently stored it as little-endian. + """ + result = convert_to_codec_pipeline(dtype=dtype, codecs=None) + assert result.array_bytes_codec == BytesCodec(endian=expected_endian) + + def test_mixed_byte_order_struct_raises(self): + """Zarr v3 gives an array a single bytes codec, so one array cannot store both.""" + with pytest.raises(ValueError, match="single byte order"): + convert_to_codec_pipeline( + dtype=np.dtype([("big", ">f4"), ("little", "