Skip to content

Commit d43470f

Browse files
committed
Merge branch 'up/4334' into up/4377
2 parents b8c6c5b + 6707b45 commit d43470f

3 files changed

Lines changed: 72 additions & 42 deletions

File tree

‎changes/4334.bugfix.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,6 @@ A chunk edge length is now always at least 1, while an array extent may be 0. Ev
22

33
`FixedDimension(size=0, ...)` now raises a `ValueError`. `ArrayV2Metadata(chunks=(0,))` is still accepted and written as given; an array built from such metadata (with `create_hierarchy`, for example) reads the chunk size as a stored chunk size of 0 is read (below). `create_hierarchy` now builds every array and group before it deletes or stores anything, so a node that cannot be built fails with the store untouched. The regular chunk grid metadata class reads a `bool` chunk edge length as the `int` it equals, and rejects a string or a mapping as a chunk shape as a whole. `RectilinearChunkGridMetadata`, which is experimental, now reads a `bool` edge length as the `int` it equals and rejects a NumPy integer or a float edge length such as `4.0` with a `TypeError`; it used to keep them as given, so that it could not store a NumPy integer, could not read back a `bool`, and stored a float as a JSON float, which the Zarr specification does not allow.
44

5-
Stored metadata with a regular chunk size of 0 or JSON `false` (Zarr format 2 `chunks`, Zarr format 3 `regular` `chunk_shape`) is now read as one chunk spanning the axis (a multiple of the inner chunk size for sharded arrays, which previously failed to open). zarr-python wrote such sizes for arrays created with a zero-length axis until 3.4, and, from 2.18.7 to 3.2.1, for an explicit chunk size of 0 or `False` on an axis of any length, as in `chunks=(0,)` (those arrays could store no data). On a zero-length axis this opens silently, as before; appending to such an axis then stores chunks of size 1. On an axis of positive length it opens with a `ZarrUserWarning`, which says that the axis holds only the fill value and how to store valid metadata: `array.update_attributes({})`, then `zarr.consolidate_metadata` if the metadata is consolidated. JSON `true`, as zarr-python 3.0 and 3.2 wrote for a chunk size of `True`, is read as 1, silently, in the chunk grid (regular, and the explicit edges and run-length encoded sizes of a rectilinear one) and in the inner chunk shape of every sharding codec, nested or not. A stored rectilinear chunk grid whose edge lengths are integral JSON floats (`[[4.0, 2]]`), as zarr-python 3.2 wrote for float edges, is read with those edges as integers, silently; a float anywhere no release wrote one (a regular chunk shape, the inner chunk shape of a sharding codec, a run-length repeat count, an edge below 1) is rejected.
5+
Stored metadata with a regular chunk size of 0 or JSON `false` (Zarr format 2 `chunks`, Zarr format 3 `regular` `chunk_shape`) is now read as chunk size 1 (the inner chunk size for sharded arrays, which previously failed to open), however long the axis is: that is the chunk size `chunks=-1` gives on a zero-length axis, and it does not depend on how far the axis has grown since. zarr-python wrote such sizes for arrays created with a zero-length axis until 3.4, and, from 2.18.7 to 3.2.1, for an explicit chunk size of 0 or `False` on an axis of any length, as in `chunks=(0,)` (those arrays could store no data). On a zero-length axis this opens silently, as before, and appending to it stores chunks of size 1. On an axis of positive length it opens with a `ZarrUserWarning`, which says that the axis holds only the fill value and how to store valid metadata: `array.update_attributes({})`, then `zarr.consolidate_metadata` if the metadata is consolidated. JSON `true`, as zarr-python 3.0 and 3.2 wrote for a chunk size of `True`, is read as 1, silently, in the chunk grid (regular, and the explicit edges and run-length encoded sizes of a rectilinear one) and in the inner chunk shape of every sharding codec, nested or not. A stored rectilinear chunk grid whose edge lengths are integral JSON floats (`[[4.0, 2]]`), as zarr-python 3.2 wrote for float edges, is read with those edges as integers, silently; a float anywhere no release wrote one (a regular chunk shape, the inner chunk shape of a sharding codec, a run-length repeat count, an edge below 1) is rejected.
66

