Skip to content

Commit 26a6287

Browse files
committed
refactor(chunk-grids): compare RunLengthEdges only with RunLengthEdges
A `RunLengthEdges` is not a `tuple`, so `==` against one is now false instead of an edge-by-edge comparison. This also removes the mismatch between equality and hashing. Tests compare `tuple(edges)` or whole metadata objects instead. Assisted-by: ClaudeCode:claude-fable-5.1
1 parent 7ddb469 commit 26a6287

6 files changed

Lines changed: 28 additions & 32 deletions

File tree

‎changes/4479.bugfix.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
Opening an array with a rectilinear chunk grid, and indexing it, now costs time and memory proportional to the size of the grid's run-length encoded metadata rather than to its number of chunks: a stored grid such as `[[1, 1000000000000]]`, a few hundred bytes of JSON, previously took seconds per ten million chunks to open and could exhaust memory. The explicit edges in `RectilinearChunkGridMetadata.chunk_shapes` and `VaryingDimension.edges` are now held as `zarr.core.common.RunLengthEdges`, an immutable sequence that reads like the tuple of edges and compares equal to it, but is not a `tuple` instance. `VaryingDimension.cumulative` is now computed on access. The repr of a chunk grid shows a dimension of more than 100 chunks in its run-length encoded form.
1+
Opening an array with a rectilinear chunk grid, and indexing it, now costs time and memory proportional to the size of the grid's run-length encoded metadata rather than to its number of chunks: a stored grid such as `[[1, 1000000000000]]`, a few hundred bytes of JSON, previously took seconds per ten million chunks to open and could exhaust memory. The explicit edges in `RectilinearChunkGridMetadata.chunk_shapes` and `VaryingDimension.edges` are now held as `zarr.core.common.RunLengthEdges`, an immutable sequence of the edges that is not a `tuple` and does not compare equal to one: use `tuple(edges)` to compare against a tuple. `VaryingDimension.cumulative` is now computed on access. The repr of a chunk grid shows a dimension of more than 100 chunks in its run-length encoded form.

