Skip to content

Commit f93193d

Browse files
committed
refactor(metadata): scope the stored chunk shape check to regular chunk grids
`parse_stored_chunk_shape` passed non-integer entries through "for the caller's own parser", which made a regular-grid policy look like a general chunk shape routine and let it decide what a 0-length chunk means for grids it does not own. A rectilinear grid, or any other grid, is free to define its own semantics for 0-length chunks. It is now `parse_stored_regular_chunk_shape`, typed `Sequence[int]`, with no pass-through, and its docstring says it applies to Zarr format 2 `chunks` and Zarr format 3 `regular` grids only. The Zarr format 3 caller hands it a chunk shape only when the grid is named `regular` and every entry is an integer (`_is_regular_chunk_shape`); anything else is not a regular chunk shape and goes to the chunk grid parser untouched. Assisted-by: ClaudeCode:claude-opus-5
1 parent 5bb1f97 commit f93193d

4 files changed

Lines changed: 57 additions & 41 deletions

File tree

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

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
from __future__ import annotations
22

3-
import numbers
43
import warnings
5-
from typing import TYPE_CHECKING, Any, Final
4+
from typing import TYPE_CHECKING, Final
65

76
from zarr.errors import ZarrUserWarning
87

@@ -29,33 +28,32 @@ def parse_attributes(data: dict[str, JSON] | None) -> dict[str, JSON]:
2928
return dict(data)
3029

