Skip to content

Commit 6808f75

Browse files
committed
refactor(chunk-grids): make RunLengthEdges a plain value, not a sequence
Per-chunk access belongs to `VaryingDimension`, which binds edges to an extent; the metadata model only needs runs, their total and the RLE form. A `Sequence[int]` interface on `RunLengthEdges` invited `for e in edges` or `list(edges)` on a grid with 2**40 chunks. Drop `len`, indexing, slicing, iteration, membership and `count`. Add `size_of(index)` for one edge and `expand()` as the single, explicit way to visit every edge. `RectilinearChunkGridMetadata.chunk_shapes` is now annotated as what it holds, `tuple[int | RunLengthEdges, ...]`, while its constructor still accepts tuples and lists of edges. Assisted-by: ClaudeCode:claude-fable-5.1
1 parent 26a6287 commit 6808f75

10 files changed

Lines changed: 92 additions & 107 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 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.
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 run-length encoded value that is not a sequence: it has no `len`, indexing or iteration and does not compare equal to a `tuple`. Its `expand()` method yields one entry per chunk. `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/codecs/sharding.py‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@
4545
merge_and_encode_chunk,
4646
)
4747
from zarr.core.common import (
48-
RunLengthEdges,
4948
ShapeLike,
5049
parse_named_configuration,
5150
parse_shapelike,
@@ -623,8 +622,7 @@ def validate(
623622
elif isinstance(chunk_grid, RectilinearChunkGridMetadata):
624623
# One size per run of equal edges, not one per chunk.
625624
edges_per_dim = tuple(
626-
(s,) if isinstance(s, int) else RunLengthEdges.from_edges(s).sizes
627-
for s in chunk_grid.chunk_shapes
625+
(s,) if isinstance(s, int) else s.sizes for s in chunk_grid.chunk_shapes
628626
)
629627
else:
630628
raise TypeError(

‎src/zarr/core/chunk_grids.py‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ def _size_repr(self) -> str:
138138
def _edges_repr(edges: RunLengthEdges) -> str:
139139
"""The edges as a tuple, or run-length encoded if there are too many to write out."""
140140
if edges.num_edges <= _MAX_EDGES_IN_REPR:
141-
return repr(tuple(edges))
141+
return repr(tuple(edges.expand()))
142142
return repr(edges.to_rle())
143143

144144

@@ -161,7 +161,7 @@ class VaryingDimension:
161161
nchunks: int = field(init=False, repr=False) # cached at construction
162162
ngridcells: int = field(init=False, repr=False) # cached at construction
163163

164-
def __init__(self, edges: Sequence[int], extent: int) -> None:
164+
def __init__(self, edges: Sequence[int] | RunLengthEdges, extent: int) -> None:
165165
rle = RunLengthEdges.from_edges(edges)
166166
if rle.num_edges == 0:
167167
raise ValueError("VaryingDimension edges must not be empty")
@@ -185,7 +185,7 @@ def cumulative(self) -> tuple[int, ...]:
185185
Lookups do not use this: it is computed on each access, in time and
186186
memory proportional to the number of chunks.
187187
"""
188-
return tuple(itertools.accumulate(self.edges))
188+
return tuple(itertools.accumulate(self.edges.expand()))
189189

190190
def index_to_chunk(self, idx: int) -> int:
191191
if idx < 0 or idx >= self.extent:
@@ -206,23 +206,23 @@ def chunk_size(self, chunk_ix: int) -> int:
206206
Does not validate *chunk_ix* — callers must ensure it is in
207207
``[0, ngridcells)``. Use ``ChunkGrid.__getitem__`` for safe access.
208208
"""
209-
return self.edges[chunk_ix]
209+
return self.edges.size_of(chunk_ix)
210210

211211
def data_size(self, chunk_ix: int) -> int:
212212
"""Valid data region within the buffer — clipped at extent.
213213
214214
Does not validate *chunk_ix* — callers must ensure it is in
215215
``[0, ngridcells)``. Use ``ChunkGrid.__getitem__`` for safe access.
216216
"""
217-
return max(0, min(self.edges[chunk_ix], self.extent - self.chunk_offset(chunk_ix)))
217+
return max(0, min(self.edges.size_of(chunk_ix), self.extent - self.chunk_offset(chunk_ix)))
218218

219219
@property
220220
def _data_sizes(self) -> tuple[int, ...]:
221221
"""`data_size` of every chunk that holds data, in order."""
222222
if self.nchunks == 0:
223223
return ()
224224
last = self.nchunks - 1
225-
return (*itertools.islice(self.edges, last), self.data_size(last))
225+
return (*itertools.islice(self.edges.expand(), last), self.data_size(last))
226226

227227
@property
228228
def _unique_edge_lengths(self) -> Iterable[int]:
@@ -447,7 +447,7 @@ def from_metadata(cls, metadata: ArrayMetadata) -> ChunkGrid:
447447
def from_sizes(
448448
cls,
449449
array_shape: ShapeLike,
450-
chunk_sizes: Sequence[int | Sequence[int]],
450+
chunk_sizes: Sequence[int | Sequence[int] | RunLengthEdges],
451451
) -> ChunkGrid:
452452
"""Create a ChunkGrid from per-dimension chunk size specifications.
453453
@@ -459,7 +459,7 @@ def from_sizes(
459459
Per-dimension chunk sizes. Each element is either:
460460
461461
- An ``int`` — regular (fixed) chunk size for that dimension.
462-
- A ``Sequence[int]`` — explicit per-chunk edge lengths, kept as a
462+
- A ``Sequence[int]`` or a `RunLengthEdges` — explicit per-chunk edge lengths, kept as a
463463
``VaryingDimension`` even when the edges are uniform: the spec
464464
form declares the grid kind, as in `normalize_chunks_1d`.
465465
"""

‎src/zarr/core/common.py‎

Lines changed: 23 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727

2828
if TYPE_CHECKING:
2929
from collections.abc import Awaitable, Callable, Iterator
30-
from typing import Self
3130

3231
import numpy.typing as npt
3332

@@ -325,20 +324,22 @@ def parse_chunk_shape(data: object) -> tuple[int, ...]:
325324
raise TypeError(f"A chunk shape must be an iterable of chunk edge lengths, got {data!r}")
326325

327326

328-
class RunLengthEdges(Sequence[int]):
329-
"""An immutable sequence of chunk edge lengths, stored run-length encoded.
327+
class RunLengthEdges:
328+
"""The chunk edge lengths of one dimension, stored run-length encoded.
330329
331-
The sequence is held as `(size, count)` runs, with adjacent runs of the same size
330+
The edges are held as `(size, count)` runs, with adjacent runs of the same size
332331
merged, so construction, lookups and prefix sums cost time and memory in the number
333-
of runs, not in the number of edges: `RunLengthEdges([(1, 2**40)])` is one run. It
334-
reads like the sequence of edges it stands for (`len`, indexing, iteration), but
335-
only iteration visits every edge.
332+
of runs, not in the number of edges: `RunLengthEdges([(1, 2**40)])` is one run.
333+
334+
This is deliberately not a sequence: there is no `len`, indexing or iteration, so
335+
nothing visits every edge by accident. `size_of`, `offset_of` and `index_at` answer
336+
per-edge questions from the runs, and `expand` is the one method that yields every
337+
edge.
336338
337339
Sizes are not checked here: what a valid edge length is, and how to report an invalid
338340
one, is up to the caller (see `parse_chunk_edge`). Each count must be at least 1.
339341
340-
It is not a `tuple` and equals only another `RunLengthEdges` with the same edges;
341-
compare `tuple(edges)` against a tuple.
342+
Two instances are equal if they encode the same edges.
342343
"""
343344

344345
__slots__ = ("_index_stops", "_lookup_tables", "_offset_stops", "counts", "sizes")
@@ -369,10 +370,10 @@ def __init__(self, runs: Iterable[tuple[int, int]] = ()) -> None:
369370
self._lookup_tables: tuple[npt.NDArray[np.intp], ...] | None = None
370371

371372
@classmethod
372-
def from_edges(cls, edges: Iterable[int]) -> Self:
373-
"""Run-length encode `edges`, one entry per edge. An instance of this class is
374-
returned as it is."""
375-
if isinstance(edges, cls):
373+
def from_edges(cls, edges: Iterable[int] | RunLengthEdges) -> RunLengthEdges:
374+
"""Run-length encode `edges`, one entry per edge. A `RunLengthEdges` is returned
375+
as it is."""
376+
if isinstance(edges, RunLengthEdges):
376377
return edges
377378
return cls((size, sum(1 for _ in group)) for size, group in itertools.groupby(edges))
378379

@@ -383,7 +384,7 @@ def runs(self) -> tuple[tuple[int, int], ...]:
383384

384385
@property
385386
def num_edges(self) -> int:
386-
"""The number of edges. Unlike `len`, this is not limited to `sys.maxsize`."""
387+
"""The number of edges."""
387388
return self._index_stops[-1] if self._index_stops else 0
388389

389390
@property
@@ -394,37 +395,15 @@ def total(self) -> int:
394395
def __reduce__(self) -> tuple[type[RunLengthEdges], tuple[tuple[tuple[int, int], ...]]]:
395396
return type(self), (self.runs,)
396397

397-
def __len__(self) -> int:
398-
return self.num_edges
399-
400-
def __iter__(self) -> Iterator[int]:
398+
def expand(self) -> Iterator[int]:
399+
"""Every edge, in order: as many items as there are edges, however few the runs."""
401400
return itertools.chain.from_iterable(map(itertools.repeat, self.sizes, self.counts))
402401

403-
def __contains__(self, value: object) -> bool:
404-
return value in self.sizes
405-
406-
@overload
407-
def __getitem__(self, index: int) -> int: ...
408-
@overload
409-
def __getitem__(self, index: slice) -> RunLengthEdges: ...
410-
def __getitem__(self, index: int | slice) -> int | RunLengthEdges:
411-
num_edges = self.num_edges
412-
if isinstance(index, slice):
413-
start, stop, step = index.indices(num_edges)
414-
if step != 1:
415-
return RunLengthEdges.from_edges(self[i] for i in range(start, stop, step))
416-
runs: list[tuple[int, int]] = []
417-
run_start = 0
418-
for size, run_stop in zip(self.sizes, self._index_stops, strict=True):
419-
count = min(stop, run_stop) - max(start, run_start)
420-
if count > 0:
421-
runs.append((size, count))
422-
run_start = run_stop
423-
return RunLengthEdges(runs)
424-
position = index + num_edges if index < 0 else index
425-
if not 0 <= position < num_edges:
426-
raise IndexError(f"Edge index {index} is out of range for {num_edges} edges")
427-
return self.sizes[bisect.bisect_right(self._index_stops, position)]
402+
def size_of(self, index: int) -> int:
403+
"""The length of edge `index`."""
404+
if not 0 <= index < self.num_edges:
405+
raise IndexError(f"Edge index {index} is out of range for {self.num_edges} edges")
406+
return self.sizes[bisect.bisect_right(self._index_stops, index)]
428407

429408
def __eq__(self, other: object) -> bool:
430409
if isinstance(other, RunLengthEdges):
@@ -437,9 +416,6 @@ def __hash__(self) -> int:
437416
def __repr__(self) -> str:
438417
return f"{type(self).__name__}({self.to_rle()})"
439418

440-
def count(self, value: object) -> int:
441-
return sum(count for size, count in self.runs if size == value)
442-
443419
def to_rle(self) -> list[int | list[int]]:
444420
"""The mixed run-length encoding of the rectilinear chunk grid spec: a run of
445421
one edge is its bare size, a longer run is `[size, count]`."""
@@ -569,7 +545,7 @@ def validate_rectilinear_kind(kind: str | None) -> None:
569545

570546

571547
def validate_rectilinear_edges(
572-
chunk_shapes: Sequence[int | Sequence[int]], array_shape: Sequence[int]
548+
chunk_shapes: Sequence[int | Sequence[int] | RunLengthEdges], array_shape: Sequence[int]
573549
) -> None:
574550
"""Validate that rectilinear chunk edges cover the array extent per dimension.
575551

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

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,7 @@ class RectilinearChunkGridMetadataConfig(TypedDict):
245245

246246

247247
def _validate_chunk_shapes(
248-
chunk_shapes: Sequence[int | Sequence[int]],
248+
chunk_shapes: Sequence[int | Sequence[int] | RunLengthEdges],
249249
) -> tuple[int | RunLengthEdges, ...]:
250250
"""Validate per-dimension chunk specifications.
251251
@@ -304,29 +304,30 @@ def from_dict(cls, data: RegularChunkGridMetadataJSON) -> Self: # type: ignore[
304304
return cls(chunk_shape=parse_chunk_shape(configuration["chunk_shape"]))
305305

306306

307-
@dataclass(frozen=True, kw_only=True)
307+
@dataclass(frozen=True, kw_only=True, init=False)
308308
class RectilinearChunkGridMetadata(Metadata):
309309
"""Metadata-only description of a rectilinear chunk grid.
310310
311311
Each element of ``chunk_shapes`` is either:
312312
313313
- A bare ``int`` — a regular step size that repeats to cover the axis
314314
(the spec's single-integer shorthand).
315-
- A sequence of ``int`` — explicit per-chunk edge lengths. It is given
316-
as a ``tuple`` or a ``list`` and held as a `RunLengthEdges`, which
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
319-
the size of its run-length encoded form, not in its number of chunks.
315+
- A `RunLengthEdges` — explicit per-chunk edge lengths. They may be given
316+
as a ``tuple`` or a ``list`` of ``int``, and are held run-length
317+
encoded, one entry per run of equal edges, so a grid costs time and
318+
memory in the size of its run-length encoded form, not in its number
319+
of chunks. A `RunLengthEdges` is not a sequence: use its ``expand``
320+
method for one entry per chunk.
320321
321322
This distinction matters for faithful round-tripping: a bare int
322-
serializes back as a bare int, while a single-element tuple serializes
323+
serializes back as a bare int, while a single explicit edge serializes
323324
as a list.
324325
"""
325326

326-
chunk_shapes: tuple[int | Sequence[int], ...]
327+
chunk_shapes: tuple[int | RunLengthEdges, ...]
327328

328-
def __post_init__(self) -> None:
329-
object.__setattr__(self, "chunk_shapes", _validate_chunk_shapes(self.chunk_shapes))
329+
def __init__(self, *, chunk_shapes: Sequence[int | Sequence[int] | RunLengthEdges]) -> None:
330+
object.__setattr__(self, "chunk_shapes", _validate_chunk_shapes(chunk_shapes))
330331

331332
@property
332333
def ndim(self) -> int:
@@ -341,7 +342,7 @@ def to_dict(self) -> RectilinearChunkGridMetadataJSON: # type: ignore[override]
341342
else:
342343
# A run of one edge is written as its bare size, so edges without
343344
# repeats come out as a plain list.
344-
serialized_dims.append(RunLengthEdges.from_edges(dim_spec).to_rle())
345+
serialized_dims.append(dim_spec.to_rle())
345346
return {
346347
"name": "rectilinear",
347348
"configuration": {
@@ -361,17 +362,16 @@ def update_shape(
361362
Otherwise edges are kept as-is (the spec allows trailing edges
362363
beyond the array extent).
363364
"""
364-
new_chunk_shapes: list[int | Sequence[int]] = []
365+
new_chunk_shapes: list[int | RunLengthEdges] = []
365366
for dim_spec, new_ext in zip(self.chunk_shapes, new_shape, strict=True):
366367
if isinstance(dim_spec, int):
367368
# Bare int covers any extent — no change needed
368369
new_chunk_shapes.append(dim_spec)
369370
else:
370-
edges = RunLengthEdges.from_edges(dim_spec)
371-
if new_ext > edges.total:
372-
new_chunk_shapes.append(edges.with_edge(new_ext - edges.total))
371+
if new_ext > dim_spec.total:
372+
new_chunk_shapes.append(dim_spec.with_edge(new_ext - dim_spec.total))
373373
else:
374-
new_chunk_shapes.append(edges)
374+
new_chunk_shapes.append(dim_spec)
375375
return RectilinearChunkGridMetadata(chunk_shapes=tuple(new_chunk_shapes))
376376

377377
@classmethod
@@ -443,7 +443,7 @@ def create_chunk_grid_metadata(
443443
return RegularChunkGridMetadata(chunk_shape=chunks.chunk_shape)
444444
# Uniform dimensions stay bare ints — the rectilinear grid spec treats
445445
# a bare int as a step size repeating to cover the axis.
446-
chunk_shapes: list[int | Sequence[int]] = []
446+
chunk_shapes: list[int | RunLengthEdges] = []
447447
for dim in chunks.dimensions:
448448
if isinstance(dim, FixedDimension):
449449
chunk_shapes.append(dim.size)

‎src/zarr/testing/strategies.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -553,12 +553,12 @@ def chunks_param_from_rectilinear(
553553
) -> list[int | list[int]]:
554554
"""Convert rectilinear chunk grid metadata into a `chunks=` argument.
555555
556-
Explicit edge sequences become lists. Bare ints — the spec's step-size
556+
Explicit edges become lists. Bare ints — the spec's step-size
557557
shorthand meaning "repeat to cover the axis" — pass through unchanged;
558558
wrapping one in a single-element list would instead declare exactly one
559559
chunk, which fails normalization whenever the axis needs more than one.
560560
"""
561-
return [dim if isinstance(dim, int) else list(dim) for dim in meta.chunk_shapes]
561+
return [dim if isinstance(dim, int) else list(dim.expand()) for dim in meta.chunk_shapes]
562562

563563

564564
# The most chunks a drawn rectilinear grid declares along one axis, and over

‎tests/test_chunk_grids.py‎

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

5052

‎tests/test_metadata/test_repair.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -423,7 +423,7 @@ def _rectilinear_from_dict(chunk_shapes: list[Any]) -> RectilinearChunkGridMetad
423423
def _second_edge(grid: RectilinearChunkGridMetadata) -> int:
424424
edges = grid.chunk_shapes[0]
425425
assert not isinstance(edges, int)
426-
return edges[1]
426+
return edges.size_of(1)
427427

428428

429429
CHUNK_EDGE_SITES: dict[str, Callable[[Any], object]] = {

‎tests/test_properties.py‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
import json
33
import numbers
44
import warnings
5-
from collections.abc import Generator
5+
from collections.abc import Generator, Sequence
66
from typing import Any
77

88
import numpy as np
@@ -306,9 +306,11 @@ def test_block_indexing(data: st.DataObject) -> None:
306306
# The block grid is worked out from the stored declaration, not by zarr's grid code.
307307
assert isinstance(zarray.metadata, ArrayV3Metadata)
308308
grid = zarray.metadata.chunk_grid
309-
declared = (
310-
grid.chunk_shapes if isinstance(grid, RectilinearChunkGridMetadata) else grid.chunk_shape
311-
)
309+
declared: Sequence[int | tuple[int, ...]]
310+
if isinstance(grid, RectilinearChunkGridMetadata):
311+
declared = [d if isinstance(d, int) else tuple(d.expand()) for d in grid.chunk_shapes]
312+
else:
313+
declared = grid.chunk_shape
312314
chunk_sizes = tuple(
313315
declared_chunk_data_sizes(d, n) for d, n in zip(declared, zarray.shape, strict=True)
314316
)

0 commit comments

Comments
 (0)