7-
Writing data to an array read from such metadata (other than an empty selection) first stores the upgrade of the metadata the store then holds, unless another writer has stored valid metadata since, so that other readers find the chunks written; in a `ZipStore` this adds a second entry for the metadata document, as every metadata update does. If the metadata the store then holds lays out chunks differently (another writer resized the array keeping the chunk size of 0, say), the write raises a `ValueError` asking to reopen the array, and stores nothing. Apart from the array's own metadata writes (`update_attributes`, `resize`), which store the upgrade as before, this is the only operation that stores it: a group's consolidated metadata keeps such an array's metadata as it was stored (`zarr.consolidate_metadata` copies it as the array's own document holds it) until the array stores its upgrade through the group's handle, and changing a group reads and writes no metadata of its members.
7+
Writing data to an array read from such metadata (other than an empty selection) first stores the upgrade of the metadata the store then holds, unless another writer has stored valid metadata since, so that other readers find the chunks written; in a `ZipStore` this adds a second entry for the metadata document, as every metadata update does. If the metadata the store then holds lays out chunks differently (another writer stored a different chunk size since, say), the write raises a `ValueError` asking to reopen the array, and stores nothing. Apart from the array's own metadata writes (`update_attributes`, `resize`), which store the upgrade as before, this is the only operation that stores it: a group's consolidated metadata keeps such an array's metadata as it was stored (`zarr.consolidate_metadata` copies it as the array's own document holds it) until the array stores its upgrade through the group's handle, and changing a group reads and writes no metadata of its members.

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

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
from typing import TYPE_CHECKING, Final, TypeGuard, cast
2727

2828
from zarr.core._json import json_equal
29-
from zarr.core.chunk_grids import full_span_chunk_size
3029
from zarr.errors import ZarrUserWarning
3130