‎src/zarr/core/common.py‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -331,15 +331,14 @@ class RunLengthEdges(Sequence[int]):
331331
The sequence is held as `(size, count)` runs, with adjacent runs of the same size
332332
merged, so construction, lookups and prefix sums cost time and memory in the number
333333
of runs, not in the number of edges: `RunLengthEdges([(1, 2**40)])` is one run. It
334-
reads like the tuple of edges it stands for (`len`, indexing, iteration, and `==`
335-
against a `tuple`), but only iteration and the comparison with a `tuple` visit every
336-
edge.
334+
reads like the sequence of edges it stands for (`len`, indexing, iteration), but
335+
only iteration visits every edge.
337336
338337
Sizes are not checked here: what a valid edge length is, and how to report an invalid
339338
one, is up to the caller (see `parse_chunk_edge`). Each count must be at least 1.
340339
341-
Instances hash by their runs, so an instance does not hash like the `tuple` it
342-
compares equal to.
340+
It is not a `tuple` and equals only another `RunLengthEdges` with the same edges;
341+
compare `tuple(edges)` against a tuple.
343342
"""
344343

345344
__slots__ = ("_index_stops", "_lookup_tables", "_offset_stops", "counts", "sizes")
@@ -430,10 +429,6 @@ def __getitem__(self, index: int | slice) -> int | RunLengthEdges:
430429
def __eq__(self, other: object) -> bool:
431430
if isinstance(other, RunLengthEdges):
432431
return self.sizes == other.sizes and self.counts == other.counts
433-
if isinstance(other, tuple):
434-
return len(other) == self.num_edges and all(
435-
a == b for a, b in zip(self, other, strict=True)
436-
)
437432
return NotImplemented
438433

439434
def __hash__(self) -> int:

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -314,8 +314,8 @@ class RectilinearChunkGridMetadata(Metadata):
314314
(the spec's single-integer shorthand).
315315
- A sequence of ``int`` — explicit per-chunk edge lengths. It is given
316316
as a ``tuple`` or a ``list`` and held as a `RunLengthEdges`, which
317-
reads like the tuple of edges (and compares equal to it) but stores
318-
one entry per run of equal edges, so a grid costs time and memory in
317+
reads like the sequence of edges (but is not a ``tuple``, and does
318+
not compare equal to one) and stores one entry per run of equal edges, so a grid costs time and memory in
319319
the size of its run-length encoded form, not in its number of chunks.
320320
321321
This distinction matters for faithful round-tripping: a bare int

‎tests/test_chunk_grids.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ def _assert_chunks_equal(
4444
assert isinstance(a, VaryingDimension), (
4545
f"axis {axis}: expected VaryingDimension, got {a!r}"
4646
)
47-
assert a.edges == tuple(e), f"axis {axis}: edges {a.edges} != {tuple(e)}"
47+
assert tuple(a.edges) == tuple(e), f"axis {axis}: edges {a.edges} != {tuple(e)}"
4848
assert a.extent == span, f"axis {axis}: extent {a.extent} != {span}"
4949

5050

‎tests/test_properties.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -618,7 +618,7 @@ def test_chunks_param_from_rectilinear_bare_int_roundtrip() -> None:
618618
src = zarr.create_array(MemoryStore(), shape=(3, 3), chunks=([1, 2], 1), dtype="uint8")
619619
grid = src.metadata.chunk_grid # type: ignore[union-attr]
620620
assert isinstance(grid, RectilinearChunkGridMetadata)
621-
assert grid.chunk_shapes == ((1, 2), 1)
621+
assert grid == RectilinearChunkGridMetadata(chunk_shapes=((1, 2), 1))
622622
chunks = chunks_param_from_rectilinear(grid)
623623
# a list of lists, not tuples (`[1, 2] != (1, 2)`)
624624
assert chunks == [[1, 2], 1]
@@ -646,7 +646,7 @@ def test_rectilinear_chunk_grid_declarations(data: st.DataObject) -> None:
646646
"configuration": {"kind": "inline", "chunk_shapes": declaration},
647647
}
648648
meta = RectilinearChunkGridMetadata.from_dict(stored)
649-
assert meta.chunk_shapes == chunk_shapes
649+
assert meta == RectilinearChunkGridMetadata(chunk_shapes=chunk_shapes)
650650

651651
serialized = json.loads(json.dumps(meta.to_dict()))
652652
assert serialized["name"] == "rectilinear"

‎tests/test_unified_chunk_grid.py‎

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,7 @@ def test_fixed_dimension_rejects_invalid(size: int, extent: int, match: str) ->
250250
def test_varying_dimension_construction() -> None:
251251
"""VaryingDimension stores edges, cumulative sums, nchunks, and extent correctly"""
252252
d = VaryingDimension([10, 20, 30], extent=60)
253-
assert d.edges == (10, 20, 30)
253+
assert tuple(d.edges) == (10, 20, 30)
254254
assert d.cumulative == (10, 30, 60)
255255
assert d.nchunks == 3
256256
assert d.extent == 60
@@ -646,7 +646,7 @@ def test_rle_expand_names_dimension(
646646
def test_parse_rle(rle_input: list[Any]) -> None:
647647
"""parse_rle reads the edges expand_rle expands, as merged runs"""
648648
parsed = parse_rle(rle_input)
649-
assert parsed == tuple(expand_rle(rle_input))
649+
assert list(parsed) == expand_rle(rle_input)
650650
assert parsed.to_rle() == compress_rle(expand_rle(rle_input))
651651

652652

@@ -667,30 +667,31 @@ def test_parse_rle(rle_input: list[Any]) -> None:
667667
ids=["one-edge", "one-run", "mixed-runs", "no-repeats", "adjacent-equal-runs"],
668668
)
669669
def test_run_length_edges(runs: list[tuple[int, int]]) -> None:
670-
"""RunLengthEdges reads like the tuple of edges it encodes: length, indexing, slicing,
671-
iteration, equality, prefix sums and position lookups all agree with the expanded
672-
edges, and adjacent runs of one size are merged"""
670+
"""RunLengthEdges reads like the sequence of edges it encodes: length, indexing,
671+
slicing, iteration, prefix sums and position lookups all agree with the expanded
672+
edges, adjacent runs of one size are merged, and it equals only a RunLengthEdges of
673+
the same edges, never the tuple of them"""
673674
expanded = tuple(size for size, count in runs for _ in range(count))
674675
offsets = (0, *itertools.accumulate(expanded))
675676
edges = RunLengthEdges(runs)
676677

677-
assert edges == expanded
678+
assert edges != expanded
678679
assert edges == RunLengthEdges.from_edges(expanded)
679680
assert hash(edges) == hash(RunLengthEdges.from_edges(expanded))
680-
assert edges != (*expanded, 1)
681+
assert edges != RunLengthEdges.from_edges((*expanded, 1))
681682
assert pickle.loads(pickle.dumps(edges)) == edges
682683
assert all(a != b for (a, _), (b, _) in itertools.pairwise(edges.runs))
683684
assert len(edges) == edges.num_edges == len(expanded)
684685
assert edges.total == sum(expanded)
685686
assert tuple(edges) == expanded
686687
assert [edges[i] for i in range(-len(expanded), len(expanded))] == [*expanded, *expanded]
687-
assert edges[1:] == expanded[1:]
688-
assert edges[:-1] == expanded[:-1]
689-
assert edges[::2] == expanded[::2]
688+
assert tuple(edges[1:]) == expanded[1:]
689+
assert tuple(edges[:-1]) == expanded[:-1]
690+
assert tuple(edges[::2]) == expanded[::2]
690691
assert all(edges.count(size) == expanded.count(size) for size in {*expanded, 99})
691692
assert all((size in edges) == (size in expanded) for size in {*expanded, 99})
692693
assert edges.to_rle() == compress_rle(expanded)
693-
assert edges.with_edge(7) == (*expanded, 7)
694+
assert tuple(edges.with_edge(7)) == (*expanded, 7)
694695
assert [edges.offset_of(i) for i in range(len(expanded) + 1)] == list(offsets)
695696
positions = np.arange(-2, edges.total + 3, dtype=np.intp)
696697
expected_indices = np.searchsorted(offsets[1:], positions, side="right")
@@ -1352,7 +1353,7 @@ def test_mixed_scalar_and_list_dims_keep_shorthand(tmp_path: Path) -> None:
13521353
assert isinstance(arr.metadata, ArrayV3Metadata)
13531354
grid = arr.metadata.chunk_grid
13541355
assert isinstance(grid, RectilinearChunkGridMetadata)
1355-
assert grid.chunk_shapes == (5, (10, 20, 70))
1356+
assert grid == RectilinearChunkGridMetadata(chunk_shapes=(5, (10, 20, 70)))
13561357

13571358

13581359
# The chunk/shard spec matrix for `from_array` round-trip tests. Every kind of
@@ -1599,7 +1600,7 @@ def test_from_array_keep_is_o1_in_chunk_count() -> None:
15991600
dst = zarr.from_array(MemoryStore(), data=src, name="0", write_data=False)
16001601
grid = dst.metadata.chunk_grid
16011602
assert isinstance(grid, RectilinearChunkGridMetadata)
1602-
assert grid.chunk_shapes == (3, (10, 20))
1603+
assert grid == RectilinearChunkGridMetadata(chunk_shapes=(3, (10, 20)))
16031604

16041605

16051606
@pytest.mark.parametrize(
@@ -1624,7 +1625,7 @@ def test_resize_uniform_rectilinear_appends_edge(
16241625
assert isinstance(rect.metadata, ArrayV3Metadata)
16251626
rect_grid = rect.metadata.chunk_grid
16261627
assert isinstance(rect_grid, RectilinearChunkGridMetadata)
1627-
assert rect_grid.chunk_shapes == ((10, 10, 10, 15),)
1628+
assert rect_grid == RectilinearChunkGridMetadata(chunk_shapes=((10, 10, 10, 15),))
16281629
assert rect.nchunks == 4
16291630
# the appended region is exactly the new edge chunk
16301631
rect[30:45] = 1
@@ -2770,7 +2771,7 @@ def test_update_shape_shrink_creates_boundary() -> None:
27702771
new_grid = grid.update_shape((45,))
27712772
dim = new_grid._dimensions[0]
27722773
assert isinstance(dim, VaryingDimension)
2773-
assert dim.edges == (10, 20, 30)
2774+
assert tuple(dim.edges) == (10, 20, 30)
27742775
assert dim.extent == 45
27752776
assert dim.chunk_size(2) == 30
27762777
assert dim.data_size(2) == 15
@@ -2782,7 +2783,7 @@ def test_update_shape_shrink_to_exact_boundary() -> None:
27822783
new_grid = grid.update_shape((30,))
27832784
dim = new_grid._dimensions[0]
27842785
assert isinstance(dim, VaryingDimension)
2785-
assert dim.edges == (10, 20, 30)
2786+
assert tuple(dim.edges) == (10, 20, 30)
27862787
assert dim.nchunks == 2
27872788
assert dim.ngridcells == 3
27882789
assert dim.extent == 30
@@ -3314,7 +3315,7 @@ def test_rectilinear_from_dict(
33143315
) -> None:
33153316
"""RectilinearChunkGridMetadata.from_dict correctly parses all spec forms."""
33163317
grid = RectilinearChunkGridMetadata.from_dict(json_input)
3317-
assert grid.chunk_shapes == expected_chunk_shapes
3318+
assert grid == RectilinearChunkGridMetadata(chunk_shapes=expected_chunk_shapes)
33183319

33193320

33203321
@pytest.mark.parametrize(

0 commit comments

Comments
 (0)