Fix bytes codec endian for big-endian structured dtypes

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

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 (``("<f4", (3,))``) hides its byte order behind ``.base``, and a nested struct
+    hides it one level further down, hence the recursion. Values that have no byte order at all --
+    single-byte numbers, strings -- contribute nothing rather than counting as little-endian.
+    """
+    base = dtype.base  # see through a subarray field
+    if base.fields is None:
+        return set() if base.byteorder == "|" else {base.byteorder}
+    return set().union(*(_byte_orders(field[0]) for field in base.fields.values()))
+
+
+def _is_big_endian(dtype: np.dtype) -> 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()

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("<i4"), "little"),
+            # Structured dtypes: numpy reports byteorder "|" on the dtype whatever its
+            # fields say, so the byte order has to be read off the fields.
+            (np.dtype([("flux", ">f4"), ("ivar", ">f4")]), "big"),
+            (np.dtype([("flux", "<f4"), ("ivar", "<f4")]), "little"),
+            # Fields that carry no byte order (single-byte, strings) are not evidence of
+            # little-endian storage -- they are ignored.
+            (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", "<f4")]), codecs=None
+            )
+
 
 class TestExtractCodecs:
     """Test the extract_codecs function."""