3231
if TYPE_CHECKING:
@@ -74,24 +73,28 @@ def _read_chunk_size(size: JSON, span: int | None, unit: int) -> tuple[int, str
7473
Returns the edge length and, where the user must act on how it was read, how it was
7574
read; `None` if the entry cannot be read, which leaves it for the metadata
7675
constructors to check. A JSON int >= 1 is kept, JSON `true` is read as 1, and 0 or
77-
JSON `false` is read as one chunk spanning the axis of length `span`, a multiple of
78-
`unit` (the inner chunk size of a shard): on an axis of positive length no chunk
79-
can have been stored under it, so the array holds only its fill value. `span` is
80-
`None` where no stored 0 is known, as in the inner chunk shape of a sharding codec:
81-
0 is then left as stored.
76+
JSON `false` is read as `unit`, the smallest chunk edge length the axis can have (1,
77+
or the inner chunk size of a shard), whatever the length `span` of the axis. That is
78+
what `chunks=-1` gives on an axis of length 0, where these sizes were written, and it
79+
does not depend on how far the axis has grown since. On an axis of positive length no
80+
chunk can have been stored under a chunk size of 0, so the array holds only its fill
81+
value. `span` is `None` where no stored 0 is known, as in the inner chunk shape of a
82+
sharding codec: 0 is then left as stored.
8283
"""
8384
match size:
8485
case True:
8586
return 1, None
8687
case int() if size >= 1:
8788
return size, None
8889
case int() if size == 0 and span is not None:
89-
edge = full_span_chunk_size(span, unit)
9090
if span == 0:
91-
return edge, None
92-
return edge, (
93-
f"one chunk spanning the dimension ({edge}), and as no chunk can be stored "
94-
"under a chunk size of 0, the array holds only its fill value"
91+
return unit, None
92+
return unit, (
93+
"1, as no chunk can be stored under a chunk size of 0, so the array "
94+
"holds only its fill value"
95+
if unit == 1
96+
else f"{unit}, the inner chunk size, as no chunk can be stored under a "
97+
"chunk size of 0, so the array holds only its fill value"
9598
)
9699
return None
97100

‎tests/test_metadata/test_upgrades.py‎

Lines changed: 56 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -123,32 +123,32 @@ def _nested_sharded_doc(inner: list[Any], nested: list[Any]) -> dict[str, JSON]:
123123
(_v3_doc([5, 4], [True, 4]), ((1, 4),), True, None),
124124
(
125125
_v2_doc([3], [0]),
126-
((3,),),
126+
((1,),),
127127
True,
128128
(
129-
r"^The stored chunk shape \[0\] is invalid: .* read as \[3\], reading 0 in "
130-
r"dimension 0 as one chunk spanning the dimension \(3\), and .* holds only "
131-
r"its fill value\.$"
129+
r"^The stored chunk shape \[0\] is invalid: .* read as \[1\], reading 0 in "
130+
r"dimension 0 as 1, as no chunk can be stored under a chunk size of 0, so the "
131+
r"array holds only its fill value\.$"
132132
),
133133
),
134134
(
135135
_v3_doc([4, 3], [4, 0]),
136-
((4, 3),),
136+
((4, 1),),
137137
True,
138138
r"reading 0 in dimension 1 as .* holds only its fill value\.$",
139139
),
140140
(
141141
_v2_doc([0, 3], [0, 0]),
142-
((1, 3),),
142+
((1, 1),),
143143
True,
144-
r"read as \[1, 3\], reading 0 in dimension 1 as .* holds only its fill value\.$",
144+
r"read as \[1, 1\], reading 0 in dimension 1 as .* holds only its fill value\.$",
145145
),
146146
(_v3_doc([0], [0], inner=[4]), ((4,), (4,)), True, None),
147147
(
148148
_v3_doc([10], [0], inner=[4]),
149-
((12,), (4,)),
149+
((4,), (4,)),
150150
True,
151-
r"spanning the dimension \(12\), and .* holds only its fill value",
151+
r"as 4, the inner chunk size, as no chunk .* holds only its fill value",
152152
),
153153
(_v3_doc([0, 3], [0, 3], inner=[2, 3]), ((2, 3), (2, 3)), True, None),
154154
(_v3_doc([5], [True], inner=[True]), ((1,), (1,)), True, None),
@@ -177,8 +177,8 @@ def _nested_sharded_doc(inner: list[Any], nested: list[Any]) -> dict[str, JSON]:
177177
def test_upgrade_array_document(
178178
doc: dict[str, JSON], expected: tuple[Any, ...], upgraded: bool, warning: str | None
179179
) -> None:
180-
"""Valid documents pass unchanged. A stored chunk size of 0 or `false` is read as one
181-
chunk spanning the axis (a multiple of the inner chunk when sharded) and `true` as 1,
180+
"""Valid documents pass unchanged. A stored chunk size of 0 or `false` is read as 1
181+
(the inner chunk size when sharded), however long the axis, and `true` as 1,
182182
in the chunk shape and in the inner chunk shape of every sharding codec, nested or
183183
not. `from_dict` marks the metadata of an upgraded document; it warns once, naming
184184
the array, only where a chunk size of 0 was stored for a non-empty axis (which then
@@ -554,7 +554,7 @@ def _rewrite_doc(path: Path, zarr_format: Literal[2, 3], edit: Any) -> None:
554554
[
555555
(2, (0, 4), [0, 4], None, (1, 4), False),
556556
(3, (5,), [True], None, (1,), False),
557-
(3, (10,), [0], (4,), (12,), True),
557+
(3, (10,), [0], (4,), (4,), True),
558558
],
559559
ids=["v2-empty-2d", "v3-true", "v3-sharded-grown"],
560560
)
@@ -612,7 +612,7 @@ def store_legacy(doc: dict[str, Any]) -> None:
612612

613613
@pytest.mark.parametrize(
614614
("zarr_format", "shape", "inner", "expected"),
615-
[(2, (3,), None, (3,)), (3, (3,), None, (3,)), (3, (10,), (4,), (12,))],
615+
[(2, (3,), None, (1,)), (3, (3,), None, (1,)), (3, (10,), (4,), (4,))],
616616
ids=["v2", "v3", "v3-sharded"],
617617
)
618618
@pytest.mark.parametrize("api", ["sync", "async", "async-concurrent"])
@@ -720,12 +720,12 @@ def test_stale_handle_write_keeps_valid_document_as_written(
720720
with pytest.warns(ZarrUserWarning, match="is read as"):
721721
stale = zarr.open_array(store=path, mode="r+")
722722
# Valid metadata for the same array, as another writer might store it: chunk size
723-
# 3, without the optional members zarr writes, as compact JSON.
723+
# 1, without the optional members zarr writes, as compact JSON.
724724
doc_path = path / (".zarray" if zarr_format == 2 else "zarr.json")
725725
doc = json.loads(doc_path.read_text())
726726
for optional in ("dimension_separator", "attributes", "storage_transformers"):
727727
doc.pop(optional, None)
728-
_stored_chunks(doc)[0] = 3
728+
_stored_chunks(doc)[0] = 1
729729
doc_path.write_text(json.dumps(doc, separators=(",", ":")))
730730
written = doc_path.read_bytes()
731731

@@ -743,14 +743,22 @@ def _resize_to_10(doc: dict[str, Any]) -> None:
743743
doc["shape"] = [10]
744744

745745

746+
def _store_chunk_size_3(doc: dict[str, Any]) -> None:
747+
_stored_chunks(doc)[0] = 3
748+
749+
746750
def _halve_inner_chunk_shape(doc: dict[str, Any]) -> None:
747751
doc["codecs"][0]["configuration"]["chunk_shape"] = [2]
748752

749753

750754
@pytest.mark.parametrize(
751755
("zarr_format", "sharded", "change"),
752-
[(2, False, _resize_to_10), (3, False, _resize_to_10), (3, True, _halve_inner_chunk_shape)],
753-
ids=["v2-resized", "v3-resized", "v3-sharded-inner-chunk-shape"],
756+
[
757+
(2, False, _store_chunk_size_3),
758+
(3, False, _store_chunk_size_3),
759+
(3, True, _halve_inner_chunk_shape),
760+
],
761+
ids=["v2-chunk-size", "v3-chunk-size", "v3-sharded-inner-chunk-shape"],
754762
)
755763
def test_stale_handle_write_after_chunk_grid_change_raises(
756764
tmp_path: Path,
@@ -759,10 +767,9 @@ def test_stale_handle_write_after_chunk_grid_change_raises(
759767
change: Callable[[dict[str, Any]], None],
760768
) -> None:
761769
"""If the document the store holds when a handle read from an upgraded document
762-
first writes chunks lays out chunks differently from the handle's metadata (the
763-
array was resized by software that kept the stored chunk size of 0, which now reads
764-
as a larger chunk, or its inner chunk shape changed), the handle's chunks would not
765-
be found under it: the write raises and stores nothing."""
770+
first writes chunks lays out chunks differently from the handle's metadata (another
771+
writer stored a different chunk size, or changed the inner chunk shape), the handle's
772+
chunks would not be found under it: the write raises and stores nothing."""
766773
path = tmp_path / "legacy.zarr"
767774
if sharded:
768775
zarr.create_array(store=path, shape=(3,), chunks=(4,), shards=(4,), dtype="int16")
@@ -781,6 +788,26 @@ def test_stale_handle_write_after_chunk_grid_change_raises(
781788
assert {p.name: p.read_bytes() for p in path.iterdir()} == documents
782789

783790

791+
@pytest.mark.parametrize("zarr_format", [2, 3])
792+
def test_stale_handle_write_after_resize_keeping_zero(
793+
tmp_path: Path, zarr_format: Literal[2, 3]
794+
) -> None:
795+
"""A stored chunk size of 0 is read as 1 however long the axis is, so a resize by
796+
software that kept it lays out chunks as the handle does: the handle's first write
797+
stores the upgrade of the resized document, then its chunks."""
798+
path = tmp_path / "legacy.zarr"
799+
_legacy_array(path, zarr_format)
800+
with pytest.warns(ZarrUserWarning, match="is read as"):
801+
stale = zarr.open_array(store=path, mode="r+")
802+
_rewrite_doc(path, zarr_format, _resize_to_10)
803+
804+
stale[0:3] = [7, 8, 9]
805+
806+
reopened = _open_strictly(path)
807+
assert (reopened.shape, reopened.chunks) == ((10,), (1,))
808+
np.testing.assert_array_equal(reopened[...], [7, 8, 9, 0, 0, 0, 0, 0, 0, 0])
809+
810+
784811
def _set_attribute(array: AnyArray) -> None:
785812
array.attrs["x"] = 1
786813

@@ -838,7 +865,7 @@ def test_write_without_stored_document(zarr_format: Literal[2, 3]) -> None:
838865
assert not [key for key in store._store_dict if key.endswith((".zarray", "zarr.json"))]
839866

840867

841-
@pytest.mark.parametrize(("shape", "expected"), [((0,), (1,)), ((3,), (3,))])
868+
@pytest.mark.parametrize(("shape", "expected"), [((0,), (1,)), ((3,), (1,))])
842869
def test_array_from_metadata_with_chunk_size_zero(shape: tuple[int], expected: tuple[int]) -> None:
843870
"""`ArrayV2Metadata` accepts a chunk size of 0, as a stored document may hold it. An
844871
array built from such metadata reads it as the upgrades read that document, silently
@@ -1023,7 +1050,7 @@ def test_consolidated_upgraded_member_stored_by_its_first_write(
10231050

10241051
np.testing.assert_array_equal(_open_strictly(path / "a")[...], [7, 8, 9])
10251052
zarr.consolidate_metadata(path)
1026-
assert _stored_chunks(_consolidated_member(path, zarr_format, "a")) == [3]
1053+
assert _stored_chunks(_consolidated_member(path, zarr_format, "a")) == [1]
10271054

10281055

10291056
@pytest.mark.filterwarnings("ignore:Consolidated metadata is currently not part:UserWarning")
@@ -1032,14 +1059,14 @@ def test_consolidated_upgraded_member_write_after_chunk_grid_change_raises(
10321059
tmp_path: Path, zarr_format: Literal[2, 3]
10331060
) -> None:
10341061
"""A member read from its consolidated copy, whose own document has since been
1035-
resized by software that kept the stored chunk size of 0, would write chunks no
1036-
reader finds: its first write raises and stores nothing."""
1062+
stored with a different chunk size, would write chunks no reader finds: its first
1063+
write raises and stores nothing."""
10371064
path = tmp_path / "group.zarr"
10381065
_consolidated_legacy_member(path, zarr_format)
10391066
with pytest.warns(ZarrUserWarning, match="is read as"):
10401067
array = zarr.open_group(path, mode="r+", use_consolidated=True)["a"]
10411068
assert isinstance(array, zarr.Array)
1042-
_rewrite_doc(path / "a", zarr_format, _resize_to_10)
1069+
_rewrite_doc(path / "a", zarr_format, _store_chunk_size_3)
10431070
documents = {p: p.read_bytes() for p in path.rglob("*") if p.is_file()}
10441071

10451072
with pytest.raises(ValueError, match="has changed since this array was opened: reopen"):
@@ -1069,7 +1096,7 @@ def test_consolidated_upgraded_member_attributes_kept_by_group_write(
10691096
warnings.simplefilter("always")
10701097
attributes = dict(zarr.open_group(path, mode="r", use_consolidated=True)["a"].attrs)
10711098
assert attributes == {"x": 1}
1072-
assert _stored_chunks(_consolidated_member(path, zarr_format, "a")) == [3]
1099+
assert _stored_chunks(_consolidated_member(path, zarr_format, "a")) == [1]
10731100
assert not [w for w in record if "is read as" in str(w.message)]
10741101

10751102

@@ -1159,7 +1186,7 @@ def test_legacy_chunk_size_consolidated(tmp_path: Path, zarr_format: Literal[2,
11591186
]
11601187
for array in arrays:
11611188
assert isinstance(array, zarr.Array)
1162-
assert array.chunks == (2,)
1189+
assert array.chunks == (1,)
11631190
array.update_attributes({})
11641191
zarr.consolidate_metadata(path)
11651192
with warnings.catch_warnings():
@@ -1170,4 +1197,4 @@ def test_legacy_chunk_size_consolidated(tmp_path: Path, zarr_format: Literal[2,
11701197
for name in names:
11711198
array = reopened[name]
11721199
assert isinstance(array, zarr.Array)
1173-
assert array.chunks == (2,)
1200+
assert array.chunks == (1,)

0 commit comments

Comments
 (0)