Skip to content

Commit 5bb1f97

Browse files
committed
refactor(metadata): check stored chunk shapes against the array shape in one routine
The legacy zero-chunk policy was written out twice: inline in `ArrayV2Metadata.__init__`, and again in a Zarr format 3 helper. The V2 copy also zipped with `strict=False` and re-appended any trailing chunk entries only so that a separate length check, `parse_metadata`, could report a dimensionality mismatch after construction. `parse_stored_chunk_shape` in `zarr.core.metadata.common` is now the one place a stored chunk shape is checked against its array's shape, for both formats: one entry per axis, every integer chunk size at least 1, and a size of 0 (or JSON `false`) on a zero-length axis read as 1 with a warning that names the writer and how to re-save. Non-integer entries, such as edge lists, pass through for the caller's own parser. `ArrayV2Metadata.__init__` calls it directly and `parse_metadata` is gone. The Zarr format 3 adapter only locates a regular grid's `chunk_shape` in the stored document and hands it over; it still runs in `ArrayV3Metadata.__init__` because chunk grid metadata has no array shape. The Zarr format 3 import changes that existed only for the old helper are reverted. Tests for the policy now target the routine: one table of valid and legacy inputs, and one test per rejection (dimension mismatch, zero on a non-empty axis, negative). They replace metadata-level tests in test_v2.py and test_v3.py that only re-tested the same rules; the end-to-end tests still cover both formats' wiring against stored arrays. Assisted-by: ClaudeCode:claude-opus-5
1 parent 49564dc commit 5bb1f97

6 files changed

Lines changed: 139 additions & 134 deletions

File tree

‎src/zarr/core/metadata/common.py‎

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,14 @@
11
from __future__ import annotations
22

3-
from typing import TYPE_CHECKING, Final
3+
import numbers
4+
import warnings
5+
from typing import TYPE_CHECKING, Any, Final
6+
7+
from zarr.errors import ZarrUserWarning
48

59
if TYPE_CHECKING:
10+
from collections.abc import Sequence
11+
612
from zarr.core.common import JSON
713

814
RESAVE_METADATA_HINT: Final = (
@@ -21,3 +27,45 @@ def parse_attributes(data: dict[str, JSON] | None) -> dict[str, JSON]:
2127
return {}
2228

2329
return dict(data)
30+
31+
32+
def parse_stored_chunk_shape(
33+
chunk_shape: Sequence[Any], shape: Sequence[int], *, legacy_writers: str
34+
) -> tuple[Any, ...]:
35+
"""Validate a stored chunk shape against the array shape.
36+
37+
This is the one place a stored per-axis chunk shape is checked against the
38+
array it belongs to, for both Zarr formats. The chunk shape must have one
39+
entry per array axis, and every integer chunk size must be at least 1.
40+
41+
The exception is a chunk size of 0 on a zero-length axis, which
42+
`legacy_writers` stored for arrays created empty. An empty axis has no
43+
chunk for the size to describe, so it is read as 1 with a
44+
`ZarrUserWarning` that says how to re-save valid metadata. A chunk size of
45+
0 on a positive-length axis is rejected, because the metadata cannot say
46+
how the stored chunks were laid out. JSON `false` counts as 0.
47+
48+
Entries that are not integers, such as explicit chunk edge lists, are
49+
returned unchanged for the caller's own parser.
50+
"""
51+
if len(chunk_shape) != len(shape):
52+
raise ValueError(
53+
f"The chunk shape {tuple(chunk_shape)} and the array shape {tuple(shape)} "
54+
"must have the same number of dimensions."
55+
)
56+
parsed: list[Any] = []
57+
for dim_idx, (size, extent) in enumerate(zip(chunk_shape, shape, strict=True)):
58+
if isinstance(size, numbers.Integral) and size < 1:
59+
if size < 0 or extent != 0:
60+
raise ValueError(
61+
f"Dimension {dim_idx}: chunk edge length must be >= 1, got {size!r}"
62+
)
63+
warnings.warn(
64+
f"Dimension {dim_idx}: chunk edge length {size!r} on a zero-length axis "
65+
f"(as written by {legacy_writers}) is treated as 1. {RESAVE_METADATA_HINT}",
66+
ZarrUserWarning,
67+
stacklevel=3,
68+
)
69+
size = 1
70+
parsed.append(size)
71+
return tuple(parsed)

‎src/zarr/core/metadata/v2.py‎

Lines changed: 6 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
)
4343
from zarr.core.config import config, parse_indexing_order
4444
from zarr.core.json_parse import parse_field
45-
from zarr.core.metadata.common import RESAVE_METADATA_HINT, parse_attributes
45+
from zarr.core.metadata.common import parse_attributes, parse_stored_chunk_shape
4646

