From 54ffce850567a508778d32aef8490f4f3bdef21b Mon Sep 17 00:00:00 2001 From: Davis Vann Bennett Date: Fri, 12 Jun 2026 14:20:30 +0200 Subject: [PATCH 1/9] Fix structured dtype v2 round-trip and silent loss of field offsets/padding Bug 1: Nested structured dtypes failed the Zarr V2 JSON round-trip. The inner type guard check_structured_dtype_v2_inner recursed into itself for a nested field's last element, but that element is a list of [name, dtype] field pairs, not a single pair. It now validates that element with check_structured_dtype_name_v2, so metadata written by to_json(zarr_format=2) can be read back. Bug 2: Structured dtypes with non-default (aligned / padded) field layouts were silently re-packed contiguously on round-trip, changing field offsets and itemsize and corrupting stored bytes. from_native_dtype now detects non-packed layouts (recursively) and raises a clear ValueError instead. Reading existing packed data is unaffected. Co-Authored-By: Claude Fable 5 --- src/zarr/core/dtype/common.py | 5 +- src/zarr/core/dtype/npy/structured.py | 45 +++++++++- tests/test_dtype/test_npy/test_structured.py | 89 ++++++++++++++++++++ 3 files changed, 137 insertions(+), 2 deletions(-) diff --git a/src/zarr/core/dtype/common.py b/src/zarr/core/dtype/common.py index 61cbfe0360..7a507892b8 100644 --- a/src/zarr/core/dtype/common.py +++ b/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 diff --git a/src/zarr/core/dtype/npy/structured.py b/src/zarr/core/dtype/npy/structured.py index dcc523d1d2..68b8e54525 100644 --- a/src/zarr/core/dtype/npy/structured.py +++ b/src/zarr/core/dtype/npy/structured.py @@ -2,7 +2,7 @@ from collections.abc import Sequence from dataclasses import dataclass -from typing import TYPE_CHECKING, ClassVar, Literal, Self, TypeGuard, cast, overload +from typing import TYPE_CHECKING, Any, ClassVar, Literal, Self, TypeGuard, cast, overload import numpy as np @@ -30,6 +30,34 @@ StructuredScalarLike = list[object] | tuple[object, ...] | bytes | int +def _is_default_packed(dtype: np.dtype[np.void]) -> bool: + """ + Check whether a structured numpy dtype uses the default contiguous (packed) field layout. + + The Zarr structured/struct metadata only records ``(name, dtype)`` pairs and reconstructs the + native dtype by packing the fields contiguously (see ``Structured.to_native_dtype``). A dtype + created with ``align=True``, or with explicit field offsets / extra padding, therefore cannot be + represented faithfully: its field offsets and itemsize would silently change on round-trip, + corrupting stored bytes. This function returns ``False`` for any such dtype. + + The check is recursive so that padding within a nested field dtype is detected even when the + outer dtype is itself packed. + """ + names = dtype.names + if names is None: # pragma: no cover - only called on structured dtypes + return True + repacked_fields: list[tuple[str, np.dtype[Any]]] = [] + for name in names: + field_dtype = dtype.fields[name][0] # type: ignore[index] + if field_dtype.names is not None and not _is_default_packed(field_dtype): + return False + repacked_fields.append((name, field_dtype)) + repacked = np.dtype(repacked_fields) + if repacked.itemsize != dtype.itemsize: + return False + return all(dtype.fields[n][1] == repacked.fields[n][1] for n in names) # type: ignore[index] + + class StructuredJSON_V2(DTypeConfig_V2[StructuredName_V2, None]): """ A wrapper around the JSON representation of the ``Structured`` data type in Zarr V2. @@ -185,6 +213,21 @@ def from_native_dtype(cls, dtype: TBaseDType) -> Self: fields: list[tuple[str, ZDType[TBaseDType, TBaseScalar]]] = [] if cls._check_native_dtype(dtype): + if not _is_default_packed(dtype): + # 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 non-packed layout *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 serialize the structured data type {dtype}. It uses a non-default " + "field layout (e.g. it was created with align=True, or has explicit field " + "offsets or padding), which the Zarr structured data type metadata cannot " + "represent: only the field names and dtypes are stored, and the fields are " + "always packed contiguously on read. Serializing this dtype would silently " + "change its field offsets and itemsize, corrupting stored data. Use a packed " + "structured dtype (without align=True or explicit offsets) instead." + ) # 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] diff --git a/tests/test_dtype/test_npy/test_structured.py b/tests/test_dtype/test_npy/test_structured.py index 554c3b4e41..105d11c5da 100644 --- a/tests/test_dtype/test_npy/test_structured.py +++ b/tests/test_dtype/test_npy/test_structured.py @@ -14,6 +14,7 @@ Struct, Structured, UInt8, + get_data_type_from_json, ) @@ -260,3 +261,91 @@ def test_struct_from_native_dtype() -> None: struct = Struct.from_native_dtype(dtype) assert struct.fields[0][0] == "field1" assert struct.fields[1][0] == "field2" + + +@pytest.mark.filterwarnings("ignore::zarr.errors.UnstableSpecificationWarning") +@pytest.mark.parametrize( + "dtype", + [ + # one level of nesting + np.dtype([("a", " None: + """ + Regression test for nested structured dtypes failing the Zarr V2 JSON round-trip. + + ``Struct.to_json(zarr_format=2)`` emits a nested field as ``[name, [[sub, dt], ...]]``, and + ``get_data_type_from_json`` must be able to read that form back. Previously the inner type + guard recursed as a single ``[name, dtype]`` pair and rejected the list-of-fields form, + so Zarr wrote V2 metadata it could not read back. + """ + zdtype = Struct.from_native_dtype(dtype) + json_v2 = zdtype.to_json(zarr_format=2) + recovered = get_data_type_from_json(json_v2, zarr_format=2) + assert recovered == zdtype + assert recovered.to_native_dtype() == dtype + + +@pytest.mark.filterwarnings("ignore::zarr.errors.UnstableSpecificationWarning") +def test_nested_structured_v2_array_round_trip() -> None: + """End-to-end test: write and read a Zarr V2 array with a nested structured dtype.""" + import zarr + + dtype = np.dtype([("a", " None: + """ + Regression test: structured dtypes with non-default (padded / aligned) field layouts must + fail loudly rather than silently dropping the padding. + + The Zarr structured metadata records only ``(name, dtype)`` pairs and re-packs fields + contiguously on read, so an aligned dtype would round-trip to a smaller itemsize, silently + corrupting stored chunk bytes. ``from_native_dtype`` detects this and raises instead. + """ + with pytest.raises(ValueError, match="non-default field layout"): + Struct.from_native_dtype(dtype) + + +def test_packed_structured_dtype_round_trips() -> None: + """ + A packed (default-layout) structured dtype, including nested ones, must continue to round-trip + unchanged. This guards the loud-failure path for aligned dtypes against false positives, and + ensures existing data written with packed layouts keeps working. + """ + for dtype in ( + np.dtype([("a", "i1"), ("b", "i8")]), + np.dtype([("a", " Date: Sat, 12 Sep 2026 17:43:11 +0200 Subject: [PATCH 2/9] fix(dtype): reject unrepresentable structured dtypes and fix nested v2 round trip Nested structured dtypes written with Zarr format 2 could not be read back, because the inner data type name check recursed as if the nested field were a single [name, dtype] pair instead of a list of fields. Structured dtypes that NumPy allows but the Zarr struct metadata cannot record were accepted and rebuilt as something else: aligned or explicitly offset layouts came back packed with a different itemsize, a titled field came back as two fields (the fields mapping lists titles as extra keys), and a subarray field came back as raw bytes. Struct.from_native_dtype now raises ValueError naming the offending field for all three, and iterates over names rather than the fields mapping. Add zdtypes() and structured_dtypes() hypothesis strategies to zarr.testing.strategies, and three property tests: every registered data type round-trips through JSON and through NumPy, and structured dtype resolution either raises or returns the input dtype exactly. The last property is the one every bug above violated. Assisted-by: ClaudeCode:claude-fable-5-1 Co-Authored-By: Claude Fable 5.1 --- changes/188.bugfix.md | 1 + src/zarr/core/dtype/npy/structured.py | 97 ++++++++------ src/zarr/testing/strategies.py | 125 ++++++++++++++++++- tests/test_dtype/test_npy/test_structured.py | 66 ++++++---- tests/test_properties.py | 75 +++++++++++ 5 files changed, 300 insertions(+), 64 deletions(-) create mode 100644 changes/188.bugfix.md diff --git a/changes/188.bugfix.md b/changes/188.bugfix.md new file mode 100644 index 0000000000..8658aa887b --- /dev/null +++ b/changes/188.bugfix.md @@ -0,0 +1 @@ +Structured (``struct``) data types no longer silently corrupt data on the way in or out. Nested structured dtypes written with Zarr format 2 could not be read back, because the inner data type name check rejected the nested list-of-fields form that ``Struct.to_json`` itself produces. Structured dtypes that NumPy allows but the Zarr struct metadata cannot record used to be accepted and rebuilt as something else: aligned or explicitly offset layouts came back packed with a different itemsize, a titled field came back as two fields, and a subarray field came back as raw bytes. ``Struct.from_native_dtype`` now raises a ``ValueError`` naming the offending field for all three. The ``zarr.testing.strategies`` module gains ``zdtypes`` and ``structured_dtypes`` strategies, and the property tests now check that every registered data type round-trips through JSON and NumPy, and that structured dtype resolution either raises or returns the input dtype exactly. diff --git a/src/zarr/core/dtype/npy/structured.py b/src/zarr/core/dtype/npy/structured.py index 68b8e54525..e3082f4f72 100644 --- a/src/zarr/core/dtype/npy/structured.py +++ b/src/zarr/core/dtype/npy/structured.py @@ -2,7 +2,7 @@ from collections.abc import Sequence from dataclasses import dataclass -from typing import TYPE_CHECKING, Any, ClassVar, Literal, Self, TypeGuard, cast, overload +from typing import TYPE_CHECKING, ClassVar, Literal, Self, TypeGuard, cast, overload import numpy as np @@ -30,32 +30,49 @@ StructuredScalarLike = list[object] | tuple[object, ...] | bytes | int -def _is_default_packed(dtype: np.dtype[np.void]) -> bool: +def _check_representable(dtype: np.dtype[np.void]) -> str | None: """ - Check whether a structured numpy dtype uses the default contiguous (packed) field layout. - - The Zarr structured/struct metadata only records ``(name, dtype)`` pairs and reconstructs the - native dtype by packing the fields contiguously (see ``Structured.to_native_dtype``). A dtype - created with ``align=True``, or with explicit field offsets / extra padding, therefore cannot be - represented faithfully: its field offsets and itemsize would silently change on round-trip, - corrupting stored bytes. This function returns ``False`` for any such dtype. - - The check is recursive so that padding within a nested field dtype is detected even when the - outer dtype is itself packed. + Check whether a structured NumPy dtype can be represented by the Zarr struct data type. + + The Zarr struct metadata records only ``(name, dtype)`` pairs and reconstructs the native + dtype by packing those fields contiguously (see ``Structured.to_native_dtype``). Anything + NumPy allows beyond that is lost on the round trip and would silently change how stored + bytes are interpreted. This function rejects three such features, recursing into nested + fields so that a problem inside a nested field dtype is caught even when the outer dtype + is fine: + + - field titles, e.g. ``np.dtype([(("title", "name"), "i4")])`` + - subarray fields, e.g. ``np.dtype([("name", "i4", (2,))])`` + - non-default field layouts, e.g. ``np.dtype(..., align=True)`` or explicit offsets + + Returns + ------- + str | None + ``None`` if the dtype is representable, otherwise a short description of the problem. """ names = dtype.names - if names is None: # pragma: no cover - only called on structured dtypes - return True - repacked_fields: list[tuple[str, np.dtype[Any]]] = [] + 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 = dtype.fields[name][0] # type: ignore[index] - if field_dtype.names is not None and not _is_default_packed(field_dtype): - return False - repacked_fields.append((name, field_dtype)) - repacked = np.dtype(repacked_fields) - if repacked.itemsize != dtype.itemsize: - return False - return all(dtype.fields[n][1] == repacked.fields[n][1] for n in names) # type: ignore[index] + 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 = _check_representable(field_dtype) + if reason is not None: + return f"within field {name!r}: {reason}" + # NumPy dtype equality compares field offsets and itemsize, so rebuilding the dtype from + # its (name, dtype) pairs and comparing detects any padding, alignment or explicit offsets. + repacked = np.dtype([(name, fields[name][0]) for name in names]) + if repacked != dtype: + return ( + "it uses a non-default field layout (e.g. it was created with align=True, or has " + "explicit field offsets or padding)" + ) + return None class StructuredJSON_V2(DTypeConfig_V2[StructuredName_V2, None]): @@ -203,6 +220,10 @@ 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 is a structured dtype that this data type cannot represent faithfully: + one with field titles, subarray fields, or a non-default (aligned, padded or + explicitly offset) field layout. Notes ----- @@ -213,25 +234,25 @@ def from_native_dtype(cls, dtype: TBaseDType) -> Self: fields: list[tuple[str, ZDType[TBaseDType, TBaseScalar]]] = [] if cls._check_native_dtype(dtype): - if not _is_default_packed(dtype): + reason = _check_representable(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 non-packed layout *does* match this dtype - # class -- it simply cannot be represented faithfully -- so we must raise an - # error the registry propagates to the caller. + # "this dtype does not match"), but a dtype with an unrepresentable 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 serialize the structured data type {dtype}. It uses a non-default " - "field layout (e.g. it was created with align=True, or has explicit field " - "offsets or padding), which the Zarr structured data type metadata cannot " - "represent: only the field names and dtypes are stored, and the fields are " - "always packed contiguously on read. Serializing this dtype would silently " - "change its field offsets and itemsize, corrupting stored data. Use a packed " - "structured dtype (without align=True or explicit offsets) instead." + f"Cannot serialize the structured data type {dtype}: {reason}. The Zarr " + "struct data type records only field names and field data types, and " + "fields are always packed contiguously on read, so serializing this dtype " + "would silently change how the stored bytes are interpreted. Use a packed " + "structured dtype without titles, subarray fields, align=True or explicit " + "offsets instead." ) - # 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) + # 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 dtype.names: # type: ignore[union-attr] + dtype_wrapped = get_data_type_from_native_dtype(dtype.fields[key][0]) # type: ignore[index] fields.append((key, dtype_wrapped)) return cls(fields=tuple(fields)) diff --git a/src/zarr/testing/strategies.py b/src/zarr/testing/strategies.py index db01697f1e..2a17b01b4c 100644 --- a/src/zarr/testing/strategies.py +++ b/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,124 @@ 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's generic time unit carries no scale factor, so only ``scale_factor=1`` has a native + representation for it. + """ + 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]]: + """ + A strategy for instances of every registered ``ZDType`` class, including ``Struct`` with + arbitrarily nested fields. + + Struct fields are restricted to fixed-size data types, since the Zarr struct data type cannot + hold variable-length fields. + """ + 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_unrepresentable: bool = False, max_depth: int = 3 +) -> np.dtype[np.void]: + """ + A strategy for native NumPy structured dtypes, flat or nested. + + With ``allow_unrepresentable=False`` (the default) every dtype is packed, has plain field names + and scalar fields, so it can be represented by the Zarr struct data type. With + ``allow_unrepresentable=True`` the strategy also injects the NumPy features that the Zarr + struct data type cannot record: field titles, subarray fields and ``align=True`` layouts. + Each is injected independently at random, so most draws carry at least one and some carry + none. + """ + 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_unrepresentable and draw(st.booleans()): + key = (title, name) + if allow_unrepresentable 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_unrepresentable and draw(st.booleans()) + return np.dtype(specs, align=align) + + return build(0) + + def v3_dtypes() -> st.SearchStrategy[np.dtype[Any]]: return dtypes() diff --git a/tests/test_dtype/test_npy/test_structured.py b/tests/test_dtype/test_npy/test_structured.py index 105d11c5da..bdb49e0a78 100644 --- a/tests/test_dtype/test_npy/test_structured.py +++ b/tests/test_dtype/test_npy/test_structured.py @@ -1,10 +1,11 @@ from __future__ import annotations -from typing import Any +from typing import TYPE_CHECKING, Any import numpy as np import pytest +import zarr from tests.test_dtype.test_wrapper import BaseTestZDType from zarr.core.dtype import ( Float16, @@ -17,6 +18,9 @@ get_data_type_from_json, ) +if TYPE_CHECKING: + from zarr.core.common import ZarrFormat + class TestStruct(BaseTestZDType): """Test the canonical 'struct' dtype format.""" @@ -264,9 +268,12 @@ def test_struct_from_native_dtype() -> None: @pytest.mark.filterwarnings("ignore::zarr.errors.UnstableSpecificationWarning") +@pytest.mark.parametrize("zarr_format", [2, 3]) @pytest.mark.parametrize( "dtype", [ + # flat + np.dtype([("a", "i1"), ("b", "i8")]), # one level of nesting np.dtype([("a", " None: ), ], ) -def test_nested_structured_v2_round_trip(dtype: np.dtype[np.void]) -> None: +def test_packed_structured_dtype_round_trips( + dtype: np.dtype[np.void], zarr_format: ZarrFormat +) -> None: """ - Regression test for nested structured dtypes failing the Zarr V2 JSON round-trip. + A packed (default-layout) structured dtype, flat or nested, round-trips unchanged through + both the JSON form and the native form. - ``Struct.to_json(zarr_format=2)`` emits a nested field as ``[name, [[sub, dt], ...]]``, and - ``get_data_type_from_json`` must be able to read that form back. Previously the inner type - guard recursed as a single ``[name, dtype]`` pair and rejected the list-of-fields form, + Nested dtypes are the regression case: ``Struct.to_json(zarr_format=2)`` emits a nested + field as ``[name, [[sub, dt], ...]]``, and the inner V2 type guard used to reject that form, so Zarr wrote V2 metadata it could not read back. """ zdtype = Struct.from_native_dtype(dtype) - json_v2 = zdtype.to_json(zarr_format=2) - recovered = get_data_type_from_json(json_v2, zarr_format=2) + recovered = get_data_type_from_json( + zdtype.to_json(zarr_format=zarr_format), zarr_format=zarr_format + ) assert recovered == zdtype assert recovered.to_native_dtype() == dtype + assert recovered.to_native_dtype().itemsize == dtype.itemsize @pytest.mark.filterwarnings("ignore::zarr.errors.UnstableSpecificationWarning") def test_nested_structured_v2_array_round_trip() -> None: """End-to-end test: write and read a Zarr V2 array with a nested structured dtype.""" - import zarr - dtype = np.dtype([("a", " None: np.dtype([("a", "i1"), ("b", "i8")], align=True), # outer layout is packed, but a nested field dtype carries padding np.dtype([("a", "i8"), ("nested", np.dtype([("x", "i1"), ("y", "i8")], align=True))]), + # explicit offsets leave a gap without align=True + np.dtype({"names": ["a", "b"], "formats": ["i1", "i1"], "offsets": [0, 4], "itemsize": 8}), ], ) def test_padded_structured_dtype_raises(dtype: np.dtype[np.void]) -> None: """ - Regression test: structured dtypes with non-default (padded / aligned) field layouts must + Structured dtypes with non-default (padded / aligned / explicitly offset) field layouts must fail loudly rather than silently dropping the padding. - The Zarr structured metadata records only ``(name, dtype)`` pairs and re-packs fields - contiguously on read, so an aligned dtype would round-trip to a smaller itemsize, silently - corrupting stored chunk bytes. ``from_native_dtype`` detects this and raises instead. + The Zarr struct metadata records only ``(name, dtype)`` pairs and re-packs fields + contiguously on read, so a padded dtype would round-trip to a different itemsize and + silently misinterpret stored chunk bytes. """ with pytest.raises(ValueError, match="non-default field layout"): Struct.from_native_dtype(dtype) -def test_packed_structured_dtype_round_trips() -> None: +def test_titled_structured_dtype_raises() -> None: """ - A packed (default-layout) structured dtype, including nested ones, must continue to round-trip - unchanged. This guards the loud-failure path for aligned dtypes against false positives, and - ensures existing data written with packed layouts keeps working. + A structured dtype with a field title must be rejected. NumPy's ``fields`` mapping lists the + title as an extra key, so a titled field used to be read back as two separate fields. """ - for dtype in ( - np.dtype([("a", "i1"), ("b", "i8")]), - np.dtype([("a", " None: + """ + A structured dtype with a subarray field must be rejected. The subarray dtype used to be + resolved as raw bytes, silently dropping its shape and element type. + """ + dtype = np.dtype([("f0", "i4", (2,))]) + with pytest.raises(ValueError, match="field 'f0' is a subarray"): + Struct.from_native_dtype(dtype) diff --git a/tests/test_properties.py b/tests/test_properties.py index 2794ad3cb0..831c4abeef 100644 --- a/tests/test_properties.py +++ b/tests/test_properties.py @@ -19,6 +19,10 @@ from zarr.abc.store import Store from zarr.core.common import ZARR_JSON, ZARRAY_JSON, ZATTRS_JSON +from zarr.core.dtype import get_data_type_from_json, get_data_type_from_native_dtype +from zarr.core.dtype.common import HasItemSize +from zarr.core.dtype.npy.structured import Struct +from zarr.core.dtype.wrapper import ZDType from zarr.core.metadata import ArrayV2Metadata, ArrayV3Metadata from zarr.core.sync import sync from zarr.testing.strategies import ( @@ -34,7 +38,9 @@ sharded_arrays, simple_arrays, stores, + structured_dtypes, zarr_formats, + zdtypes, ) @@ -361,6 +367,75 @@ def test_roundtrip_array_metadata_from_json(data: st.DataObject, zarr_format: in assert deep_equal(orig, rt), f"Roundtrip mismatch:\nOriginal: {orig}\nRoundtripped: {rt}" +def _struct_depth(zdtype: ZDType[Any, Any]) -> int: + if not isinstance(zdtype, Struct): + return 0 + return 1 + max(_struct_depth(field_dtype) for _, field_dtype in zdtype.fields) + + +@given(zdtype=zdtypes(), zarr_format=zarr_formats) +@pytest.mark.filterwarnings("ignore::zarr.core.dtype.common.UnstableSpecificationWarning") +def test_zdtype_json_roundtrip(zdtype: ZDType[Any, Any], zarr_format: int) -> None: + """ + Every registered data type, including arbitrarily nested structs, survives a round trip + through its JSON form for both Zarr formats. + + Zarr format 3 data type names do not carry endianness (the bytes codec does), so for that + format the JSON form is compared instead of the data type instance. + """ + event(f"dtype={type(zdtype).__name__}") + event(f"struct_depth={_struct_depth(zdtype)}") + as_json = zdtype.to_json(zarr_format=zarr_format) # type: ignore[arg-type] + roundtripped = get_data_type_from_json(as_json, zarr_format=zarr_format) + assert roundtripped.to_json(zarr_format=zarr_format) == as_json # type: ignore[arg-type] + if zarr_format == 2: + assert roundtripped == zdtype + + +@given(zdtype=zdtypes()) +@pytest.mark.filterwarnings("ignore::zarr.core.dtype.common.UnstableSpecificationWarning") +def test_zdtype_native_roundtrip(zdtype: ZDType[Any, Any]) -> None: + """ + Every registered data type survives a round trip through its native NumPy dtype, and its + reported item size matches the native dtype's itemsize. + + The NumPy object dtype is shared by several Zarr data types, so resolving it is ambiguous by + design and must raise instead. + """ + event(f"dtype={type(zdtype).__name__}") + native = zdtype.to_native_dtype() + if native.kind == "O": + event("native=object") + with pytest.raises(ValueError, match="ambiguous"): + get_data_type_from_native_dtype(native) + return + roundtripped = get_data_type_from_native_dtype(native) + assert roundtripped == zdtype + assert roundtripped.to_native_dtype() == native + if isinstance(zdtype, HasItemSize): + assert zdtype.item_size == native.itemsize + + +@given(dtype=structured_dtypes(allow_unrepresentable=True)) +@pytest.mark.filterwarnings("ignore::zarr.core.dtype.common.UnstableSpecificationWarning") +def test_structured_dtype_never_silently_changes(dtype: np.dtype[np.void]) -> None: + """ + For any native structured dtype, including ones with field titles, subarray fields or + aligned layouts, resolving a Zarr data type either raises ``ValueError`` or yields a data type + whose native form is exactly the input: same field names, offsets and itemsize. + + This is the property that the silent-corruption bugs in structured dtype handling violated: + a dtype was accepted but came back with different fields, offsets or itemsize. + """ + try: + zdtype = get_data_type_from_native_dtype(dtype) + except ValueError: + event("outcome=rejected") + return + event("outcome=accepted") + assert zdtype.to_native_dtype() == dtype + + # @st.composite # def advanced_indices(draw, *, shape): # basic_idxr = draw( From 0395360aab5524348fcba0e95e1ead42253a5ad7 Mon Sep 17 00:00:00 2001 From: Davis Vann Bennett Date: Sat, 12 Sep 2026 17:49:09 +0200 Subject: [PATCH 3/9] chore: renumber changelog fragment to upstream PR 4340 Assisted-by: ClaudeCode:claude-fable-5-1 Co-Authored-By: Claude Fable 5.1 --- changes/{188.bugfix.md => 4340.bugfix.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changes/{188.bugfix.md => 4340.bugfix.md} (100%) diff --git a/changes/188.bugfix.md b/changes/4340.bugfix.md similarity index 100% rename from changes/188.bugfix.md rename to changes/4340.bugfix.md From 404979915e5c2fc3c0ac2bd7763e325deaedbf97 Mon Sep 17 00:00:00 2001 From: Davis Vann Bennett Date: Sat, 12 Sep 2026 17:53:41 +0200 Subject: [PATCH 4/9] =?UTF-8?q?fix(dtype):=20reject=20scaled=20generic=20t?= =?UTF-8?q?ime=20units=20and=20normalize=20the=20=CE=BCs=20alias?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DateTime64 and TimeDelta64 accepted two values that did not survive a round trip through NumPy: unit="generic" with scale_factor != 1 (NumPy's generic time type carries no scale, so np.dtype silently dropped it and the Zarr V2 dtype string lost it while the V3 metadata kept it), and unit="μs" (NumPy only reports "us", so the instance compared unequal to itself after to_native_dtype/from_native_dtype). The constructor now raises for the first and stores "us" for the second; metadata spelling the unit "μs" still reads. Assisted-by: ClaudeCode:claude-fable-5-1 Co-Authored-By: Claude Fable 5.1 --- changes/+time-units.bugfix.md | 1 + src/zarr/core/dtype/npy/time.py | 12 +++++++++ tests/test_dtype/test_npy/test_time.py | 35 ++++++++++++++++++++++++++ 3 files changed, 48 insertions(+) create mode 100644 changes/+time-units.bugfix.md diff --git a/changes/+time-units.bugfix.md b/changes/+time-units.bugfix.md new file mode 100644 index 0000000000..f5b687fba5 --- /dev/null +++ b/changes/+time-units.bugfix.md @@ -0,0 +1 @@ +`DateTime64` and `TimeDelta64` now reject the `generic` unit combined with a `scale_factor` other than 1, and store the `μs` unit under NumPy's `us` spelling. Previously both values were accepted but did not survive a round trip: NumPy has no scale for the generic unit, so `DateTime64(unit="generic", scale_factor=2).to_native_dtype()` silently became plain `datetime64` (and the Zarr V2 dtype string dropped the scale factor while the V3 metadata kept it), and `DateTime64(unit="μs")` came back from NumPy as `DateTime64(unit="us")`, which compared unequal to the original. Metadata that spells the unit `μs` still reads correctly. diff --git a/src/zarr/core/dtype/npy/time.py b/src/zarr/core/dtype/npy/time.py index 4efa0be7bb..21a02a7e2a 100644 --- a/src/zarr/core/dtype/npy/time.py +++ b/src/zarr/core/dtype/npy/time.py @@ -233,6 +233,18 @@ def __post_init__(self) -> None: raise ValueError(f"scale_factor must be < 2147483648, got {self.scale_factor}.") if self.unit not in get_args(DateTimeUnit): raise ValueError(f"unit must be one of {get_args(DateTimeUnit)}, got {self.unit!r}.") + if self.unit == "μs": + # NumPy spells the microsecond unit "us" and resolves "μs" to it, so an + # instance built with "μs" would not round-trip through to_native_dtype(). + # Store the NumPy spelling; "μs" stays accepted as input and in stored metadata. + object.__setattr__(self, "unit", "us") + if self.unit == "generic" and self.scale_factor != 1: + # NumPy's generic (unit-less) time type carries no scale, so a scale factor + # other than 1 is silently dropped by np.dtype and by the Zarr V2 dtype string. + raise ValueError( + f"The 'generic' unit does not take a scale factor, got scale_factor={self.scale_factor}. " + "Use scale_factor=1 with the 'generic' unit." + ) @classmethod def from_native_dtype(cls, dtype: TBaseDType) -> Self: diff --git a/tests/test_dtype/test_npy/test_time.py b/tests/test_dtype/test_npy/test_time.py index 67ba3bd130..5ad1cff6e4 100644 --- a/tests/test_dtype/test_npy/test_time.py +++ b/tests/test_dtype/test_npy/test_time.py @@ -43,6 +43,7 @@ class TestDateTime64(_TestTimeBase): valid_json_v3 = ( {"name": "numpy.datetime64", "configuration": {"unit": "ns", "scale_factor": 10}}, {"name": "numpy.datetime64", "configuration": {"unit": "us", "scale_factor": 1}}, + {"name": "numpy.datetime64", "configuration": {"unit": "generic", "scale_factor": 1}}, ) invalid_json_v2 = ( "datetime64", @@ -93,6 +94,7 @@ class TestTimeDelta64(_TestTimeBase): valid_json_v3 = ( {"name": "numpy.timedelta64", "configuration": {"unit": "ns", "scale_factor": 10}}, {"name": "numpy.timedelta64", "configuration": {"unit": "us", "scale_factor": 1}}, + {"name": "numpy.timedelta64", "configuration": {"unit": "generic", "scale_factor": 1}}, ) invalid_json_v2 = ( "timedelta64", @@ -166,6 +168,39 @@ def test_time_scale_factor_too_high() -> None: TimeDelta64(scale_factor=scale_factor) +def test_time_generic_unit_rejects_scale_factor() -> None: + """ + Test that the 'generic' unit with a scale factor other than 1 raises a ValueError. + + NumPy's generic time unit has no scale, so ``np.dtype("M8[2generic]")`` silently drops + the 2 and the value would not survive ``to_native_dtype`` or the Zarr V2 dtype string. + """ + scale_factor = 2 + msg = f"The 'generic' unit does not take a scale factor, got scale_factor={scale_factor}." + with pytest.raises(ValueError, match=re.escape(msg)): + DateTime64(unit="generic", scale_factor=scale_factor) + with pytest.raises(ValueError, match=re.escape(msg)): + TimeDelta64(unit="generic", scale_factor=scale_factor) + + +@pytest.mark.parametrize("cls", [DateTime64, TimeDelta64]) +def test_time_microsecond_alias_normalized(cls: type[DateTime64 | TimeDelta64]) -> None: + """ + Test that the 'μs' unit is stored as NumPy's 'us' spelling. + + The two spellings are equivalent, but NumPy only ever reports 'us', so an instance + that kept 'μs' would compare unequal to itself after a trip through NumPy. Stored + metadata may still spell the unit 'μs' and reads back as the normalized instance. + """ + zdtype = cls(unit="μs", scale_factor=3) + assert zdtype.unit == "us" + assert zdtype == cls(unit="us", scale_factor=3) + assert cls.from_native_dtype(zdtype.to_native_dtype()) == zdtype + json_v3 = {"name": cls._zarr_v3_name, "configuration": {"unit": "μs", "scale_factor": 3}} + assert cls.from_json(json_v3, zarr_format=3) == zdtype + assert zdtype.to_json(zarr_format=3)["configuration"]["unit"] == "us" + + @pytest.mark.parametrize("unit", get_args(DateTimeUnit)) @pytest.mark.parametrize("scale_factor", [1, 10]) @pytest.mark.parametrize("value", [0, 1, 10]) From f213d9ad27aa45f875c7094d8f71e3193c31e536 Mon Sep 17 00:00:00 2001 From: Davis Vann Bennett Date: Sat, 12 Sep 2026 17:53:45 +0200 Subject: [PATCH 5/9] fix(zarr-metadata): add numpy time configuration validators MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add numpy_datetime64_configuration, numpy_timedelta64_configuration and the shared numpy_time_unit validator, following the leaf-validator precedent (hex_float16, raw_bytes_dtype_name) for constraints the configuration TypedDicts cannot express: scale_factor must be an integer in [1, 2**31 - 1], the generic unit takes scale_factor 1 only, and the equivalent "μs" spelling of the microsecond unit is normalized to "us". The unit vocabulary, previously duplicated in both leaf modules, moves to a private shared module and is re-exported unchanged. Assisted-by: ClaudeCode:claude-fable-5-1 Co-Authored-By: Claude Fable 5.1 --- .../changes/+time-units.bugfix.md | 1 + .../zarr_metadata/v3/data_type/_numpy_time.py | 100 ++++++++++++++++++ .../v3/data_type/numpy_datetime64.py | 30 +++++- .../v3/data_type/numpy_timedelta64.py | 52 ++++----- .../numpy_datetime64/test_validators.py | 66 ++++++++++++ .../numpy_timedelta64/test_validators.py | 66 ++++++++++++ 6 files changed, 286 insertions(+), 29 deletions(-) create mode 100644 packages/zarr-metadata/changes/+time-units.bugfix.md create mode 100644 packages/zarr-metadata/src/zarr_metadata/v3/data_type/_numpy_time.py create mode 100644 packages/zarr-metadata/tests/v3/data_type/numpy_datetime64/test_validators.py create mode 100644 packages/zarr-metadata/tests/v3/data_type/numpy_timedelta64/test_validators.py diff --git a/packages/zarr-metadata/changes/+time-units.bugfix.md b/packages/zarr-metadata/changes/+time-units.bugfix.md new file mode 100644 index 0000000000..99255143e6 --- /dev/null +++ b/packages/zarr-metadata/changes/+time-units.bugfix.md @@ -0,0 +1 @@ +Added `numpy_datetime64_configuration` and `numpy_timedelta64_configuration` validators (and the shared `numpy_time_unit`) to the `numpy.datetime64` and `numpy.timedelta64` leaf modules. They enforce the constraints the configuration TypedDicts cannot express: `scale_factor` must be an integer in `[1, 2**31 - 1]`, the `generic` unit must have `scale_factor` 1 because NumPy's generic time type carries no scale, and the equivalent `μs` spelling of the microsecond unit is normalized to `us`. diff --git a/packages/zarr-metadata/src/zarr_metadata/v3/data_type/_numpy_time.py b/packages/zarr-metadata/src/zarr_metadata/v3/data_type/_numpy_time.py new file mode 100644 index 0000000000..caf3f68283 --- /dev/null +++ b/packages/zarr-metadata/src/zarr_metadata/v3/data_type/_numpy_time.py @@ -0,0 +1,100 @@ +""" +Vocabulary and validation shared by the `numpy.datetime64` and `numpy.timedelta64` data types. + +This module is private (underscore-prefixed); the public names are re-exported by +`zarr_metadata.v3.data_type.numpy_datetime64` and +`zarr_metadata.v3.data_type.numpy_timedelta64`. +""" + +from collections.abc import Mapping +from typing import Final, Literal, cast + +NumpyTimeUnit = Literal[ + "Y", "M", "W", "D", "h", "m", "s", "ms", "us", "μs", "ns", "ps", "fs", "as", "generic" +] +"""Time unit codes used by numpy.datetime64 and numpy.timedelta64.""" + +NUMPY_TIME_UNIT: Final = ( + "Y", + "M", + "W", + "D", + "h", + "m", + "s", + "ms", + "us", + "μs", + "ns", + "ps", + "fs", + "as", + "generic", +) +"""Runtime tuple of the permitted `numpy.timedelta64`/`numpy.datetime64` unit strings.""" + +MAX_NUMPY_TIME_SCALE_FACTOR: Final = 2**31 - 1 +"""The largest `scale_factor` NumPy accepts for a datetime64 or timedelta64 dtype.""" + +_CONFIGURATION_KEYS: Final = frozenset({"unit", "scale_factor"}) + + +def numpy_time_unit(value: str) -> NumpyTimeUnit: + """Validate `value` as a NumPy time unit and return its canonical spelling. + + The spec lists `"us"` and `"μs"` as equivalent spellings of the microsecond + unit; NumPy itself only ever reports `"us"`, so `"μs"` is returned as `"us"`. + + Raises ValueError if `value` is not one of `NUMPY_TIME_UNIT`. + """ + if value not in NUMPY_TIME_UNIT: + raise ValueError(f"Expected one of {NUMPY_TIME_UNIT}, got {value!r}") + if value == "μs": + return "us" + return cast("NumpyTimeUnit", value) + + +def numpy_time_configuration(value: Mapping[str, object]) -> tuple[NumpyTimeUnit, int]: + """Validate a `numpy.datetime64` / `numpy.timedelta64` configuration object. + + Returns the `(unit, scale_factor)` pair with the unit in its canonical spelling + (see `numpy_time_unit`). + + Raises TypeError if `unit` is not a string or `scale_factor` is not an + integer. Raises ValueError if the object has keys other than exactly `unit` + and `scale_factor`, if `unit` is not a `NumpyTimeUnit`, if `scale_factor` is + outside `[1, MAX_NUMPY_TIME_SCALE_FACTOR]`, or if the unit is `"generic"` + with a `scale_factor` other than 1. NumPy's generic (unit-less) time type + carries no scale, so any other scale factor would be silently dropped when + the data type is materialized. + """ + keys = frozenset(value) + if keys != _CONFIGURATION_KEYS: + raise ValueError( + f"Expected exactly the keys {sorted(_CONFIGURATION_KEYS)}, got {sorted(keys)}" + ) + raw_unit = value["unit"] + if not isinstance(raw_unit, str): + raise TypeError(f"Expected 'unit' to be a string, got {raw_unit!r}") + unit = numpy_time_unit(raw_unit) + scale_factor = value["scale_factor"] + if isinstance(scale_factor, bool) or not isinstance(scale_factor, int): + raise TypeError(f"Expected 'scale_factor' to be an integer, got {scale_factor!r}") + if not 1 <= scale_factor <= MAX_NUMPY_TIME_SCALE_FACTOR: + raise ValueError( + f"Expected 'scale_factor' in [1, {MAX_NUMPY_TIME_SCALE_FACTOR}], got {scale_factor}" + ) + if unit == "generic" and scale_factor != 1: + raise ValueError( + f"The 'generic' unit does not take a scale factor, got scale_factor={scale_factor}" + ) + return unit, scale_factor + + +__all__ = [ + "MAX_NUMPY_TIME_SCALE_FACTOR", + "NUMPY_TIME_UNIT", + "NumpyTimeUnit", + "numpy_time_configuration", + "numpy_time_unit", +] diff --git a/packages/zarr-metadata/src/zarr_metadata/v3/data_type/numpy_datetime64.py b/packages/zarr-metadata/src/zarr_metadata/v3/data_type/numpy_datetime64.py index 8784160f71..953772e0d4 100644 --- a/packages/zarr-metadata/src/zarr_metadata/v3/data_type/numpy_datetime64.py +++ b/packages/zarr-metadata/src/zarr_metadata/v3/data_type/numpy_datetime64.py @@ -4,21 +4,23 @@ See https://github.com/zarr-developers/zarr-extensions/tree/main/data-types/numpy.datetime64 """ +from collections.abc import Mapping from typing import Final, Literal from typing_extensions import ReadOnly, TypedDict +from zarr_metadata.v3.data_type._numpy_time import ( + NumpyTimeUnit, + numpy_time_configuration, + numpy_time_unit, +) + NUMPY_DATETIME64_DATA_TYPE_NAME: Final = "numpy.datetime64" """The `name` field value of the `numpy.datetime64` data type.""" NumpyDatetime64DataTypeName = Literal["numpy.datetime64"] """Literal type of the `name` field of the `numpy.datetime64` data type.""" -NumpyTimeUnit = Literal[ - "Y", "M", "W", "D", "h", "m", "s", "ms", "us", "μs", "ns", "ps", "fs", "as", "generic" -] -"""Time unit codes used by numpy.datetime64.""" - class NumpyDatetime64Configuration(TypedDict): """ @@ -36,6 +38,22 @@ class NumpyDatetime64Configuration(TypedDict): scale_factor: ReadOnly[int] +def numpy_datetime64_configuration(value: Mapping[str, object]) -> NumpyDatetime64Configuration: + """Validate `value` as a `numpy.datetime64` configuration and normalize it. + + The returned configuration spells the microsecond unit `"us"` even when the + input used the equivalent `"μs"`. + + Raises TypeError if `unit` is not a string or `scale_factor` is not an + integer. Raises ValueError if `value` does not have exactly the keys `unit` + and `scale_factor`, if `unit` is not a `NumpyTimeUnit`, if `scale_factor` is + outside `[1, 2**31 - 1]`, or if the `"generic"` unit is combined with a + `scale_factor` other than 1 (NumPy's generic time type has no scale). + """ + unit, scale_factor = numpy_time_configuration(value) + return {"unit": unit, "scale_factor": scale_factor} + + class NumpyDatetime64(TypedDict): """`numpy.datetime64` data type metadata.""" @@ -57,4 +75,6 @@ class NumpyDatetime64(TypedDict): "NumpyDatetime64DataTypeName", "NumpyDatetime64FillValue", "NumpyTimeUnit", + "numpy_datetime64_configuration", + "numpy_time_unit", ] diff --git a/packages/zarr-metadata/src/zarr_metadata/v3/data_type/numpy_timedelta64.py b/packages/zarr-metadata/src/zarr_metadata/v3/data_type/numpy_timedelta64.py index f5c8c77bf8..2180d61c73 100644 --- a/packages/zarr-metadata/src/zarr_metadata/v3/data_type/numpy_timedelta64.py +++ b/packages/zarr-metadata/src/zarr_metadata/v3/data_type/numpy_timedelta64.py @@ -4,40 +4,24 @@ See https://github.com/zarr-developers/zarr-extensions/tree/main/data-types/numpy.timedelta64 """ +from collections.abc import Mapping from typing import Final, Literal from typing_extensions import ReadOnly, TypedDict +from zarr_metadata.v3.data_type._numpy_time import ( + NUMPY_TIME_UNIT, + NumpyTimeUnit, + numpy_time_configuration, + numpy_time_unit, +) + NUMPY_TIMEDELTA64_DATA_TYPE_NAME: Final = "numpy.timedelta64" """The `name` field value of the `numpy.timedelta64` data type.""" NumpyTimedelta64DataTypeName = Literal["numpy.timedelta64"] """Literal type of the `name` field of the `numpy.timedelta64` data type.""" -NumpyTimeUnit = Literal[ - "Y", "M", "W", "D", "h", "m", "s", "ms", "us", "μs", "ns", "ps", "fs", "as", "generic" -] -"""Time unit codes used by numpy.timedelta64.""" - -NUMPY_TIME_UNIT: Final = ( - "Y", - "M", - "W", - "D", - "h", - "m", - "s", - "ms", - "us", - "μs", - "ns", - "ps", - "fs", - "as", - "generic", -) -"""Runtime tuple of the permitted `numpy.timedelta64`/`numpy.datetime64` unit strings.""" - class NumpyTimedelta64Configuration(TypedDict): """ @@ -55,6 +39,24 @@ class NumpyTimedelta64Configuration(TypedDict): scale_factor: ReadOnly[int] +def numpy_timedelta64_configuration( + value: Mapping[str, object], +) -> NumpyTimedelta64Configuration: + """Validate `value` as a `numpy.timedelta64` configuration and normalize it. + + The returned configuration spells the microsecond unit `"us"` even when the + input used the equivalent `"μs"`. + + Raises TypeError if `unit` is not a string or `scale_factor` is not an + integer. Raises ValueError if `value` does not have exactly the keys `unit` + and `scale_factor`, if `unit` is not a `NumpyTimeUnit`, if `scale_factor` is + outside `[1, 2**31 - 1]`, or if the `"generic"` unit is combined with a + `scale_factor` other than 1 (NumPy's generic time type has no scale). + """ + unit, scale_factor = numpy_time_configuration(value) + return {"unit": unit, "scale_factor": scale_factor} + + class NumpyTimedelta64(TypedDict): """`numpy.timedelta64` data type metadata.""" @@ -77,4 +79,6 @@ class NumpyTimedelta64(TypedDict): "NumpyTimedelta64Configuration", "NumpyTimedelta64DataTypeName", "NumpyTimedelta64FillValue", + "numpy_time_unit", + "numpy_timedelta64_configuration", ] diff --git a/packages/zarr-metadata/tests/v3/data_type/numpy_datetime64/test_validators.py b/packages/zarr-metadata/tests/v3/data_type/numpy_datetime64/test_validators.py new file mode 100644 index 0000000000..c18006fcc2 --- /dev/null +++ b/packages/zarr-metadata/tests/v3/data_type/numpy_datetime64/test_validators.py @@ -0,0 +1,66 @@ +"""Cover the `numpy_datetime64_configuration` validator. + +The pydantic-driven fixture tests only check the structural shape of a +configuration; the constraints that tie `unit` and `scale_factor` together +live in the validator function and are covered directly here. +""" + +from __future__ import annotations + +import pytest + +from zarr_metadata.v3.data_type.numpy_datetime64 import numpy_datetime64_configuration + +# (input, expected normalized output) +VALID = [ + ({"unit": "ns", "scale_factor": 1}, {"unit": "ns", "scale_factor": 1}), + ({"unit": "s", "scale_factor": 10}, {"unit": "s", "scale_factor": 10}), + ({"unit": "generic", "scale_factor": 1}, {"unit": "generic", "scale_factor": 1}), + ({"unit": "us", "scale_factor": 2}, {"unit": "us", "scale_factor": 2}), + ({"unit": "μs", "scale_factor": 2}, {"unit": "us", "scale_factor": 2}), + ({"unit": "Y", "scale_factor": 2**31 - 1}, {"unit": "Y", "scale_factor": 2**31 - 1}), +] + + +@pytest.mark.parametrize(("value", "expected"), VALID, ids=lambda x: str(x)) +def test_valid(value: dict[str, object], expected: dict[str, object]) -> None: + assert numpy_datetime64_configuration(value) == expected + + +@pytest.mark.parametrize( + "value", + [{}, {"unit": "s"}, {"scale_factor": 1}, {"unit": "s", "scale_factor": 1, "extra": 0}], + ids=lambda x: str(x), +) +def test_wrong_keys(value: dict[str, object]) -> None: + with pytest.raises(ValueError, match="Expected exactly the keys"): + numpy_datetime64_configuration(value) + + +@pytest.mark.parametrize("unit", [1, None], ids=str) +def test_unit_not_a_string(unit: object) -> None: + with pytest.raises(TypeError, match="Expected 'unit' to be a string"): + numpy_datetime64_configuration({"unit": unit, "scale_factor": 1}) + + +@pytest.mark.parametrize("unit", ["", "invalid", "US", "seconds"], ids=str) +def test_unknown_unit(unit: str) -> None: + with pytest.raises(ValueError, match="Expected one of"): + numpy_datetime64_configuration({"unit": unit, "scale_factor": 1}) + + +@pytest.mark.parametrize("scale_factor", [1.0, "1", True, None], ids=str) +def test_scale_factor_not_an_integer(scale_factor: object) -> None: + with pytest.raises(TypeError, match="Expected 'scale_factor' to be an integer"): + numpy_datetime64_configuration({"unit": "s", "scale_factor": scale_factor}) + + +@pytest.mark.parametrize("scale_factor", [-1, 0, 2**31], ids=str) +def test_scale_factor_out_of_range(scale_factor: int) -> None: + with pytest.raises(ValueError, match=r"Expected 'scale_factor' in \[1, 2147483647\]"): + numpy_datetime64_configuration({"unit": "s", "scale_factor": scale_factor}) + + +def test_generic_unit_rejects_scale_factor() -> None: + with pytest.raises(ValueError, match="'generic' unit does not take a scale factor"): + numpy_datetime64_configuration({"unit": "generic", "scale_factor": 2}) diff --git a/packages/zarr-metadata/tests/v3/data_type/numpy_timedelta64/test_validators.py b/packages/zarr-metadata/tests/v3/data_type/numpy_timedelta64/test_validators.py new file mode 100644 index 0000000000..e301292a7d --- /dev/null +++ b/packages/zarr-metadata/tests/v3/data_type/numpy_timedelta64/test_validators.py @@ -0,0 +1,66 @@ +"""Cover the `numpy_timedelta64_configuration` validator. + +The pydantic-driven fixture tests only check the structural shape of a +configuration; the constraints that tie `unit` and `scale_factor` together +live in the validator function and are covered directly here. +""" + +from __future__ import annotations + +import pytest + +from zarr_metadata.v3.data_type.numpy_timedelta64 import numpy_timedelta64_configuration + +# (input, expected normalized output) +VALID = [ + ({"unit": "ns", "scale_factor": 1}, {"unit": "ns", "scale_factor": 1}), + ({"unit": "s", "scale_factor": 10}, {"unit": "s", "scale_factor": 10}), + ({"unit": "generic", "scale_factor": 1}, {"unit": "generic", "scale_factor": 1}), + ({"unit": "us", "scale_factor": 2}, {"unit": "us", "scale_factor": 2}), + ({"unit": "μs", "scale_factor": 2}, {"unit": "us", "scale_factor": 2}), + ({"unit": "Y", "scale_factor": 2**31 - 1}, {"unit": "Y", "scale_factor": 2**31 - 1}), +] + + +@pytest.mark.parametrize(("value", "expected"), VALID, ids=lambda x: str(x)) +def test_valid(value: dict[str, object], expected: dict[str, object]) -> None: + assert numpy_timedelta64_configuration(value) == expected + + +@pytest.mark.parametrize( + "value", + [{}, {"unit": "s"}, {"scale_factor": 1}, {"unit": "s", "scale_factor": 1, "extra": 0}], + ids=lambda x: str(x), +) +def test_wrong_keys(value: dict[str, object]) -> None: + with pytest.raises(ValueError, match="Expected exactly the keys"): + numpy_timedelta64_configuration(value) + + +@pytest.mark.parametrize("unit", [1, None], ids=str) +def test_unit_not_a_string(unit: object) -> None: + with pytest.raises(TypeError, match="Expected 'unit' to be a string"): + numpy_timedelta64_configuration({"unit": unit, "scale_factor": 1}) + + +@pytest.mark.parametrize("unit", ["", "invalid", "US", "seconds"], ids=str) +def test_unknown_unit(unit: str) -> None: + with pytest.raises(ValueError, match="Expected one of"): + numpy_timedelta64_configuration({"unit": unit, "scale_factor": 1}) + + +@pytest.mark.parametrize("scale_factor", [1.0, "1", True, None], ids=str) +def test_scale_factor_not_an_integer(scale_factor: object) -> None: + with pytest.raises(TypeError, match="Expected 'scale_factor' to be an integer"): + numpy_timedelta64_configuration({"unit": "s", "scale_factor": scale_factor}) + + +@pytest.mark.parametrize("scale_factor", [-1, 0, 2**31], ids=str) +def test_scale_factor_out_of_range(scale_factor: int) -> None: + with pytest.raises(ValueError, match=r"Expected 'scale_factor' in \[1, 2147483647\]"): + numpy_timedelta64_configuration({"unit": "s", "scale_factor": scale_factor}) + + +def test_generic_unit_rejects_scale_factor() -> None: + with pytest.raises(ValueError, match="'generic' unit does not take a scale factor"): + numpy_timedelta64_configuration({"unit": "generic", "scale_factor": 2}) From 339c2e052c775e0f66274b3a6cf16ae0b27c9a2a Mon Sep 17 00:00:00 2001 From: Davis Vann Bennett Date: Sat, 12 Sep 2026 17:55:04 +0200 Subject: [PATCH 6/9] test: drop the time-unit workarounds from the zdtype strategy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The DateTime64/TimeDelta64 constructor now normalizes "μs" and rejects a scaled generic unit, so the strategy draws every unit and conditions the scale factor on the unit instead of filtering and post-hoc rewriting. Assisted-by: ClaudeCode:claude-fable-5-1 Co-Authored-By: Claude Fable 5.1 --- src/zarr/testing/strategies.py | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/zarr/testing/strategies.py b/src/zarr/testing/strategies.py index 2a17b01b4c..dec29fddcc 100644 --- a/src/zarr/testing/strategies.py +++ b/src/zarr/testing/strategies.py @@ -95,23 +95,21 @@ def _leaf_zdtypes(cls: type[ZDType[TBaseDType, TBaseScalar]]) -> SearchStrategy[ 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) + # The constructor normalizes the "μs" alias to "us", so every unit is safe to draw, but + # the generic unit only accepts scale_factor=1, so the scale factor depends on the unit. + return st.sampled_from(DATETIME_UNIT).flatmap( + lambda unit: st.builds( + cls, + unit=st.just(unit), + scale_factor=st.just(1) + if unit == "generic" + else st.integers(min_value=1, max_value=2**31 - 1), + **kwargs, + ) + ) return st.builds(cls, **kwargs) -def _normalize_generic_scale_factor(zdtype: Any) -> Any: - """ - NumPy's generic time unit carries no scale factor, so only ``scale_factor=1`` has a native - representation for it. - """ - 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]]: From 40a62dff2d105e9a0092f032cec47d2c5994c9d6 Mon Sep 17 00:00:00 2001 From: Davis Vann Bennett Date: Sat, 12 Sep 2026 17:57:37 +0200 Subject: [PATCH 7/9] chore: renumber changelog fragments to upstream PR 4342 Assisted-by: ClaudeCode:claude-fable-5-1 Co-Authored-By: Claude Fable 5.1 --- changes/{+time-units.bugfix.md => 4342.bugfix.md} | 0 .../changes/{+time-units.bugfix.md => 4342.bugfix.md} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename changes/{+time-units.bugfix.md => 4342.bugfix.md} (100%) rename packages/zarr-metadata/changes/{+time-units.bugfix.md => 4342.bugfix.md} (100%) diff --git a/changes/+time-units.bugfix.md b/changes/4342.bugfix.md similarity index 100% rename from changes/+time-units.bugfix.md rename to changes/4342.bugfix.md diff --git a/packages/zarr-metadata/changes/+time-units.bugfix.md b/packages/zarr-metadata/changes/4342.bugfix.md similarity index 100% rename from packages/zarr-metadata/changes/+time-units.bugfix.md rename to packages/zarr-metadata/changes/4342.bugfix.md From e3def93d07a53161be34b52a577bd8fbebd5a770 Mon Sep 17 00:00:00 2001 From: Davis Vann Bennett Date: Sun, 13 Sep 2026 17:34:42 +0200 Subject: [PATCH 8/9] docs(dtype): distinguish serialization limits from format constraints Assisted-by: Codex:GPT-6 --- changes/4340.bugfix.md | 2 +- changes/4342.bugfix.md | 2 +- packages/zarr-metadata/changes/4342.bugfix.md | 2 +- .../src/zarr_metadata/v3/data_type/_numpy_time.py | 6 +++--- .../src/zarr_metadata/v3/data_type/numpy_datetime64.py | 3 ++- .../src/zarr_metadata/v3/data_type/numpy_timedelta64.py | 3 ++- src/zarr/core/dtype/npy/structured.py | 7 +++++-- src/zarr/core/dtype/npy/time.py | 4 ++-- src/zarr/testing/strategies.py | 3 +-- tests/test_dtype/test_npy/test_time.py | 4 ++-- 10 files changed, 20 insertions(+), 16 deletions(-) diff --git a/changes/4340.bugfix.md b/changes/4340.bugfix.md index 8658aa887b..9a23b5abf6 100644 --- a/changes/4340.bugfix.md +++ b/changes/4340.bugfix.md @@ -1 +1 @@ -Structured (``struct``) data types no longer silently corrupt data on the way in or out. Nested structured dtypes written with Zarr format 2 could not be read back, because the inner data type name check rejected the nested list-of-fields form that ``Struct.to_json`` itself produces. Structured dtypes that NumPy allows but the Zarr struct metadata cannot record used to be accepted and rebuilt as something else: aligned or explicitly offset layouts came back packed with a different itemsize, a titled field came back as two fields, and a subarray field came back as raw bytes. ``Struct.from_native_dtype`` now raises a ``ValueError`` naming the offending field for all three. The ``zarr.testing.strategies`` module gains ``zdtypes`` and ``structured_dtypes`` strategies, and the property tests now check that every registered data type round-trips through JSON and NumPy, and that structured dtype resolution either raises or returns the input dtype exactly. +Structured (``struct``) data types no longer silently corrupt data on the way in or out. Nested structured dtypes written with Zarr format 2 could not be read back, because the inner data type name check rejected the nested list-of-fields form that ``Struct.to_json`` itself produces. Structured dtypes that NumPy allows but this implementation does not preserve used to be accepted and rebuilt as something else: aligned or explicitly offset layouts came back packed with a different itemsize, a titled field came back as two fields, and a subarray field came back as raw bytes. ``Struct.from_native_dtype`` now raises a ``ValueError`` naming the offending field for all three. The ``zarr.testing.strategies`` module gains ``zdtypes`` and ``structured_dtypes`` strategies, and the property tests now check that every registered data type round-trips through JSON and NumPy, and that structured dtype resolution either raises or returns the input dtype exactly. diff --git a/changes/4342.bugfix.md b/changes/4342.bugfix.md index f5b687fba5..d37957c5db 100644 --- a/changes/4342.bugfix.md +++ b/changes/4342.bugfix.md @@ -1 +1 @@ -`DateTime64` and `TimeDelta64` now reject the `generic` unit combined with a `scale_factor` other than 1, and store the `μs` unit under NumPy's `us` spelling. Previously both values were accepted but did not survive a round trip: NumPy has no scale for the generic unit, so `DateTime64(unit="generic", scale_factor=2).to_native_dtype()` silently became plain `datetime64` (and the Zarr V2 dtype string dropped the scale factor while the V3 metadata kept it), and `DateTime64(unit="μs")` came back from NumPy as `DateTime64(unit="us")`, which compared unequal to the original. Metadata that spells the unit `μs` still reads correctly. +`DateTime64` and `TimeDelta64` now reject the `generic` unit with a `scale_factor` other than 1, and normalize `μs` to `us`. NumPy retains a generic scale internally, but its `dtype.str` omits that scale, so the Zarr V2 dtype-string round trip loses it. The generic-scale rejection is an implementation policy, not an explicit constraint in the V3 temporal extension specification. The equivalent microsecond spellings remain readable and serialize as `us`. diff --git a/packages/zarr-metadata/changes/4342.bugfix.md b/packages/zarr-metadata/changes/4342.bugfix.md index 99255143e6..d75412b349 100644 --- a/packages/zarr-metadata/changes/4342.bugfix.md +++ b/packages/zarr-metadata/changes/4342.bugfix.md @@ -1 +1 @@ -Added `numpy_datetime64_configuration` and `numpy_timedelta64_configuration` validators (and the shared `numpy_time_unit`) to the `numpy.datetime64` and `numpy.timedelta64` leaf modules. They enforce the constraints the configuration TypedDicts cannot express: `scale_factor` must be an integer in `[1, 2**31 - 1]`, the `generic` unit must have `scale_factor` 1 because NumPy's generic time type carries no scale, and the equivalent `μs` spelling of the microsecond unit is normalized to `us`. +Added `numpy_datetime64_configuration`, `numpy_timedelta64_configuration`, and `numpy_time_unit` leaf validators. They validate the documented unit vocabulary and integer scale range `[1, 2**31 - 1]`, and normalize `μs` to `us`. They additionally reject generic units with scales other than 1 as an implementation policy to avoid loss through NumPy dtype strings; this additional restriction is not stated in the V3 extension specification. diff --git a/packages/zarr-metadata/src/zarr_metadata/v3/data_type/_numpy_time.py b/packages/zarr-metadata/src/zarr_metadata/v3/data_type/_numpy_time.py index caf3f68283..b0e1bd862f 100644 --- a/packages/zarr-metadata/src/zarr_metadata/v3/data_type/_numpy_time.py +++ b/packages/zarr-metadata/src/zarr_metadata/v3/data_type/_numpy_time.py @@ -64,9 +64,9 @@ def numpy_time_configuration(value: Mapping[str, object]) -> tuple[NumpyTimeUnit integer. Raises ValueError if the object has keys other than exactly `unit` and `scale_factor`, if `unit` is not a `NumpyTimeUnit`, if `scale_factor` is outside `[1, MAX_NUMPY_TIME_SCALE_FACTOR]`, or if the unit is `"generic"` - with a `scale_factor` other than 1. NumPy's generic (unit-less) time type - carries no scale, so any other scale factor would be silently dropped when - the data type is materialized. + with a `scale_factor` other than 1. The generic-scale restriction is an + implementation policy: NumPy retains that scale internally, but its dtype + string omits it. The V3 extension specification does not state this restriction. """ keys = frozenset(value) if keys != _CONFIGURATION_KEYS: diff --git a/packages/zarr-metadata/src/zarr_metadata/v3/data_type/numpy_datetime64.py b/packages/zarr-metadata/src/zarr_metadata/v3/data_type/numpy_datetime64.py index 953772e0d4..cfb456e58f 100644 --- a/packages/zarr-metadata/src/zarr_metadata/v3/data_type/numpy_datetime64.py +++ b/packages/zarr-metadata/src/zarr_metadata/v3/data_type/numpy_datetime64.py @@ -48,7 +48,8 @@ def numpy_datetime64_configuration(value: Mapping[str, object]) -> NumpyDatetime integer. Raises ValueError if `value` does not have exactly the keys `unit` and `scale_factor`, if `unit` is not a `NumpyTimeUnit`, if `scale_factor` is outside `[1, 2**31 - 1]`, or if the `"generic"` unit is combined with a - `scale_factor` other than 1 (NumPy's generic time type has no scale). + `scale_factor` other than 1 (an implementation restriction to avoid scale + loss through NumPy dtype strings, not a stated V3 format constraint). """ unit, scale_factor = numpy_time_configuration(value) return {"unit": unit, "scale_factor": scale_factor} diff --git a/packages/zarr-metadata/src/zarr_metadata/v3/data_type/numpy_timedelta64.py b/packages/zarr-metadata/src/zarr_metadata/v3/data_type/numpy_timedelta64.py index 2180d61c73..7868ce4779 100644 --- a/packages/zarr-metadata/src/zarr_metadata/v3/data_type/numpy_timedelta64.py +++ b/packages/zarr-metadata/src/zarr_metadata/v3/data_type/numpy_timedelta64.py @@ -51,7 +51,8 @@ def numpy_timedelta64_configuration( integer. Raises ValueError if `value` does not have exactly the keys `unit` and `scale_factor`, if `unit` is not a `NumpyTimeUnit`, if `scale_factor` is outside `[1, 2**31 - 1]`, or if the `"generic"` unit is combined with a - `scale_factor` other than 1 (NumPy's generic time type has no scale). + `scale_factor` other than 1 (an implementation restriction to avoid scale + loss through NumPy dtype strings, not a stated V3 format constraint). """ unit, scale_factor = numpy_time_configuration(value) return {"unit": unit, "scale_factor": scale_factor} diff --git a/src/zarr/core/dtype/npy/structured.py b/src/zarr/core/dtype/npy/structured.py index e3082f4f72..d8ca3a105f 100644 --- a/src/zarr/core/dtype/npy/structured.py +++ b/src/zarr/core/dtype/npy/structured.py @@ -32,9 +32,9 @@ def _check_representable(dtype: np.dtype[np.void]) -> str | None: """ - Check whether a structured NumPy dtype can be represented by the Zarr struct data type. + Check whether this implementation preserves a structured NumPy dtype. - The Zarr struct metadata records only ``(name, dtype)`` pairs and reconstructs the native + This implementation records only ``(name, dtype)`` pairs and reconstructs the native dtype by packing those fields contiguously (see ``Structured.to_native_dtype``). Anything NumPy allows beyond that is lost on the round trip and would silently change how stored bytes are interpreted. This function rejects three such features, recursing into nested @@ -45,6 +45,9 @@ def _check_representable(dtype: np.dtype[np.void]) -> str | None: - subarray fields, e.g. ``np.dtype([("name", "i4", (2,))])`` - non-default field layouts, e.g. ``np.dtype(..., align=True)`` or explicit offsets + These are implementation restrictions. In particular, Zarr V2 supports subarray + field descriptors; this implementation does not preserve them. + Returns ------- str | None diff --git a/src/zarr/core/dtype/npy/time.py b/src/zarr/core/dtype/npy/time.py index 21a02a7e2a..ef6e55ac30 100644 --- a/src/zarr/core/dtype/npy/time.py +++ b/src/zarr/core/dtype/npy/time.py @@ -239,8 +239,8 @@ def __post_init__(self) -> None: # Store the NumPy spelling; "μs" stays accepted as input and in stored metadata. object.__setattr__(self, "unit", "us") if self.unit == "generic" and self.scale_factor != 1: - # NumPy's generic (unit-less) time type carries no scale, so a scale factor - # other than 1 is silently dropped by np.dtype and by the Zarr V2 dtype string. + # NumPy retains the generic scale internally, but dtype.str omits it. + # This restriction prevents loss through the Zarr V2 dtype string. raise ValueError( f"The 'generic' unit does not take a scale factor, got scale_factor={self.scale_factor}. " "Use scale_factor=1 with the 'generic' unit." diff --git a/src/zarr/testing/strategies.py b/src/zarr/testing/strategies.py index dec29fddcc..e823f4f5bf 100644 --- a/src/zarr/testing/strategies.py +++ b/src/zarr/testing/strategies.py @@ -153,8 +153,7 @@ def structured_dtypes( With ``allow_unrepresentable=False`` (the default) every dtype is packed, has plain field names and scalar fields, so it can be represented by the Zarr struct data type. With - ``allow_unrepresentable=True`` the strategy also injects the NumPy features that the Zarr - struct data type cannot record: field titles, subarray fields and ``align=True`` layouts. + ``allow_unrepresentable=True`` the strategy also injects NumPy features rejected by this implementation: field titles, subarray fields and ``align=True`` layouts. Each is injected independently at random, so most draws carry at least one and some carry none. """ diff --git a/tests/test_dtype/test_npy/test_time.py b/tests/test_dtype/test_npy/test_time.py index 5ad1cff6e4..ebbf0d3724 100644 --- a/tests/test_dtype/test_npy/test_time.py +++ b/tests/test_dtype/test_npy/test_time.py @@ -172,8 +172,8 @@ def test_time_generic_unit_rejects_scale_factor() -> None: """ Test that the 'generic' unit with a scale factor other than 1 raises a ValueError. - NumPy's generic time unit has no scale, so ``np.dtype("M8[2generic]")`` silently drops - the 2 and the value would not survive ``to_native_dtype`` or the Zarr V2 dtype string. + NumPy retains the scale in ``np.dtype("M8[2generic]")``, but its ``dtype.str`` + representation omits it. This implementation rejects it to avoid loss through V2 JSON. """ scale_factor = 2 msg = f"The 'generic' unit does not take a scale factor, got scale_factor={scale_factor}." From 381a3c88cba54dcf812e849db2890b5e212bba79 Mon Sep 17 00:00:00 2001 From: Davis Vann Bennett Date: Sun, 13 Sep 2026 18:28:57 +0200 Subject: [PATCH 9/9] fix(dtype): preserve scaled generic temporal data through storage Assisted-by: Codex:GPT-6 --- changes/4342.bugfix.md | 2 +- packages/zarr-metadata/changes/4342.bugfix.md | 2 +- .../zarr_metadata/v3/data_type/_numpy_time.py | 9 +- .../v3/data_type/numpy_datetime64.py | 4 +- .../v3/data_type/numpy_timedelta64.py | 4 +- .../numpy_datetime64/test_validators.py | 10 +-- .../numpy_timedelta64/test_validators.py | 10 +-- src/zarr/core/buffer/cpu.py | 48 ++++++++++- src/zarr/core/dtype/npy/structured.py | 17 +++- src/zarr/core/dtype/npy/time.py | 22 ++--- src/zarr/testing/strategies.py | 15 +--- tests/test_buffer.py | 29 +++++++ tests/test_dtype/test_npy/test_time.py | 84 +++++++++++++++---- 13 files changed, 190 insertions(+), 66 deletions(-) diff --git a/changes/4342.bugfix.md b/changes/4342.bugfix.md index d37957c5db..5b8f4bb342 100644 --- a/changes/4342.bugfix.md +++ b/changes/4342.bugfix.md @@ -1 +1 @@ -`DateTime64` and `TimeDelta64` now reject the `generic` unit with a `scale_factor` other than 1, and normalize `μs` to `us`. NumPy retains a generic scale internally, but its `dtype.str` omits that scale, so the Zarr V2 dtype-string round trip loses it. The generic-scale rejection is an implementation policy, not an explicit constraint in the V3 temporal extension specification. The equivalent microsecond spellings remain readable and serialize as `us`. +`DateTime64` and `TimeDelta64` normalize `μs` to `us` and preserve generic-unit scale factors when converting native NumPy dtypes and serializing V2/V3 metadata. V2 writes an explicit suffix such as `[2generic]` to avoid the scale loss in NumPy's `dtype.str` and `dtype.name`. Generic temporal arrays also preserve dtype parameters during CPU buffer allocation and correctly convert byte order; generic datetime fill values can be read back, including in structured fields. diff --git a/packages/zarr-metadata/changes/4342.bugfix.md b/packages/zarr-metadata/changes/4342.bugfix.md index d75412b349..240c7c8632 100644 --- a/packages/zarr-metadata/changes/4342.bugfix.md +++ b/packages/zarr-metadata/changes/4342.bugfix.md @@ -1 +1 @@ -Added `numpy_datetime64_configuration`, `numpy_timedelta64_configuration`, and `numpy_time_unit` leaf validators. They validate the documented unit vocabulary and integer scale range `[1, 2**31 - 1]`, and normalize `μs` to `us`. They additionally reject generic units with scales other than 1 as an implementation policy to avoid loss through NumPy dtype strings; this additional restriction is not stated in the V3 extension specification. +Added `numpy_datetime64_configuration`, `numpy_timedelta64_configuration`, and `numpy_time_unit` leaf validators. They validate the documented unit vocabulary and integer scale range `[1, 2**31 - 1]`, including scaled generic units, and normalize `μs` to `us`. diff --git a/packages/zarr-metadata/src/zarr_metadata/v3/data_type/_numpy_time.py b/packages/zarr-metadata/src/zarr_metadata/v3/data_type/_numpy_time.py index b0e1bd862f..6d93cf3942 100644 --- a/packages/zarr-metadata/src/zarr_metadata/v3/data_type/_numpy_time.py +++ b/packages/zarr-metadata/src/zarr_metadata/v3/data_type/_numpy_time.py @@ -63,10 +63,7 @@ def numpy_time_configuration(value: Mapping[str, object]) -> tuple[NumpyTimeUnit Raises TypeError if `unit` is not a string or `scale_factor` is not an integer. Raises ValueError if the object has keys other than exactly `unit` and `scale_factor`, if `unit` is not a `NumpyTimeUnit`, if `scale_factor` is - outside `[1, MAX_NUMPY_TIME_SCALE_FACTOR]`, or if the unit is `"generic"` - with a `scale_factor` other than 1. The generic-scale restriction is an - implementation policy: NumPy retains that scale internally, but its dtype - string omits it. The V3 extension specification does not state this restriction. + outside `[1, MAX_NUMPY_TIME_SCALE_FACTOR]`. """ keys = frozenset(value) if keys != _CONFIGURATION_KEYS: @@ -84,10 +81,6 @@ def numpy_time_configuration(value: Mapping[str, object]) -> tuple[NumpyTimeUnit raise ValueError( f"Expected 'scale_factor' in [1, {MAX_NUMPY_TIME_SCALE_FACTOR}], got {scale_factor}" ) - if unit == "generic" and scale_factor != 1: - raise ValueError( - f"The 'generic' unit does not take a scale factor, got scale_factor={scale_factor}" - ) return unit, scale_factor diff --git a/packages/zarr-metadata/src/zarr_metadata/v3/data_type/numpy_datetime64.py b/packages/zarr-metadata/src/zarr_metadata/v3/data_type/numpy_datetime64.py index cfb456e58f..8e6fb83bfb 100644 --- a/packages/zarr-metadata/src/zarr_metadata/v3/data_type/numpy_datetime64.py +++ b/packages/zarr-metadata/src/zarr_metadata/v3/data_type/numpy_datetime64.py @@ -47,9 +47,7 @@ def numpy_datetime64_configuration(value: Mapping[str, object]) -> NumpyDatetime Raises TypeError if `unit` is not a string or `scale_factor` is not an integer. Raises ValueError if `value` does not have exactly the keys `unit` and `scale_factor`, if `unit` is not a `NumpyTimeUnit`, if `scale_factor` is - outside `[1, 2**31 - 1]`, or if the `"generic"` unit is combined with a - `scale_factor` other than 1 (an implementation restriction to avoid scale - loss through NumPy dtype strings, not a stated V3 format constraint). + outside `[1, 2**31 - 1]`. """ unit, scale_factor = numpy_time_configuration(value) return {"unit": unit, "scale_factor": scale_factor} diff --git a/packages/zarr-metadata/src/zarr_metadata/v3/data_type/numpy_timedelta64.py b/packages/zarr-metadata/src/zarr_metadata/v3/data_type/numpy_timedelta64.py index 7868ce4779..c0bcf9c06d 100644 --- a/packages/zarr-metadata/src/zarr_metadata/v3/data_type/numpy_timedelta64.py +++ b/packages/zarr-metadata/src/zarr_metadata/v3/data_type/numpy_timedelta64.py @@ -50,9 +50,7 @@ def numpy_timedelta64_configuration( Raises TypeError if `unit` is not a string or `scale_factor` is not an integer. Raises ValueError if `value` does not have exactly the keys `unit` and `scale_factor`, if `unit` is not a `NumpyTimeUnit`, if `scale_factor` is - outside `[1, 2**31 - 1]`, or if the `"generic"` unit is combined with a - `scale_factor` other than 1 (an implementation restriction to avoid scale - loss through NumPy dtype strings, not a stated V3 format constraint). + outside `[1, 2**31 - 1]`. """ unit, scale_factor = numpy_time_configuration(value) return {"unit": unit, "scale_factor": scale_factor} diff --git a/packages/zarr-metadata/tests/v3/data_type/numpy_datetime64/test_validators.py b/packages/zarr-metadata/tests/v3/data_type/numpy_datetime64/test_validators.py index c18006fcc2..e8a70ef047 100644 --- a/packages/zarr-metadata/tests/v3/data_type/numpy_datetime64/test_validators.py +++ b/packages/zarr-metadata/tests/v3/data_type/numpy_datetime64/test_validators.py @@ -16,6 +16,11 @@ ({"unit": "ns", "scale_factor": 1}, {"unit": "ns", "scale_factor": 1}), ({"unit": "s", "scale_factor": 10}, {"unit": "s", "scale_factor": 10}), ({"unit": "generic", "scale_factor": 1}, {"unit": "generic", "scale_factor": 1}), + ({"unit": "generic", "scale_factor": 2}, {"unit": "generic", "scale_factor": 2}), + ( + {"unit": "generic", "scale_factor": 2**31 - 1}, + {"unit": "generic", "scale_factor": 2**31 - 1}, + ), ({"unit": "us", "scale_factor": 2}, {"unit": "us", "scale_factor": 2}), ({"unit": "μs", "scale_factor": 2}, {"unit": "us", "scale_factor": 2}), ({"unit": "Y", "scale_factor": 2**31 - 1}, {"unit": "Y", "scale_factor": 2**31 - 1}), @@ -59,8 +64,3 @@ def test_scale_factor_not_an_integer(scale_factor: object) -> None: def test_scale_factor_out_of_range(scale_factor: int) -> None: with pytest.raises(ValueError, match=r"Expected 'scale_factor' in \[1, 2147483647\]"): numpy_datetime64_configuration({"unit": "s", "scale_factor": scale_factor}) - - -def test_generic_unit_rejects_scale_factor() -> None: - with pytest.raises(ValueError, match="'generic' unit does not take a scale factor"): - numpy_datetime64_configuration({"unit": "generic", "scale_factor": 2}) diff --git a/packages/zarr-metadata/tests/v3/data_type/numpy_timedelta64/test_validators.py b/packages/zarr-metadata/tests/v3/data_type/numpy_timedelta64/test_validators.py index e301292a7d..67316176fb 100644 --- a/packages/zarr-metadata/tests/v3/data_type/numpy_timedelta64/test_validators.py +++ b/packages/zarr-metadata/tests/v3/data_type/numpy_timedelta64/test_validators.py @@ -16,6 +16,11 @@ ({"unit": "ns", "scale_factor": 1}, {"unit": "ns", "scale_factor": 1}), ({"unit": "s", "scale_factor": 10}, {"unit": "s", "scale_factor": 10}), ({"unit": "generic", "scale_factor": 1}, {"unit": "generic", "scale_factor": 1}), + ({"unit": "generic", "scale_factor": 2}, {"unit": "generic", "scale_factor": 2}), + ( + {"unit": "generic", "scale_factor": 2**31 - 1}, + {"unit": "generic", "scale_factor": 2**31 - 1}, + ), ({"unit": "us", "scale_factor": 2}, {"unit": "us", "scale_factor": 2}), ({"unit": "μs", "scale_factor": 2}, {"unit": "us", "scale_factor": 2}), ({"unit": "Y", "scale_factor": 2**31 - 1}, {"unit": "Y", "scale_factor": 2**31 - 1}), @@ -59,8 +64,3 @@ def test_scale_factor_not_an_integer(scale_factor: object) -> None: def test_scale_factor_out_of_range(scale_factor: int) -> None: with pytest.raises(ValueError, match=r"Expected 'scale_factor' in \[1, 2147483647\]"): numpy_timedelta64_configuration({"unit": "s", "scale_factor": scale_factor}) - - -def test_generic_unit_rejects_scale_factor() -> None: - with pytest.raises(ValueError, match="'generic' unit does not take a scale factor"): - numpy_timedelta64_configuration({"unit": "generic", "scale_factor": 2}) diff --git a/src/zarr/core/buffer/cpu.py b/src/zarr/core/buffer/cpu.py index 8994281b58..859e04396d 100644 --- a/src/zarr/core/buffer/cpu.py +++ b/src/zarr/core/buffer/cpu.py @@ -4,6 +4,7 @@ TYPE_CHECKING, Any, Literal, + cast, ) import numpy as np @@ -155,20 +156,61 @@ def create( ) -> Self: # np.zeros is much faster than np.full, and therefore using it when possible is better. if fill_value is None or (isinstance(fill_value, int) and fill_value == 0): - return cls(np.zeros(shape=tuple(shape), dtype=dtype, order=order)) + data = np.zeros(shape=tuple(shape), dtype=dtype, order=order) else: - return cls(np.full(shape=tuple(shape), fill_value=fill_value, dtype=dtype, order=order)) + parsed_dtype = np.dtype(dtype) + if ( + isinstance(parsed_dtype, np.dtypes.DateTime64DType | np.dtypes.TimeDelta64DType) + and np.datetime_data( + cast("np.dtypes.DateTime64DType | np.dtypes.TimeDelta64DType", parsed_dtype) + )[0] + == "generic" + ): + # np.full can also drop the requested byte order. Restore the dtype + # before assignment so values are encoded in the requested byte order. + data = np.empty(shape=tuple(shape), dtype=parsed_dtype, order=order).view( + parsed_dtype + ) + data[...] = fill_value + else: + data = np.full(shape=tuple(shape), fill_value=fill_value, dtype=dtype, order=order) + if data.dtype.kind in "mM": + # NumPy allocation can discard generic temporal scales. A view retains them. + data = data.view(dtype=dtype) + return cls(data) @classmethod def empty( cls, shape: tuple[int, ...], dtype: npt.DTypeLike, order: Literal["C", "F"] = "C" ) -> Self: - return cls(np.empty(shape=shape, dtype=dtype, order=order)) + data = np.empty(shape=shape, dtype=dtype, order=order) + if data.dtype.kind in "mM": + data = data.view(dtype=dtype) + return cls(data) @classmethod def from_numpy_array(cls, array_like: npt.ArrayLike) -> Self: return cls.from_ndarray_like(np.asanyarray(array_like)) + def astype(self, dtype: npt.DTypeLike, order: Literal["K", "A", "C", "F"] = "K") -> Self: + target = np.dtype(dtype) + if ( + self.dtype.kind in "mM" + and isinstance(target, np.dtypes.DateTime64DType | np.dtypes.TimeDelta64DType) + and target.kind == self.dtype.kind + and np.datetime_data(self.dtype)[0] == "generic" + and np.datetime_data(self.dtype) + == np.datetime_data( + cast("np.dtypes.DateTime64DType | np.dtypes.TimeDelta64DType", target) + ) + ): + # NumPy's generic-time astype can change the byte-order marker without + # swapping the data. Convert the underlying counts for an endian-only cast. + counts = self.as_numpy_array().view(self.dtype.byteorder + "i8") + converted = counts.astype(target.byteorder + "i8", order=order) + return self.__class__(converted.view(target)) + return super().astype(dtype, order=order) + def as_numpy_array(self) -> npt.NDArray[Any]: """Returns the buffer as a NumPy array (host memory). diff --git a/src/zarr/core/dtype/npy/structured.py b/src/zarr/core/dtype/npy/structured.py index d8ca3a105f..518becc4a7 100644 --- a/src/zarr/core/dtype/npy/structured.py +++ b/src/zarr/core/dtype/npy/structured.py @@ -509,7 +509,22 @@ def default_scalar(self) -> np.void: cast to this structured data type. """ - return self._cast_scalar_unchecked(0) + values: list[object] = [] + for _, field in self.fields: + dtype = field.to_native_dtype() + if isinstance(field, Structured): + value = field.default_scalar() + elif ( + isinstance(dtype, np.dtypes.DateTime64DType) + and np.datetime_data(cast("np.dtypes.DateTime64DType", dtype))[0] == "generic" + ): + # NumPy rejects casting integer zero to generic datetime, but a + # zero count is representable by viewing the integer storage. + value = np.zeros(1, dtype=dtype.byteorder + "i8").view(dtype)[0] + else: + value = np.array([0], dtype=dtype)[0] + values.append(value) + return self._cast_scalar_unchecked(tuple(values)) def from_json_scalar(self, data: JSON, *, zarr_format: ZarrFormat) -> np.void: """ diff --git a/src/zarr/core/dtype/npy/time.py b/src/zarr/core/dtype/npy/time.py index ef6e55ac30..dddf65fd72 100644 --- a/src/zarr/core/dtype/npy/time.py +++ b/src/zarr/core/dtype/npy/time.py @@ -238,13 +238,6 @@ def __post_init__(self) -> None: # instance built with "μs" would not round-trip through to_native_dtype(). # Store the NumPy spelling; "μs" stays accepted as input and in stored metadata. object.__setattr__(self, "unit", "us") - if self.unit == "generic" and self.scale_factor != 1: - # NumPy retains the generic scale internally, but dtype.str omits it. - # This restriction prevents loss through the Zarr V2 dtype string. - raise ValueError( - f"The 'generic' unit does not take a scale factor, got scale_factor={self.scale_factor}. " - "Use scale_factor=1 with the 'generic' unit." - ) @classmethod def from_native_dtype(cls, dtype: TBaseDType) -> Self: @@ -268,7 +261,7 @@ def from_native_dtype(cls, dtype: TBaseDType) -> Self: """ if cls._check_native_dtype(dtype): - unit, scale_factor = np.datetime_data(dtype.name) + unit, scale_factor = np.datetime_data(dtype) unit = cast("DateTimeUnit", unit) return cls( unit=unit, @@ -504,7 +497,10 @@ def to_json(self, zarr_format: ZarrFormat) -> TimeDelta64JSON_V2 | TimeDelta64JS If the zarr_format is not 2 or 3. """ if zarr_format == 2: - name = self.to_native_dtype().str + name: str = self.to_native_dtype().str + if self.unit == "generic" and self.scale_factor != 1: + # NumPy omits generic scale from dtype.str; preserve it explicitly. + name += f"[{self.scale_factor}generic]" return {"name": name, "object_codec_id": None} elif zarr_format == 3: return { @@ -789,7 +785,10 @@ def to_json(self, zarr_format: ZarrFormat) -> DateTime64JSON_V2 | DateTime64JSON If the zarr_format is not 2 or 3. """ if zarr_format == 2: - name = self.to_native_dtype().str + name: str = self.to_native_dtype().str + if self.unit == "generic" and self.scale_factor != 1: + # NumPy omits generic scale from dtype.str; preserve it explicitly. + name += f"[{self.scale_factor}generic]" return {"name": name, "object_codec_id": None} elif zarr_format == 3: return { @@ -830,6 +829,9 @@ def _cast_scalar_unchecked(self, data: DateTimeLike) -> np.datetime64: numpy.datetime64 The input cast to a NumPy datetime scalar. """ + if isinstance(data, int): + # The scalar constructor rejects integer counts with a generic unit. + return datetime_from_int(data, unit=self.unit, scale_factor=self.scale_factor) # numpy 2.x stub: datetime64(scalar, formatted_unit_str) is runtime-valid # but no overload matches the dynamic f-string unit argument. return self.to_native_dtype().type(data, f"{self.scale_factor}{self.unit}") # type: ignore[call-overload, no-any-return] diff --git a/src/zarr/testing/strategies.py b/src/zarr/testing/strategies.py index e823f4f5bf..62ff07276e 100644 --- a/src/zarr/testing/strategies.py +++ b/src/zarr/testing/strategies.py @@ -95,18 +95,9 @@ def _leaf_zdtypes(cls: type[ZDType[TBaseDType, TBaseScalar]]) -> SearchStrategy[ if "length" in params: kwargs["length"] = st.integers(min_value=1, max_value=16) if "unit" in params: - # The constructor normalizes the "μs" alias to "us", so every unit is safe to draw, but - # the generic unit only accepts scale_factor=1, so the scale factor depends on the unit. - return st.sampled_from(DATETIME_UNIT).flatmap( - lambda unit: st.builds( - cls, - unit=st.just(unit), - scale_factor=st.just(1) - if unit == "generic" - else st.integers(min_value=1, max_value=2**31 - 1), - **kwargs, - ) - ) + # The constructor normalizes the microsecond alias; all units accept a scale. + kwargs["unit"] = st.sampled_from(DATETIME_UNIT) + kwargs["scale_factor"] = st.integers(min_value=1, max_value=2**31 - 1) return st.builds(cls, **kwargs) diff --git a/tests/test_buffer.py b/tests/test_buffer.py index b4a16ed1de..2f1cdb58f0 100644 --- a/tests/test_buffer.py +++ b/tests/test_buffer.py @@ -238,3 +238,32 @@ def test_empty( assert result.flags.c_contiguous # type: ignore[attr-defined] else: assert result.flags.f_contiguous # type: ignore[attr-defined] + + +@pytest.mark.parametrize("kind", ["M8", "m8"]) +@pytest.mark.parametrize("byteorder", ["<", ">"]) +@pytest.mark.parametrize("scale_factor", [2, 2**31 - 1]) +@pytest.mark.parametrize("fill_value", [None, 0, "NaT"]) +@pytest.mark.parametrize("order", ["C", "F"]) +@pytest.mark.filterwarnings( + "ignore:The 'generic' unit for NumPy timedelta is deprecated:DeprecationWarning" +) +def test_cpu_generic_time_allocation( + kind: str, + byteorder: str, + scale_factor: int, + fill_value: int | str | None, + order: Literal["C", "F"], +) -> None: + """Both allocation paths retain generic scale even when NumPy drops it.""" + dtype = np.dtype(f"{byteorder}{kind}[{scale_factor}generic]") + empty = cpu.NDBuffer.empty((2, 3), dtype=dtype, order=order) + filled = cpu.NDBuffer.create(shape=(2, 3), dtype=dtype, order=order, fill_value=fill_value) + for buffer in (empty, filled): + assert buffer.dtype == dtype + array = buffer.as_numpy_array() + assert array.flags.c_contiguous if order == "C" else array.flags.f_contiguous + expected = -(2**63) if fill_value == "NaT" else 0 + np.testing.assert_array_equal( + filled.as_numpy_array().view(dtype.byteorder + "i8"), np.full((2, 3), expected) + ) diff --git a/tests/test_dtype/test_npy/test_time.py b/tests/test_dtype/test_npy/test_time.py index ebbf0d3724..900ea9b853 100644 --- a/tests/test_dtype/test_npy/test_time.py +++ b/tests/test_dtype/test_npy/test_time.py @@ -1,15 +1,19 @@ from __future__ import annotations import re -from typing import get_args +from typing import TYPE_CHECKING, get_args import numpy as np import pytest +import zarr from tests.test_dtype.test_wrapper import BaseTestZDType from zarr.core.dtype.npy.common import DateTimeUnit from zarr.core.dtype.npy.time import DateTime64, TimeDelta64, datetime_from_int +if TYPE_CHECKING: + from zarr.core.common import ZarrFormat + class _TestTimeBase(BaseTestZDType): def json_scalar_equals(self, scalar1: object, scalar2: object) -> bool: @@ -168,19 +172,33 @@ def test_time_scale_factor_too_high() -> None: TimeDelta64(scale_factor=scale_factor) -def test_time_generic_unit_rejects_scale_factor() -> None: - """ - Test that the 'generic' unit with a scale factor other than 1 raises a ValueError. - - NumPy retains the scale in ``np.dtype("M8[2generic]")``, but its ``dtype.str`` - representation omits it. This implementation rejects it to avoid loss through V2 JSON. - """ - scale_factor = 2 - msg = f"The 'generic' unit does not take a scale factor, got scale_factor={scale_factor}." - with pytest.raises(ValueError, match=re.escape(msg)): - DateTime64(unit="generic", scale_factor=scale_factor) - with pytest.raises(ValueError, match=re.escape(msg)): - TimeDelta64(unit="generic", scale_factor=scale_factor) +@pytest.mark.parametrize("cls", [DateTime64, TimeDelta64]) +@pytest.mark.parametrize("unit", get_args(DateTimeUnit)) +@pytest.mark.parametrize("scale_factor", [1, 2, 2**31 - 1]) +@pytest.mark.parametrize("byteorder", ["<", ">"]) +def test_time_dtype_roundtrip( + cls: type[DateTime64 | TimeDelta64], + unit: DateTimeUnit, + scale_factor: int, + byteorder: str, +) -> None: + """Native and JSON conversions must preserve temporal parameters, including generic scale.""" + kind = "M8" if cls is DateTime64 else "m8" + native = np.dtype(f"{byteorder}{kind}[{scale_factor}{unit}]") + expected_unit = "us" if unit == "μs" else unit + dtype = cls.from_native_dtype(native) + assert (dtype.unit, dtype.scale_factor) == (expected_unit, scale_factor) + restored_native = dtype.to_native_dtype() + assert np.datetime_data(restored_native) == (expected_unit, scale_factor) + assert restored_native == native + json_v2 = dtype.to_json(zarr_format=2) + assert np.datetime_data(np.dtype(json_v2["name"])) == (expected_unit, scale_factor) + assert cls.from_json(json_v2, zarr_format=2) == dtype + json_v3 = dtype.to_json(zarr_format=3) + assert json_v3["configuration"]["unit"] == expected_unit + assert json_v3["configuration"]["scale_factor"] == scale_factor + restored_v3 = cls.from_json(json_v3, zarr_format=3) + assert np.datetime_data(restored_v3.to_native_dtype()) == (expected_unit, scale_factor) @pytest.mark.parametrize("cls", [DateTime64, TimeDelta64]) @@ -210,3 +228,41 @@ def test_datetime_from_int(unit: DateTimeUnit, scale_factor: int, value: int) -> """ expected = np.int64(value).view(f"datetime64[{scale_factor}{unit}]") assert datetime_from_int(value, unit=unit, scale_factor=scale_factor) == expected + + +@pytest.mark.parametrize("unit", ["generic", "us"]) +@pytest.mark.parametrize("kind", ["M8", "m8"]) +@pytest.mark.parametrize("byteorder", ["<", ">"]) +@pytest.mark.parametrize("scale_factor", [1, 2, 2**31 - 1]) +@pytest.mark.parametrize("zarr_format", [2, 3]) +@pytest.mark.parametrize("structured", [False, True]) +@pytest.mark.filterwarnings("ignore::zarr.errors.UnstableSpecificationWarning") +@pytest.mark.filterwarnings( + "ignore:The 'generic' unit for NumPy timedelta is deprecated:DeprecationWarning" +) +def test_generic_time_array_roundtrip( + unit: str, + kind: str, + byteorder: str, + scale_factor: int, + zarr_format: ZarrFormat, + structured: bool, +) -> None: + """Persist counts and generic scale through metadata, chunk IO, and output allocation.""" + leaf = np.dtype(f"{byteorder}{kind}[{scale_factor}{unit}]") + dtype = np.dtype([("time", leaf)]) if structured else leaf + counts = np.array([0, 1, -2, 100], dtype=f"{byteorder}i8") + data = counts.view(dtype) + array = zarr.create_array( + store={}, data=data, chunks=2, zarr_format=zarr_format, compressors=None + ) + array.resize((6,)) + reopened = zarr.open_array(array.store, mode="r") + result = np.asarray(reopened[:]) + values = result["time"] if structured else result + assert np.datetime_data(values.dtype) == (unit, scale_factor) + np.testing.assert_array_equal(values[:4].view(values.dtype.byteorder + "i8"), counts) + expected_fill = 0 if structured else -(2**63) + np.testing.assert_array_equal( + values[4:].view(values.dtype.byteorder + "i8"), [expected_fill, expected_fill] + )