fix(dtype): fix nested v2 round trips and detect unsupported field conversions

changes/4340.bugfix.md

@@ -0,0 +1 @@
+Fix nested structured dtype round-tripping through Zarr format 2 metadata. Native NumPy dtypes with field titles or subarray fields now raise `ValueError` identifying the unsupported field during conversion, rather than being converted to duplicate fields or raw-byte fields. This reflects a limitation of the current Zarr-Python dtype implementation; the V2 format supports subarray fields, as did Zarr-Python 2.x. Padded layouts continue to be converted to packed layouts, now with a `ZarrUserWarning` explaining that field values are preserved when writing arrays but offsets and itemsize may change. These checks also apply to nested fields. Add dtype serialization and layout-conversion property tests using new `zdtypes` and `structured_dtypes` strategies.

docs/user-guide/data_types.md

@@ -84,8 +84,18 @@ arbitrary fixed-size byte strings. The `str` attribute of a regular NumPy void
 data type is the same as the `str` of a NumPy structured data type. This means that the `str`
 attribute does not convey information about the fields contained in a structured data type.
 For these reasons, Zarr V2 uses a special data type encoding for structured data types.
-They are stored in JSON as lists of pairs, where the first element is a string, and the second
-element is a Zarr V2 data type specification. This representation supports recursion.
+The [V2 specification](https://zarr-specs.readthedocs.io/en/latest/v2/v2.0.html#data-type-encoding)
+represents fields as `[fieldname, datatype]` or `[fieldname, datatype, shape]`, where the
+optional shape describes a subarray field. Field data types can themselves be structured.
+For example, `[["position", "<f4", [2]]]` describes a field containing two float32 values.
+
+Zarr-Python's current structured dtype conversion uses a packed layout. NumPy arrays with alignment, padding,
+or explicit field offsets remain accepted, with a `ZarrUserWarning`: writing preserves
+field values, but the stored dtype's offsets and itemsize may differ from the input.
+The current conversion rejects field titles and subarray fields, including nested ones,
+for both Zarr formats. Subarray rejection is an implementation limitation: V2 supports
+subarray metadata, and Zarr-Python 2.x supported these fields. The V2 specification describes
+field names as strings and does not specify NumPy's `(title, name)` form.
 
 For example:
 

src/zarr/core/dtype/common.py

@@ -80,7 +80,10 @@ def check_structured_dtype_v2_inner(data: object) -> TypeGuard[StructuredName_V2
     if isinstance(data[-1], str):
         return True
     elif isinstance(data[-1], Sequence):
-        return check_structured_dtype_v2_inner(data[-1])
+        # A nested structured dtype's field has the form [name, [[sub_name, sub_dtype], ...]],
+        # i.e. the last element is itself a sequence of field pairs rather than a single
+        # [name, dtype] pair, so it must be validated as a list of fields, not a single field.
+        return check_structured_dtype_name_v2(data[-1])
     return False
 
 

src/zarr/core/dtype/npy/structured.py

@@ -1,10 +1,13 @@
 from __future__ import annotations
 
+import warnings
 from collections.abc import Sequence
 from dataclasses import dataclass
+from pathlib import Path
 from typing import TYPE_CHECKING, ClassVar, Literal, Self, TypeGuard, cast, overload
 
 import numpy as np
+from numpy.lib.recfunctions import repack_fields
 
 from zarr.core.common import NamedConfig
 from zarr.core.dtype.common import (
@@ -22,13 +25,54 @@
     check_json_str,
 )
 from zarr.core.dtype.wrapper import TBaseDType, TBaseScalar, ZDType
-from zarr.errors import DataTypeValidationError
+from zarr.errors import DataTypeValidationError, ZarrUserWarning
 
 if TYPE_CHECKING:
     from zarr.core.common import JSON, ZarrFormat
 
 StructuredScalarLike = list[object] | tuple[object, ...] | bytes | int
 
+# The root of the zarr package, used to attribute layout warnings to the caller outside zarr.
+_ZARR_PACKAGE_ROOT = str(Path(__file__).parents[3])
+
+
+def _unsupported_field_feature(dtype: np.dtype[np.void]) -> str | None:
+    """
+    Check for field features unsupported by this implementation's native dtype conversion.
+
+    `Structured.fields` stores `(name, ZDType)` pairs and `to_native_dtype` packs them
+    contiguously. This conversion does not preserve NumPy field titles or subarray shapes.
+    Reject those features, including in nested fields, rather than silently losing them:
+
+    - field titles, e.g. `np.dtype([(("title", "name"), "i4")])`
+    - subarray fields, e.g. `np.dtype([("name", "i4", (2,))])`
+
+    Returns
+    -------
+    str | None
+        `None` if these field features are supported, otherwise a description of the problem.
+
+    Notes
+    -----
+    This is an implementation limitation, not a statement about the V2 format, which has
+    an encoding for subarray fields.
+    """
+    names = dtype.names
+    fields = dtype.fields
+    if names is None or fields is None:  # pragma: no cover - only called on structured dtypes
+        return None
+    for name in names:
+        field_dtype, _offset, *title = fields[name]
+        if title:
+            return f"field {name!r} has a title ({title[0]!r})"
+        if field_dtype.subdtype is not None:
+            return f"field {name!r} is a subarray with shape {field_dtype.shape}"
+        if field_dtype.names is not None:
+            reason = _unsupported_field_feature(field_dtype)
+            if reason is not None:
+                return f"within field {name!r}: {reason}"
+    return None
+
 
 class StructuredJSON_V2(DTypeConfig_V2[StructuredName_V2, None]):
     """
@@ -175,6 +219,15 @@ def from_native_dtype(cls, dtype: TBaseDType) -> Self:
         DataTypeValidationError
             If the input data type is not an instance of np.dtypes.VoidDType with a non-null
             ``fields`` attribute.
+        ValueError
+            If the input has field titles or subarray fields, which this implementation's
+            native dtype conversion does not support.
+
+        Warns
+        -----
+        ZarrUserWarning
+            If a non-default field layout is converted to a packed layout. Field values are
+            preserved when writing arrays, but offsets and itemsize may change.
 
         Notes
         -----
@@ -185,10 +238,38 @@ def from_native_dtype(cls, dtype: TBaseDType) -> Self:
 
         fields: list[tuple[str, ZDType[TBaseDType, TBaseScalar]]] = []
         if cls._check_native_dtype(dtype):
-            # fields of a structured numpy dtype are either 2-tuples or 3-tuples. we only
-            # care about the first element in either case.
-            for key, (dtype_instance, *_) in dtype.fields.items():  # type: ignore[union-attr]
-                dtype_wrapped = get_data_type_from_native_dtype(dtype_instance)
+            reason = _unsupported_field_feature(dtype)
+            if reason is not None:
+                # NOTE: this is a ValueError rather than a DataTypeValidationError on purpose.
+                # The data type registry suppresses DataTypeValidationError (treating it as
+                # "this dtype does not match"), but a dtype with an unsupported field feature
+                # *does* match this dtype class -- it simply cannot be represented faithfully --
+                # so we must raise an error the registry propagates to the caller.
+                raise ValueError(
+                    f"Cannot convert the structured data type {dtype}: {reason}. "
+                    "Zarr-Python's current structured dtype conversion does not support "
+                    "field titles or subarray fields."
+                )
+            # Repack once, at the top level, before resolving the fields. Nested fields then
+            # reach the registry already packed, so a padded nested field warns exactly once,
+            # here, rather than once per level of nesting.
+            packed = cast("np.dtype[np.void]", repack_fields(dtype, recurse=True))
+            if packed != dtype:
+                warnings.warn(
+                    "The structured dtype is converted to a packed field layout. "
+                    "Field values are preserved when writing arrays, but field offsets and "
+                    "itemsize may change.",
+                    ZarrUserWarning,
+                    # Attribute the warning to the first frame outside the zarr package, since
+                    # the depth of the call chain that leads here varies by entry point. This
+                    # reaches the caller for direct uses of the dtype API; the synchronous array
+                    # API runs on the event loop thread, where no caller frame is available.
+                    skip_file_prefixes=(_ZARR_PACKAGE_ROOT,),
+                )
+            # Iterate over `names` rather than `fields`: the `fields` mapping also
+            # contains an entry for every field title, which would duplicate titled fields.
+            for key in packed.names:  # type: ignore[union-attr]
+                dtype_wrapped = get_data_type_from_native_dtype(packed.fields[key][0])  # type: ignore[index]
                 fields.append((key, dtype_wrapped))
 
             return cls(fields=tuple(fields))

src/zarr/testing/strategies.py

@@ -1,3 +1,4 @@
+import dataclasses
 import itertools
 import math
 import sys
@@ -27,7 +28,11 @@
 from zarr.core.array import Array, CompressorsLike, SerializerLike
 from zarr.core.chunk_key_encodings import DefaultChunkKeyEncoding
 from zarr.core.common import JSON, AccessModeLiteral, ZarrFormat
-from zarr.core.dtype import get_data_type_from_native_dtype
+from zarr.core.dtype import data_type_registry, get_data_type_from_native_dtype
+from zarr.core.dtype.common import HasItemSize
+from zarr.core.dtype.npy.common import DATETIME_UNIT
+from zarr.core.dtype.npy.structured import Struct
+from zarr.core.dtype.wrapper import TBaseDType, TBaseScalar, ZDType
 from zarr.core.metadata import ArrayV2Metadata, ArrayV3Metadata
 from zarr.core.metadata.v3 import RectilinearChunkGridMetadata, RegularChunkGridMetadata
 from zarr.core.sync import sync
@@ -70,6 +75,123 @@ def dtypes() -> st.SearchStrategy[np.dtype[Any]]:
     )
 
 
+_field_names = st.text(
+    alphabet=st.characters(min_codepoint=97, max_codepoint=122), min_size=1, max_size=4
+)
+_field_titles = st.text(
+    alphabet=st.characters(min_codepoint=65, max_codepoint=90), min_size=1, max_size=4
+)
+
+
+def _leaf_zdtypes(cls: type[ZDType[TBaseDType, TBaseScalar]]) -> SearchStrategy[ZDType[Any, Any]]:
+    """
+    A strategy for instances of a single non-struct `ZDType` class, drawing each constructor
+    parameter the class declares from its valid range.
+    """
+    params = {f.name for f in dataclasses.fields(cls)}
+    kwargs: dict[str, SearchStrategy[Any]] = {}
+    if "endianness" in params:
+        kwargs["endianness"] = st.sampled_from(["little", "big"])
+    if "length" in params:
+        kwargs["length"] = st.integers(min_value=1, max_value=16)
+    if "unit" in params:
+        # NumPy spells the microsecond unit "us", so the "μs" alias never round-trips as-is.
+        kwargs["unit"] = st.sampled_from([u for u in DATETIME_UNIT if u != "μs"])
+        kwargs["scale_factor"] = st.integers(min_value=1, max_value=2**31 - 1)
+        return st.builds(cls, **kwargs).map(_normalize_generic_scale_factor)
+    return st.builds(cls, **kwargs)
+
+
+def _normalize_generic_scale_factor(zdtype: Any) -> Any:
+    """
+    NumPy retains generic scale factors internally, but its dtype string omits them.
+    Use `scale_factor=1` so the generated dtype survives Zarr V2 string serialization.
+    """
+    if zdtype.unit == "generic":
+        return dataclasses.replace(zdtype, scale_factor=1)
+    return zdtype
+
+
+def _struct_zdtypes(
+    children: SearchStrategy[ZDType[Any, Any]],
+) -> SearchStrategy[ZDType[Any, Any]]:
+    """A strategy for `Struct` instances whose field data types are drawn from `children`."""
+
+    @st.composite
+    def _draw(draw: st.DrawFn) -> ZDType[Any, Any]:
+        num_fields = draw(st.integers(min_value=1, max_value=4))
+        # suffix with the index so that names are unique without filtering
+        names = [f"{draw(_field_names)}{i}" for i in range(num_fields)]
+        return Struct(fields=tuple((name, draw(children)) for name in names))
+
+    return _draw()
+
+
+def zdtypes(*, max_leaves: int = 6) -> SearchStrategy[ZDType[Any, Any]]:
+    """
+    Generate instances of the built-in registered `ZDType` classes, including nested `Struct`.
+
+    Struct fields are restricted to fixed-size data types, as required by the V3 `struct`
+    extension. This strategy samples bounded lengths and normalized datetime units/scales;
+    it does not cover every valid instance or arbitrary third-party dtype constructors.
+    """
+    leaf_classes = [cls for cls in data_type_registry.contents.values() if cls is not Struct]
+    leaves = st.one_of([_leaf_zdtypes(cls) for cls in leaf_classes])
+    fixed_size_leaves = st.one_of(
+        [_leaf_zdtypes(cls) for cls in leaf_classes if issubclass(cls, HasItemSize)]
+    )
+    structs = st.recursive(fixed_size_leaves, _struct_zdtypes, max_leaves=max_leaves).filter(
+        lambda dt: isinstance(dt, Struct)
+    )
+    return leaves | structs
+
+
+@st.composite
+def structured_dtypes(
+    draw: st.DrawFn, *, allow_extended: bool = False, max_depth: int = 3
+) -> np.dtype[np.void]:
+    """
+    A strategy for native NumPy structured dtypes, flat or nested.
+
+    With `allow_extended=False` (the default), generate packed fields without titles or
+    subarray shapes. With `allow_extended=True`, also generate field titles, subarray fields,
+    and `align=True` layouts, independently. The current native dtype conversion rejects
+    titles and subarray fields and accepts padding with a warning. These are implementation
+    behaviors, not restrictions imposed by the V2 format.
+    """
+    fixed_size_leaves = st.one_of(
+        [
+            _leaf_zdtypes(cls)
+            for cls in data_type_registry.contents.values()
+            if cls is not Struct and issubclass(cls, HasItemSize)
+        ]
+    )
+
+    def build(depth: int) -> np.dtype[np.void]:
+        num_fields = draw(st.integers(min_value=1, max_value=4))
+        # suffix with the index so that names and titles are unique without filtering; titles
+        # draw from a different alphabet so they never collide with names either
+        names = [f"{draw(_field_names)}{i}" for i in range(num_fields)]
+        titles = [f"{draw(_field_titles)}{i}" for i in range(num_fields)]
+        specs: list[tuple[Any, Any]] = []
+        for name, title in zip(names, titles, strict=True):
+            field_dtype: Any
+            if depth < max_depth and draw(st.booleans()):
+                field_dtype = build(depth + 1)
+            else:
+                field_dtype = draw(fixed_size_leaves).to_native_dtype()
+            key: Any = name
+            if allow_extended and draw(st.booleans()):
+                key = (title, name)
+            if allow_extended and draw(st.booleans()):
+                field_dtype = (field_dtype, draw(npst.array_shapes(max_dims=2, max_side=3)))
+            specs.append((key, field_dtype))
+        align = allow_extended and draw(st.booleans())
+        return np.dtype(specs, align=align)
+
+    return build(0)
+
+
 def v3_dtypes() -> st.SearchStrategy[np.dtype[Any]]:
     return dtypes()