4747

4848
class ArrayV2MetadataDict(TypedDict):
@@ -88,32 +88,11 @@ def __init__(
8888
Metadata for a Zarr format 2 array.
8989
"""
9090
shape_parsed = parse_shapelike(shape)
91-
chunks_parsed = parse_shapelike(chunks)
92-
# Same invariant as the Zarr format 3 chunk grid metadata: every chunk edge
93-
# length is at least 1. zarr-python 2.18.7, and 3.x before 3.4, can write
94-
# `chunks: [0]` for a zero-length axis (e.g. `chunks=False`, `-1` or `(0,)`).
95-
# Normalize that empty axis to chunk size 1 so it can use the positive-size
96-
# grid model. This is a compatibility policy for legacy metadata, not a
97-
# statement about every historical reader. A zero chunk size on a
98-
# positive-length axis is rejected; metadata alone cannot establish
99-
# whether the store contains chunk payloads.
100-
normalized_chunks: list[int] = []
101-
for dim_idx, (extent, chunk) in enumerate(zip(shape_parsed, chunks_parsed, strict=False)):
102-
if chunk < 1:
103-
if chunk < 0 or extent != 0:
104-
raise ValueError(
105-
f"Dimension {dim_idx}: chunk edge length must be >= 1, got {chunk}"
106-
)
107-
warnings.warn(
108-
f"Dimension {dim_idx}: chunk edge length 0 on a zero-length axis "
109-
"(as written by zarr-python 2.x, and by 3.x before 3.4) is treated "
110-
f"as 1. {RESAVE_METADATA_HINT}",
111-
ZarrUserWarning,
112-
stacklevel=2,
113-
)
114-
chunk = 1
115-
normalized_chunks.append(chunk)
116-
chunks_parsed = tuple(normalized_chunks) + chunks_parsed[len(shape_parsed) :]
91+
chunks_parsed = parse_stored_chunk_shape(
92+
parse_shapelike(chunks),
93+
shape_parsed,
94+
legacy_writers="zarr-python 2.x, and by 3.x before 3.4",
95+
)
11796
compressor_parsed = parse_compressor(compressor)
11897
order_parsed = parse_indexing_order(order)
11998
dimension_separator_parsed = parse_separator(dimension_separator)
@@ -136,7 +115,6 @@ def __init__(
136115
object.__setattr__(self, "attributes", attributes_parsed)
137116

138117
# ensure that the metadata document is consistent
139-
_ = parse_metadata(self)
140118

141119
@property
142120
def ndim(self) -> int:
@@ -348,16 +326,6 @@ def parse_compressor(data: object) -> Numcodec | None:
348326
raise ValueError(msg)
349327

350328

351-
def parse_metadata(data: ArrayV2Metadata) -> ArrayV2Metadata:
352-
if (l_chunks := len(data.chunks)) != (l_shape := len(data.shape)):
353-
msg = (
354-
f"The `shape` and `chunks` attributes must have the same length. "
355-
f"`chunks` has length {l_chunks}, but `shape` has length {l_shape}."
356-
)
357-
raise ValueError(msg)
358-
return data
359-
360-
361329
def get_object_codec_id(maybe_object_codecs: Sequence[JSON]) -> str | None:
362330
"""
363331
Inspect a sequence of codecs / filters for an "object codec", i.e. a codec

‎src/zarr/core/metadata/v3.py‎

Lines changed: 12 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,10 @@
11
from __future__ import annotations
22

33
import json
4-
import warnings
54
from collections.abc import Iterable, Mapping, Sequence
65
from dataclasses import dataclass, field, replace
76
from typing import TYPE_CHECKING, Any, Final, Literal, NotRequired, TypeGuard, cast
87

9-
import numpy as np
108
from typing_extensions import TypedDict
119

1210
from zarr.abc.codec import ArrayArrayCodec, ArrayBytesCodec, BytesBytesCodec, Codec
@@ -37,8 +35,8 @@
3735
from zarr.core.dtype import VariableLengthUTF8, ZDType, get_data_type_from_json
3836
from zarr.core.dtype.common import check_dtype_spec_v3
3937
from zarr.core.json_parse import parse_field
40-
from zarr.core.metadata.common import RESAVE_METADATA_HINT, parse_attributes
41-
from zarr.errors import MetadataValidationError, NodeTypeValidationError, ZarrUserWarning
38+
from zarr.core.metadata.common import parse_attributes, parse_stored_chunk_shape
39+
from zarr.errors import MetadataValidationError, NodeTypeValidationError
4240
from zarr.registry import get_codec_class
4341

4442
if TYPE_CHECKING:
@@ -482,17 +480,12 @@ def _read_legacy_zero_chunk_sizes(
482480
chunk_grid: dict[str, JSON] | ChunkGridMetadata | NamedConfig[str, Any],
483481
shape: tuple[int, ...],
484482
) -> dict[str, JSON] | ChunkGridMetadata | NamedConfig[str, Any]:
485-
"""Read a stored regular chunk size of 0 on a zero-length axis as 1.
486-
487-
zarr-python 3.0 and 3.1 stored `chunk_shape: [0]` for an array created with
488-
a zero-length axis and `chunks=(0,)` or `chunks=False` (3.0 stored `false`
489-
for the latter). Chunk sizes must be at least 1, and a zero-length axis has
490-
no chunks for the size to describe until the array grows, so the size is
491-
read as 1 with a warning. This is the policy `ArrayV2Metadata` applies to
492-
Zarr format 2 metadata. It needs the array shape, which the chunk grid
493-
metadata does not carry, so it runs here rather than in the grid parser.
494-
A zero chunk size on a positive-length axis is left for that parser to
495-
reject.
483+
"""Check a stored regular grid's chunk shape against the array shape.
484+
485+
zarr-python 3.0 and 3.1 stored `chunk_shape: [0]` (and 3.0 `[false]`) for
486+
an array created with a zero-length axis; `parse_stored_chunk_shape`
487+
decides what that means. It needs the array shape, which chunk grid
488+
metadata does not carry, so this runs here rather than in the grid parser.
496489
"""
497490
if not isinstance(chunk_grid, Mapping) or chunk_grid.get("name") != "regular":
498491
return chunk_grid
@@ -502,25 +495,10 @@ def _read_legacy_zero_chunk_sizes(
502495
chunk_shape = configuration.get("chunk_shape")
503496
if not isinstance(chunk_shape, Sequence) or isinstance(chunk_shape, str):
504497
return chunk_grid
505-
if len(chunk_shape) != len(shape):
506-
return chunk_grid # the dimensionality check reports this
507-
normalized = list(chunk_shape)
508-
changed = False
509-
for dim_idx, (size, extent) in enumerate(zip(chunk_shape, shape, strict=True)):
510-
if isinstance(size, int | np.integer) and size == 0 and extent == 0:
511-
warnings.warn(
512-
f"Dimension {dim_idx}: chunk edge length {size!r} on a zero-length axis "
513-
"(as written by zarr-python 3.0 and 3.1) is treated as 1. "
514-
f"{RESAVE_METADATA_HINT}",
515-
ZarrUserWarning,
516-
stacklevel=2,
517-
)
518-
normalized[dim_idx] = 1
519-
changed = True
520-
if not changed:
521-
return chunk_grid
522-
corrected: dict[str, JSON] = {**configuration, "chunk_shape": normalized}
523-
return {"name": "regular", "configuration": corrected}
498+
parsed = parse_stored_chunk_shape(chunk_shape, shape, legacy_writers="zarr-python 3.0 and 3.1")
499+
corrected: dict[str, Any] = dict(chunk_grid)
500+
corrected["configuration"] = {**configuration, "chunk_shape": list(parsed)}
501+
return corrected
524502

525503

526504
@dataclass(frozen=True, kw_only=True)

‎tests/test_metadata/test_common.py‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
"""Tests for metadata helpers shared by both Zarr formats."""
2+
3+
from __future__ import annotations
4+
5+
import warnings
6+
from typing import Any
7+
8+
import numpy as np
9+
import pytest
10+
11+
from zarr.core.metadata.common import parse_stored_chunk_shape
12+
from zarr.errors import ZarrUserWarning
13+
14+
15+
@pytest.mark.parametrize(
16+
("chunk_shape", "shape", "expected", "warns"),
17+
[
18+
((4, 5), (10, 10), (4, 5), False),
19+
((1, 1), (0, 0), (1, 1), False),
20+
((0,), (0,), (1,), True),
21+
((False,), (0,), (1,), True),
22+
((np.int64(0),), (0,), (1,), True),
23+
((4, 0), (4, 0), (4, 1), True),
24+
((0, 0), (0, 0), (1, 1), True),
25+
((2, [5, 10, 5]), (6, 20), (2, [5, 10, 5]), False),
26+
],
27+
ids=[
28+
"valid",
29+
"valid-on-empty-axes",
30+
"legacy-zero",
31+
"legacy-json-false",
32+
"legacy-numpy-zero",
33+
"only-empty-axis-corrected",
34+
"every-empty-axis-corrected",
35+
"edge-list-passed-through",
36+
],
37+
)
38+
def test_parse_stored_chunk_shape(
39+
chunk_shape: tuple[Any, ...], shape: tuple[int, ...], expected: tuple[Any, ...], warns: bool
40+
) -> None:
41+
"""A valid chunk shape is returned as is; a chunk size of 0 on a zero-length
42+
axis is read as 1, with a warning naming the writer and how to re-save."""
43+
with warnings.catch_warnings(record=True) as record:
44+
warnings.simplefilter("always")
45+
parsed = parse_stored_chunk_shape(chunk_shape, shape, legacy_writers="an old writer")
46+
assert parsed == expected
47+
messages = [str(w.message) for w in record if issubclass(w.category, ZarrUserWarning)]
48+
assert bool(messages) is warns
49+
for message in messages:
50+
assert "an old writer" in message
51+
assert "update_attributes({})" in message
52+
53+
54+
def test_parse_stored_chunk_shape_rejects_dimension_mismatch() -> None:
55+
"""The chunk shape needs one entry per array axis."""
56+
with pytest.raises(ValueError, match="same number of dimensions"):
57+
parse_stored_chunk_shape((4,), (10, 10), legacy_writers="an old writer")
58+
59+
60+
@pytest.mark.parametrize(("chunk_shape", "shape"), [((0,), (5,)), ((4, 0), (4, 3))])
61+
def test_parse_stored_chunk_shape_rejects_zero_on_nonempty_axis(
62+
chunk_shape: tuple[int, ...], shape: tuple[int, ...]
63+
) -> None:
64+
"""A chunk size of 0 is only tolerated on a zero-length axis."""
65+
with pytest.raises(ValueError, match="chunk edge length must be >= 1, got 0"):
66+
parse_stored_chunk_shape(chunk_shape, shape, legacy_writers="an old writer")
67+
68+
69+
def test_parse_stored_chunk_shape_rejects_negative() -> None:
70+
"""A negative chunk size is rejected even on a zero-length axis."""
71+
with pytest.raises(ValueError, match="chunk edge length must be >= 1, got -1"):
72+
parse_stored_chunk_shape((-1,), (0,), legacy_writers="an old writer")

‎tests/test_metadata/test_v2.py‎

Lines changed: 0 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -309,35 +309,6 @@ def test_from_dict_extra_fields() -> None:
309309
assert result == expected
310310

311311

312-
@pytest.mark.parametrize(
313-
("shape", "chunks", "expected"),
314-
[((0,), (0,), (1,)), ((4, 0), (4, 0), (4, 1)), ((0, 0), (0, 0), (1, 1))],
315-
)
316-
def test_zero_chunk_edge_on_empty_axis_normalized(
317-
shape: tuple[int, ...], chunks: tuple[int, ...], expected: tuple[int, ...]
318-
) -> None:
319-
"""A stored chunk edge of 0 on a zero-length axis is read as 1, with a warning.
320-
321-
This checks the compatibility policy for legacy metadata: the in-memory
322-
chunk size becomes positive while the extent remains zero. It does not
323-
test historical reader behavior or perform array I/O.
324-
"""
325-
with pytest.warns(ZarrUserWarning, match="chunk edge length 0 on a zero-length axis"):
326-
meta = ArrayV2Metadata(
327-
shape=shape, dtype=Float64(), chunks=chunks, fill_value=0.0, order="C"
328-
)
329-
assert meta.chunks == expected
330-
331-
332-
@pytest.mark.parametrize(("shape", "chunks"), [((5,), (0,)), ((4, 3), (4, 0))])
333-
def test_zero_chunk_edge_with_positive_extent_rejected(
334-
shape: tuple[int, ...], chunks: tuple[int, ...]
335-
) -> None:
336-
"""A chunk edge of 0 on a positive-length axis is rejected."""
337-
with pytest.raises(ValueError, match="chunk edge length must be >= 1"):
338-
ArrayV2Metadata(shape=shape, dtype=Float64(), chunks=chunks, fill_value=0.0, order="C")
339-
340-
341312
def test_eq_nan_fill_value() -> None:
342313
"""Two metadata objects with an identical NaN fill_value compare equal.
343314

‎tests/test_metadata/test_v3.py‎

Lines changed: 0 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -463,35 +463,3 @@ def test_group_metadata_to_dict_consolidated(attributes: dict[str, Any] | None)
463463
},
464464
},
465465
}
466-
467-
468-
@pytest.mark.parametrize(
469-
("shape", "chunk_shape", "expected"),
470-
[
471-
((0,), [0], (1,)),
472-
((4, 0), [4, 0], (4, 1)),
473-
((0, 0), [0, 0], (1, 1)),
474-
((0,), [False], (1,)),
475-
],
476-
ids=["1d", "one-empty-axis", "all-empty-axes", "json-false"],
477-
)
478-
def test_zero_chunk_size_on_empty_axis_normalized(
479-
shape: tuple[int, ...], chunk_shape: list[Any], expected: tuple[int, ...]
480-
) -> None:
481-
"""A stored regular chunk size of 0 on a zero-length axis is read as 1, with a warning.
482-
483-
zarr-python 3.0 and 3.1 wrote this for an array created with a zero-length
484-
axis; 3.0 wrote JSON `false` for `chunks=False`. This is the Zarr format 3
485-
counterpart of the policy `ArrayV2Metadata` applies.
486-
"""
487-
from zarr.core.metadata.v3 import RegularChunkGridMetadata
488-
from zarr.errors import ZarrUserWarning
489-
490-
d = minimal_metadata_dict_v3(
491-
shape=shape,
492-
chunk_grid={"name": "regular", "configuration": {"chunk_shape": chunk_shape}},
493-
)
494-
with pytest.warns(ZarrUserWarning, match="zero-length axis"):
495-
meta = ArrayV3Metadata.from_dict(d) # type: ignore[arg-type]
496-
assert isinstance(meta.chunk_grid, RegularChunkGridMetadata)
497-
assert meta.chunk_grid.chunk_shape == expected

0 commit comments

Comments
 (0)