fix: tighten array metadata field validation

changes/4494.bugfix.md

@@ -0,0 +1 @@
+Tighten array metadata field validation: `dimension_names` rejects a bare string and `storage_transformers` rejects a mapping. `shape` fields that contain booleans and `attributes` fields that are not mappings with string keys still parse as before, but emit a `ZarrDeprecationWarning` ahead of rejection in a future release.

src/zarr/core/common.py

@@ -21,7 +21,7 @@
 
 from zarr.core.config import config as zarr_config
 from zarr.core.json_parse import convert, parse_field
-from zarr.errors import ZarrRuntimeWarning
+from zarr.errors import ZarrDeprecationWarning, ZarrRuntimeWarning
 
 if TYPE_CHECKING:
     from collections.abc import Awaitable, Callable, Iterator
@@ -211,10 +211,21 @@ def parse_named_configuration(
     return name_parsed, configuration_parsed
 
 
+def _warn_bool_shapelike() -> None:
+    msg = (
+        "Boolean values in shape-like fields are deprecated and will be "
+        "rejected in a future release."
+    )
+    warnings.warn(msg, ZarrDeprecationWarning, stacklevel=3)
+
+
 def parse_shapelike(data: ShapeLike) -> tuple[int, ...]:
     """
     Parse a shape-like input into an explicit shape.
     """
+    if isinstance(data, bool):
+        # bools are int subclasses; accept them for now but deprecate
+        _warn_bool_shapelike()
     if isinstance(data, int | np.integer):
         if data < 0:
             raise ValueError(f"Expected a non-negative integer. Got {data} instead")
@@ -228,6 +239,8 @@ def parse_shapelike(data: ShapeLike) -> tuple[int, ...]:
     if not all(isinstance(v, int | np.integer) for v in data_tuple):
         msg = f"Expected an iterable of integers. Got {data} instead."
         raise TypeError(msg)
+    if any(isinstance(v, bool) for v in data_tuple):
+        _warn_bool_shapelike()
     if not all(v > -1 for v in data_tuple):
         msg = f"Expected all values to be non-negative. Got {data} instead."
         raise ValueError(msg)

src/zarr/core/metadata/common.py

@@ -1,13 +1,23 @@
 from __future__ import annotations
 
-from typing import TYPE_CHECKING
+import warnings
+from collections.abc import Mapping
+from typing import TYPE_CHECKING, cast
+
+from zarr.errors import ZarrDeprecationWarning
 
 if TYPE_CHECKING:
     from zarr.core.common import JSON
 
 
-def parse_attributes(data: dict[str, JSON] | None) -> dict[str, JSON]:
+def parse_attributes(data: object) -> dict[str, JSON]:
     if data is None:
         return {}
-
-    return dict(data)
+    if not isinstance(data, Mapping) or not all(isinstance(k, str) for k in data):
+        msg = (
+            "Attributes metadata that is not a mapping with string keys is "
+            "deprecated and will be rejected in a future release. "
+            f"Got {data!r}."
+        )
+        warnings.warn(msg, ZarrDeprecationWarning, stacklevel=2)
+    return dict(cast("Mapping[str, JSON]", data))

src/zarr/core/metadata/v3.py

@@ -137,7 +137,11 @@ def validate_codecs(codecs: tuple[Codec, ...], dtype: ZDType[TBaseDType, TBaseSc
 def parse_dimension_names(data: object) -> tuple[str | None, ...] | None:
     if data is None:
         return data
-    elif isinstance(data, Iterable) and all(isinstance(x, type(None) | str) for x in data):
+    elif (
+        isinstance(data, Iterable)
+        and not isinstance(data, str | bytes)
+        and all(isinstance(x, type(None) | str) for x in data)
+    ):
         return tuple(data)
     else:
         msg = f"Expected either None or an iterable of str, got {type(data)}"
@@ -151,7 +155,7 @@ def parse_storage_transformers(data: object) -> tuple[dict[str, JSON], ...]:
     """
     if data is None:
         return ()
-    if isinstance(data, Iterable) and not isinstance(data, (str, bytes)):
+    if isinstance(data, Iterable) and not isinstance(data, str | bytes | dict):
         # Materialise once. The previous implementation called ``len(tuple(data))``
         # and then returned ``data`` itself, which exhausted (and discarded) a
         # one-shot iterable and could return a value typed as a tuple that was not

tests/test_array.py

@@ -344,7 +344,7 @@ def test_storage_transformers(store: MemoryStore, zarr_format: ZarrFormat | str)
             "chunk_key_encoding": {"name": "v2", "configuration": {"separator": "/"}},
             "codecs": (BytesCodec().to_dict(),),
             "fill_value": 0,
-            "storage_transformers": ({"test": "should_raise"}),
+            "storage_transformers": [{"test": "should_raise"}],
         }
     else:
         metadata_dict = {
@@ -356,7 +356,7 @@ def test_storage_transformers(store: MemoryStore, zarr_format: ZarrFormat | str)
             "codecs": (BytesCodec().to_dict(),),
             "fill_value": 0,
             "order": "C",
-            "storage_transformers": ({"test": "should_raise"}),
+            "storage_transformers": [{"test": "should_raise"}],
         }
     if zarr_format == 3:
         match = "Arrays with storage transformers are not supported in zarr-python at this time."

tests/test_codecs/test_sharding.py

@@ -35,6 +35,7 @@
 from zarr.core.buffer import NDArrayLike, default_buffer_prototype
 from zarr.core.dtype import Int32
 from zarr.core.indexing import lexicographic_order_coords
+from zarr.errors import ZarrDeprecationWarning
 from zarr.core.metadata.v3 import ArrayV3Metadata
 from zarr.storage import MemoryStore, StorePath, ZipStore
 
@@ -1492,15 +1493,25 @@ def test_nested_sharding_rejects_indivisible_inner_chunk_shape(
     assert store._store_dict == {}
 
 
-@pytest.mark.parametrize("chunk_shape", [(0, 5), (5, 0), (False, 5), [0], 0])
+@pytest.mark.parametrize("chunk_shape", [(0, 5), (5, 0), [0], 0])
 def test_sharding_codec_rejects_inner_chunk_size_zero(chunk_shape: Any) -> None:
-    """`ShardingCodec` rejects an inner chunk size of 0 (or `False`) when it is
+    """`ShardingCodec` rejects an inner chunk size of 0 when it is
     constructed, with a `ValueError` naming the dimension, so no array metadata, nested
     codec or stored document can hold one."""
     with pytest.raises(ValueError, match=r"Dimension \d: chunk edge length must be >= 1, got"):
         ShardingCodec(chunk_shape=chunk_shape)
 
 
+def test_sharding_codec_rejects_inner_chunk_size_false() -> None:
+    """`False` in an inner chunk shape warns (booleans in shape-like fields are
+    deprecated) and is still rejected as a zero edge length."""
+    with pytest.warns(ZarrDeprecationWarning, match="Boolean values in shape-like"):
+        with pytest.raises(
+            ValueError, match=r"Dimension \d: chunk edge length must be >= 1, got"
+        ):
+            ShardingCodec(chunk_shape=(False, 5))
+
+
 @pytest.mark.filterwarnings(
     "ignore:Combining a `sharding_indexed` codec:zarr.errors.ZarrUserWarning"
 )

tests/test_common.py

@@ -21,6 +21,7 @@
     product,
 )
 from zarr.core.config import parse_indexing_order
+from zarr.errors import ZarrDeprecationWarning
 
 if TYPE_CHECKING:
     from typing import Any, Literal
@@ -223,6 +224,13 @@ def test_parse_shapelike_invalid_iterable_types(data: Any) -> None:
         parse_shapelike(data)
 
 
+@pytest.mark.parametrize(("data", "expected"), [(True, (1,)), ([True], (1,)), ((True, 1), (1, 1))])
+def test_parse_shapelike_bool_deprecated(data: Any, expected: tuple[int, ...]) -> None:
+    """Bools still parse as ints for backward compatibility, but warn."""
+    with pytest.warns(ZarrDeprecationWarning, match="Boolean values"):
+        assert parse_shapelike(data) == expected
+
+
 @pytest.mark.parametrize("data", [(1, 2, 3, -1), (-10,)])
 def test_parse_shapelike_invalid_iterable_values(data: Any) -> None:
     """

tests/test_json_parse.py

@@ -85,3 +85,12 @@ def test_generator_not_exhausted(self) -> None:
     def test_non_iterable_rejected(self) -> None:
         with pytest.raises(TypeError, match="Expected an iterable"):
             parse_storage_transformers(5)
+
+    def test_dict_rejected(self) -> None:
+        """A dict is iterable, but iterating it yields keys rather than
+        transformer objects, so it must be rejected rather than silently
+        swallowed."""
+        with pytest.raises(TypeError, match="Expected an iterable"):
+            parse_storage_transformers({})
+        with pytest.raises(TypeError, match="Expected an iterable"):
+            parse_storage_transformers({"name": "x"})

tests/test_metadata/test_repair.py

@@ -27,7 +27,7 @@
 from zarr.core.metadata.v3 import RectilinearChunkGridMetadata, RegularChunkGridMetadata
 from zarr.core.sync import sync
 from zarr.dtype import Int16
-from zarr.errors import ZarrUserWarning
+from zarr.errors import ZarrDeprecationWarning, ZarrUserWarning
 from zarr.storage import LocalStore, MemoryStore, StorePath
 from zarr.storage._common import make_store_path
 
@@ -518,7 +518,6 @@ def _sharding_chunk_shape(chunks: Any) -> tuple[tuple[int, ...], object]:
         (np.int64(4), (4,)),
         ((np.int64(4),), (4,)),
         (np.array([4]), (4,)),
-        ((True,), (1,)),
         (range(4, 5), (4,)),
     ],
 )
@@ -527,14 +526,23 @@ def test_chunk_shape_read_as_array_shape(
 ) -> None:
     """`ArrayV2Metadata` and `ShardingCodec` read their chunk shape as `parse_shapelike`
     reads an array shape: an integer or an iterable of integers, including NumPy
-    integers and bools."""
+    integers."""
     parsed, written = CHUNK_SHAPE_SITES[site](chunks)
     assert parsed == expected
     assert all(type(size) is int for size in parsed)
     assert written == expected
 
 
-@pytest.mark.parametrize("chunks", [(0,), (False,)])
+@pytest.mark.parametrize("site", CHUNK_SHAPE_SITES)
+def test_chunk_shape_bool_deprecated(site: str) -> None:
+    """A bool chunk size still reads as 1, but warns pending removal."""
+    with pytest.warns(ZarrDeprecationWarning, match="Boolean values"):
+        parsed, written = CHUNK_SHAPE_SITES[site]((True,))
+    assert parsed == (1,)
+    assert written == (1,)
+
+
+@pytest.mark.parametrize("chunks", [(0,)])
 def test_v2_chunk_size_zero_written_as_given(chunks: tuple[Any, ...]) -> None:
     """`ArrayV2Metadata` accepts a chunk size of 0 and writes it back as given; reading
     a stored 0 is `zarr.core.metadata.repair`' business. `ShardingCodec` rejects one."""
@@ -543,6 +551,15 @@ def test_v2_chunk_size_zero_written_as_given(chunks: tuple[Any, ...]) -> None:
     assert metadata.to_dict()["chunks"] == (0,)
 
 
+def test_v2_chunk_size_false_written_as_given() -> None:
+    """A bool chunk size still reads and writes as before, but warns pending
+    removal."""
+    with pytest.warns(ZarrDeprecationWarning, match="Boolean values"):
+        metadata = _v2_metadata((False,))
+    assert metadata.chunks == (0,)
+    assert metadata.to_dict()["chunks"] == (0,)
+
+
 @pytest.mark.parametrize("site", CHUNK_SHAPE_SITES)
 def test_chunk_shape_read_as_array_shape_rejects_negative(site: str) -> None:
     with pytest.raises(ValueError, match="Expected all values to be non-negative"):

tests/test_metadata/test_v3.py

@@ -31,6 +31,7 @@
     MetadataValidationError,
     NodeTypeValidationError,
     UnknownCodecError,
+    ZarrDeprecationWarning,
 )
 
 if TYPE_CHECKING:
@@ -89,13 +90,32 @@ def test_parse_dimension_names_valid(data: Any) -> None:
         assert result == tuple(data)
 
 
-@pytest.mark.parametrize("data", [[1, 2, "a"], [None, 3]])
+@pytest.mark.parametrize("data", [[1, 2, "a"], [None, 3], "xy"])
 def test_parse_dimension_names_invalid(data: Any) -> None:
     """Iterables containing non-string elements are rejected."""
     with pytest.raises(TypeError, match="Expected either None or"):
         parse_dimension_names(data)
 
 
+@pytest.mark.parametrize(
+    ("data", "expected"), [([], {}), ([["a", 1]], {"a": 1}), ({1: "x"}, {1: "x"})]
+)
+def test_parse_attributes_lenient_deprecated(data: Any, expected: dict[str, Any]) -> None:
+    """Non-mapping attributes still parse as before, but warn."""
+    from zarr.core.metadata.common import parse_attributes
+
+    with pytest.warns(ZarrDeprecationWarning, match="mapping with string keys"):
+        assert parse_attributes(data) == expected
+
+
+@pytest.mark.parametrize("data", [{}, {"a": 1}])
+def test_parse_attributes_valid(data: Any) -> None:
+    """Dicts with string keys are accepted without a warning."""
+    from zarr.core.metadata.common import parse_attributes
+
+    assert parse_attributes(data) == data
+
+
 def test_parse_codecs_unknown_raises(monkeypatch: pytest.MonkeyPatch) -> None:
     """An unregistered codec name raises UnknownCodecError."""
     from collections import defaultdict