3130

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.
31+
def parse_stored_regular_chunk_shape(
32+
chunk_shape: Sequence[int], shape: Sequence[int], *, legacy_writers: str
33+
) -> tuple[int, ...]:
34+
"""Validate a stored regular chunk grid's chunk shape against the array shape.
3635
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.
36+
This is for regular chunk grids only: Zarr format 2 `chunks`, and the
37+
`chunk_shape` of a Zarr format 3 `regular` grid. Another chunk grid, such
38+
as the rectilinear grid, is free to define its own meaning for a chunk of
39+
length 0, so its chunk sizes must not be passed here.
4040
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
41+
The chunk shape must have one entry per array axis, and every chunk size
42+
must be at least 1. The exception is a chunk size of 0 on a zero-length
43+
axis, which `legacy_writers` stored for arrays created empty. An empty axis
44+
has no chunk for the size to describe, so it is read as 1 with a
4445
`ZarrUserWarning` that says how to re-save valid metadata. A chunk size of
4546
0 on a positive-length axis is rejected, because the metadata cannot say
4647
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.
5048
"""
5149
if len(chunk_shape) != len(shape):
5250
raise ValueError(
5351
f"The chunk shape {tuple(chunk_shape)} and the array shape {tuple(shape)} "
5452
"must have the same number of dimensions."
5553
)
56-
parsed: list[Any] = []
54+
parsed: list[int] = []
5755
for dim_idx, (size, extent) in enumerate(zip(chunk_shape, shape, strict=True)):
58-
if isinstance(size, numbers.Integral) and size < 1:
56+
if size < 1:
5957
if size < 0 or extent != 0:
6058
raise ValueError(
6159
f"Dimension {dim_idx}: chunk edge length must be >= 1, got {size!r}"

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

Lines changed: 2 additions & 2 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 parse_attributes, parse_stored_chunk_shape
45+
from zarr.core.metadata.common import parse_attributes, parse_stored_regular_chunk_shape
4646

4747

4848
class ArrayV2MetadataDict(TypedDict):
@@ -88,7 +88,7 @@ def __init__(
8888
Metadata for a Zarr format 2 array.
8989
"""
9090
shape_parsed = parse_shapelike(shape)
91-
chunks_parsed = parse_stored_chunk_shape(
91+
chunks_parsed = parse_stored_regular_chunk_shape(
9292
parse_shapelike(chunks),
9393
shape_parsed,
9494
legacy_writers="zarr-python 2.x, and by 3.x before 3.4",

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

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
from dataclasses import dataclass, field, replace
66
from typing import TYPE_CHECKING, Any, Final, Literal, NotRequired, TypeGuard, cast
77

8+
import numpy as np
89
from typing_extensions import TypedDict
910

1011
from zarr.abc.codec import ArrayArrayCodec, ArrayBytesCodec, BytesBytesCodec, Codec
@@ -35,7 +36,7 @@
3536
from zarr.core.dtype import VariableLengthUTF8, ZDType, get_data_type_from_json
3637
from zarr.core.dtype.common import check_dtype_spec_v3
3738
from zarr.core.json_parse import parse_field
38-
from zarr.core.metadata.common import parse_attributes, parse_stored_chunk_shape
39+
from zarr.core.metadata.common import parse_attributes, parse_stored_regular_chunk_shape
3940
from zarr.errors import MetadataValidationError, NodeTypeValidationError
4041
from zarr.registry import get_codec_class
4142

@@ -476,26 +477,43 @@ class ArrayMetadataJSON_V3(TypedDict, extra_items=AllowedExtraField): # type: i
476477
}
477478

478479

479-
def _read_legacy_zero_chunk_sizes(
480+
def _is_regular_chunk_shape(value: object) -> TypeGuard[Sequence[int]]:
481+
"""Whether a stored `chunk_shape` is a regular chunk shape: a sequence of integers.
482+
483+
JSON `false` counts, because `bool` is an integer type.
484+
"""
485+
return (
486+
isinstance(value, Sequence)
487+
and not isinstance(value, str)
488+
and all(isinstance(size, int | np.integer) for size in value)
489+
)
490+
491+
492+
def _parse_stored_regular_chunk_grid(
480493
chunk_grid: dict[str, JSON] | ChunkGridMetadata | NamedConfig[str, Any],
481494
shape: tuple[int, ...],
482495
) -> dict[str, JSON] | ChunkGridMetadata | NamedConfig[str, Any]:
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.
496+
"""Check a stored regular chunk grid's chunk shape against the array shape.
497+
498+
Only a `regular` grid whose `chunk_shape` is all integers is a regular chunk
499+
shape, and only that is handed to `parse_stored_regular_chunk_shape`.
500+
Anything else is not a regular chunk shape and is left for the chunk grid
501+
parser: other grids define their own chunk semantics. zarr-python 3.0 and
502+
3.1 stored `chunk_shape: [0]` (and 3.0 `[false]`) for an array created with
503+
a zero-length axis. This runs here rather than in the grid parser because
504+
it needs the array shape, which chunk grid metadata does not carry.
489505
"""
490506
if not isinstance(chunk_grid, Mapping) or chunk_grid.get("name") != "regular":
491507
return chunk_grid
492508
configuration = chunk_grid.get("configuration")
493509
if not isinstance(configuration, Mapping):
494510
return chunk_grid
495511
chunk_shape = configuration.get("chunk_shape")
496-
if not isinstance(chunk_shape, Sequence) or isinstance(chunk_shape, str):
512+
if not _is_regular_chunk_shape(chunk_shape):
497513
return chunk_grid
498-
parsed = parse_stored_chunk_shape(chunk_shape, shape, legacy_writers="zarr-python 3.0 and 3.1")
514+
parsed = parse_stored_regular_chunk_shape(
515+
chunk_shape, shape, legacy_writers="zarr-python 3.0 and 3.1"
516+
)
499517
corrected: dict[str, Any] = dict(chunk_grid)
500518
corrected["configuration"] = {**configuration, "chunk_shape": list(parsed)}
501519
return corrected
@@ -536,7 +554,7 @@ def __init__(
536554

537555
shape_parsed = parse_shapelike(shape)
538556
chunk_grid_parsed = parse_chunk_grid(
539-
_read_legacy_zero_chunk_sizes(chunk_grid, shape_parsed)
557+
_parse_stored_regular_chunk_grid(chunk_grid, shape_parsed)
540558
)
541559
chunk_key_encoding_parsed = parse_chunk_key_encoding(chunk_key_encoding)
542560
dimension_names_parsed = parse_dimension_names(dimension_names)

‎tests/test_metadata/test_common.py‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
import numpy as np
99
import pytest
1010

11-
from zarr.core.metadata.common import parse_stored_chunk_shape
11+
from zarr.core.metadata.common import parse_stored_regular_chunk_shape
1212
from zarr.errors import ZarrUserWarning
1313

1414

@@ -22,7 +22,6 @@
2222
((np.int64(0),), (0,), (1,), True),
2323
((4, 0), (4, 0), (4, 1), True),
2424
((0, 0), (0, 0), (1, 1), True),
25-
((2, [5, 10, 5]), (6, 20), (2, [5, 10, 5]), False),
2625
],
2726
ids=[
2827
"valid",
@@ -32,17 +31,18 @@
3231
"legacy-numpy-zero",
3332
"only-empty-axis-corrected",
3433
"every-empty-axis-corrected",
35-
"edge-list-passed-through",
3634
],
3735
)
38-
def test_parse_stored_chunk_shape(
36+
def test_parse_stored_regular_chunk_shape(
3937
chunk_shape: tuple[Any, ...], shape: tuple[int, ...], expected: tuple[Any, ...], warns: bool
4038
) -> None:
4139
"""A valid chunk shape is returned as is; a chunk size of 0 on a zero-length
4240
axis is read as 1, with a warning naming the writer and how to re-save."""
4341
with warnings.catch_warnings(record=True) as record:
4442
warnings.simplefilter("always")
45-
parsed = parse_stored_chunk_shape(chunk_shape, shape, legacy_writers="an old writer")
43+
parsed = parse_stored_regular_chunk_shape(
44+
chunk_shape, shape, legacy_writers="an old writer"
45+
)
4646
assert parsed == expected
4747
messages = [str(w.message) for w in record if issubclass(w.category, ZarrUserWarning)]
4848
assert bool(messages) is warns
@@ -51,22 +51,22 @@ def test_parse_stored_chunk_shape(
5151
assert "update_attributes({})" in message
5252

5353

54-
def test_parse_stored_chunk_shape_rejects_dimension_mismatch() -> None:
54+
def test_parse_stored_regular_chunk_shape_rejects_dimension_mismatch() -> None:
5555
"""The chunk shape needs one entry per array axis."""
5656
with pytest.raises(ValueError, match="same number of dimensions"):
57-
parse_stored_chunk_shape((4,), (10, 10), legacy_writers="an old writer")
57+
parse_stored_regular_chunk_shape((4,), (10, 10), legacy_writers="an old writer")
5858

5959

6060
@pytest.mark.parametrize(("chunk_shape", "shape"), [((0,), (5,)), ((4, 0), (4, 3))])
61-
def test_parse_stored_chunk_shape_rejects_zero_on_nonempty_axis(
61+
def test_parse_stored_regular_chunk_shape_rejects_zero_on_nonempty_axis(
6262
chunk_shape: tuple[int, ...], shape: tuple[int, ...]
6363
) -> None:
6464
"""A chunk size of 0 is only tolerated on a zero-length axis."""
6565
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")
66+
parse_stored_regular_chunk_shape(chunk_shape, shape, legacy_writers="an old writer")
6767

6868

69-
def test_parse_stored_chunk_shape_rejects_negative() -> None:
69+
def test_parse_stored_regular_chunk_shape_rejects_negative() -> None:
7070
"""A negative chunk size is rejected even on a zero-length axis."""
7171
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")
72+
parse_stored_regular_chunk_shape((-1,), (0,), legacy_writers="an old writer")

0 commit comments

Comments
 (0)