refactor(chunk-grids): remove unused members of the dimension grids

Nothing in the library called `with_extent`, `ngridcells` or
`_unique_edge_lengths` on `FixedDimension` / `VaryingDimension`, did an
`isinstance` check against `DimensionGrid`, or used `is_regular_1d` /
`is_regular_nd`. Remove them along with the tests that only exercised
them.

`VaryingDimension.chunk_offset` no longer maps a negative chunk index
to 0, a guard left over from indexing a tuple of prefix sums. Both
dimension types raise the same `IndexError` message from
`index_to_chunk`, and docstrings that described the old tuple-backed
behaviour are corrected.

Assisted-by: ClaudeCode:claude-fable-5.1

Author: d-v-b

changes/4479.bugfix.md

@@ -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 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 removed; `VaryingDimension.chunk_offset` gives the start of a chunk. The repr of a chunk grid shows a dimension of more than 100 chunks in its run-length encoded form.
+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 removed; `VaryingDimension.chunk_offset` gives the start of a chunk. The repr of a chunk grid shows a dimension of more than 100 chunks in its run-length encoded form. The unused `with_extent` and `ngridcells` members of `FixedDimension` and `VaryingDimension`, and the unused helpers `zarr.core.chunk_grids.is_regular_1d` and `is_regular_nd`, are removed; both dimension types now raise the same `IndexError` message from `index_to_chunk`.

src/zarr/core/chunk_grids.py

@@ -14,7 +14,6 @@
     Protocol,
     SupportsIndex,
     cast,
-    runtime_checkable,
 )
 
 import numpy as np
@@ -53,26 +52,21 @@ class FixedDimension:
     size: int  # chunk edge length (>= 1)
     extent: int  # array dimension length (>= 0)
     nchunks: int = field(init=False, repr=False)
-    ngridcells: int = field(init=False, repr=False)
 
     def __post_init__(self) -> None:
         if self.size < 1:
             raise ValueError(f"FixedDimension size must be >= 1, got {self.size}")
         if self.extent < 0:
             raise ValueError(f"FixedDimension extent must be >= 0, got {self.extent}")
-        n = ceildiv_int(self.extent, self.size)
-        object.__setattr__(self, "nchunks", n)
-        object.__setattr__(self, "ngridcells", n)
+        object.__setattr__(self, "nchunks", ceildiv_int(self.extent, self.size))
 
     def index_to_chunk(self, idx: int) -> int:
-        if idx < 0:
-            raise IndexError(f"Negative index {idx} is not allowed")
-        if idx >= self.extent:
-            raise IndexError(f"Index {idx} is out of bounds for extent {self.extent}")
+        if idx < 0 or idx >= self.extent:
+            raise IndexError(f"Index {idx} out of bounds for dimension with extent {self.extent}")
         return idx // self.size
 
     def chunk_offset(self, chunk_ix: int) -> int:
-        """Byte-aligned start position of chunk *chunk_ix* in array coordinates.
+        """Start position of chunk *chunk_ix* in array coordinates.
 
         Does not validate *chunk_ix* — callers must ensure it is in
         ``[0, nchunks)``. Use ``ChunkGrid.__getitem__`` for safe access.
@@ -95,34 +89,12 @@ def data_size(self, chunk_ix: int) -> int:
         """
         return max(0, min(self.size, self.extent - chunk_ix * self.size))
 
-    @property
-    def _unique_edge_lengths(self) -> Iterable[int]:
-        """Distinct chunk edge lengths for this dimension.
-
-        Used by shard validation to check that every unique edge length
-        is divisible by the inner chunk size. O(1) for fixed dimensions
-        since there is only one edge length.
-        """
-        return (self.size,)
-
     def indices_to_chunks(self, indices: npt.NDArray[np.intp]) -> npt.NDArray[np.intp]:
         return indices // self.size
 
-    def with_extent(self, new_extent: int) -> FixedDimension:
-        """Re-bind to *new_extent* without modifying edges.
-
-        Used when constructing a grid from existing metadata where edges
-        are already correct. Raises on
-        ``VaryingDimension`` if edges don't cover the new extent.
-        """
-        return FixedDimension(size=self.size, extent=new_extent)
-
     def resize(self, new_extent: int) -> FixedDimension:
-        """Adapt for a user-initiated array resize, growing edges if needed.
-
-        For ``FixedDimension`` this is identical to ``with_extent`` since
-        regular grids don't store explicit edges.
-        """
+        """Adapt for a user-initiated array resize: the chunk size is kept and
+        re-bound to *new_extent*."""
         return FixedDimension(size=self.size, extent=new_extent)
 
     @property
@@ -159,7 +131,6 @@ class VaryingDimension:
     edges: RunLengthEdges  # per-chunk edge lengths (all > 0), run-length encoded
     extent: int  # array dimension length (may be < sum(edges) after resize)
     nchunks: int = field(init=False, repr=False)  # cached at construction
-    ngridcells: int = field(init=False, repr=False)  # cached at construction
 
     def __init__(self, edges: Sequence[int] | RunLengthEdges, extent: int) -> None:
         rle = RunLengthEdges.from_edges(edges)
@@ -176,34 +147,33 @@ def __init__(self, edges: Sequence[int] | RunLengthEdges, extent: int) -> None:
         # Cache nchunks: number of chunks that overlap [0, extent)
         n = 0 if extent == 0 else rle.index_at(extent - 1) + 1
         object.__setattr__(self, "nchunks", n)
-        object.__setattr__(self, "ngridcells", rle.num_edges)
 
     def index_to_chunk(self, idx: int) -> int:
         if idx < 0 or idx >= self.extent:
             raise IndexError(f"Index {idx} out of bounds for dimension with extent {self.extent}")
         return self.edges.index_at(idx)
 
     def chunk_offset(self, chunk_ix: int) -> int:
-        """Start position of chunk *chunk_ix* in array coordinates.
+        """Start position of chunk *chunk_ix* in array coordinates; for
+        ``chunk_ix == edges.num_edges``, the sum of all edges.
 
-        Does not validate *chunk_ix* — callers must ensure it is in
-        ``[0, ngridcells)``. Use ``ChunkGrid.__getitem__`` for safe access.
+        Raises ``IndexError`` for any other *chunk_ix* outside
+        ``[0, edges.num_edges)``.
         """
-        return self.edges.offset_of(chunk_ix) if chunk_ix > 0 else 0
+        return self.edges.offset_of(chunk_ix)
 
     def chunk_size(self, chunk_ix: int) -> int:
         """Buffer size for codec processing.
 
-        Does not validate *chunk_ix* — callers must ensure it is in
-        ``[0, ngridcells)``. Use ``ChunkGrid.__getitem__`` for safe access.
+        Raises ``IndexError`` for *chunk_ix* outside ``[0, edges.num_edges)``.
         """
         return self.edges.size_of(chunk_ix)
 
     def data_size(self, chunk_ix: int) -> int:
-        """Valid data region within the buffer — clipped at extent.
+        """Valid data region within the buffer — clipped at extent; 0 for an
+        edge that lies entirely past the extent.
 
-        Does not validate *chunk_ix* — callers must ensure it is in
-        ``[0, ngridcells)``. Use ``ChunkGrid.__getitem__`` for safe access.
+        Raises ``IndexError`` for *chunk_ix* outside ``[0, edges.num_edges)``.
         """
         return max(0, min(self.edges.size_of(chunk_ix), self.extent - self.chunk_offset(chunk_ix)))
 
@@ -215,38 +185,14 @@ def _data_sizes(self) -> tuple[int, ...]:
         last = self.nchunks - 1
         return (*itertools.islice(self.edges.expand(), last), self.data_size(last))
 
-    @property
-    def _unique_edge_lengths(self) -> Iterable[int]:
-        """Distinct chunk edge lengths for this dimension, in order of first use.
-
-        Used by shard validation to check that every unique edge length
-        is divisible by the inner chunk size. Reads one size per run of
-        equal edges, not one per chunk.
-        """
-        return tuple(dict.fromkeys(self.edges.sizes))
-
     def indices_to_chunks(self, indices: npt.NDArray[np.intp]) -> npt.NDArray[np.intp]:
         return self.edges.indices_at(indices)
 
-    def with_extent(self, new_extent: int) -> VaryingDimension:
-        """Re-bind to *new_extent* without modifying edges.
-
-        Used when constructing a grid from existing metadata where edges
-        are already correct. Raises if the
-        existing edges don't cover *new_extent*.
-        """
-        edge_sum = self.edges.total
-        if edge_sum < new_extent:
-            raise ValueError(
-                f"VaryingDimension edge sum {edge_sum} is less than new extent {new_extent}"
-            )
-        return VaryingDimension(self.edges, extent=new_extent)
-
     def resize(self, new_extent: int) -> VaryingDimension:
         """Adapt for a user-initiated array resize, growing edges if needed.
 
-        Unlike ``with_extent``, this never fails — if *new_extent* exceeds
-        the current edge sum, a new chunk is appended to cover the gap.
+        If *new_extent* exceeds the current edge sum, a new chunk is
+        appended to cover the gap.
         Shrinking preserves all edges (the spec allows trailing edges
         beyond the array extent).
         """
@@ -263,24 +209,18 @@ def _size_repr(self) -> str:
         return _edges_repr(self.edges)
 
 
-@runtime_checkable
 class DimensionGrid(Protocol):
     """Structural interface shared by FixedDimension and VaryingDimension."""
 
     @property
     def nchunks(self) -> int: ...
     @property
-    def ngridcells(self) -> int: ...
-    @property
     def extent(self) -> int: ...
     def index_to_chunk(self, idx: int) -> int: ...
     def chunk_offset(self, chunk_ix: int) -> int: ...
     def chunk_size(self, chunk_ix: int) -> int: ...
     def data_size(self, chunk_ix: int) -> int: ...
     def indices_to_chunks(self, indices: npt.NDArray[np.intp]) -> npt.NDArray[np.intp]: ...
-    @property
-    def _unique_edge_lengths(self) -> Iterable[int]: ...
-    def with_extent(self, new_extent: int) -> DimensionGrid: ...
     def resize(self, new_extent: int) -> DimensionGrid: ...
     @property
     def _size_repr(self) -> str: ...
@@ -365,28 +305,6 @@ def _chunk_list(chunks: object) -> list[Any]:
         ) from None
 
 
-def is_regular_1d(dim_chunks: Sequence[int]) -> bool:
-    """Check if a single dimension's chunk sizes represent a regular grid.
-
-    A regular dimension has either all chunks the same size, or all
-    but the last chunk the same size with the last chunk smaller
-    (boundary chunk).
-    """
-    if len(dim_chunks) <= 1:
-        return True
-    first = dim_chunks[0]
-    for c in dim_chunks[1:-1]:
-        if c != first:
-            return False
-    # Last chunk must be the same size or a smaller boundary chunk
-    return dim_chunks[-1] <= first
-
-
-def is_regular_nd(chunks: Iterable[Sequence[int]]) -> bool:
-    """Check if an N-dimensional chunk specification represents a regular grid."""
-    return all(is_regular_1d(d) for d in chunks)
-
-
 @dataclass(frozen=True)
 class ChunkGrid:
     """

tests/test_metadata/test_v3.py

@@ -11,7 +11,7 @@
 from tests.conftest import Expect, ExpectFail
 from tests.test_metadata.conftest import minimal_metadata_dict_v3
 from zarr.core.buffer import default_buffer_prototype
-from zarr.core.chunk_grids import ChunkGrid, is_regular_1d, is_regular_nd
+from zarr.core.chunk_grids import ChunkGrid
 from zarr.core.config import config
 from zarr.core.dtype import Float64, UInt8
 from zarr.core.group import GroupMetadata, parse_node_type
@@ -108,47 +108,6 @@ def test_parse_codecs_unknown_raises(monkeypatch: pytest.MonkeyPatch) -> None:
         parse_codecs([{"name": "unknown"}])
 
 
-# ---------------------------------------------------------------------------
-# Chunk-grid regularity helpers
-# ---------------------------------------------------------------------------
-
-_REGULAR_1D_CASES: list[Expect[list[int], bool]] = [
-    Expect(input=[], output=True, id="empty"),
-    Expect(input=[10], output=True, id="single-chunk"),
-    Expect(input=[10, 10, 10], output=True, id="all-equal"),
-    Expect(input=[10, 10, 10, 7], output=True, id="smaller-boundary"),
-    Expect(input=[10, 10, 10, 10], output=True, id="exact-multiple"),
-    Expect(input=[10, 5, 10], output=False, id="middle-mismatch"),
-    Expect(input=[10, 10, 10, 12], output=False, id="last-larger"),
-    # The first chunk anchors the size; later mismatches in the middle fail
-    # before the boundary check.
-    Expect(input=[5, 10, 5], output=False, id="middle-larger"),
-]
-
-
-@pytest.mark.parametrize("case", _REGULAR_1D_CASES, ids=lambda c: c.id)
-def test_is_regular_1d_sequence(case: Expect[list[int], bool]) -> None:
-    """`is_regular_1d` accepts plain Python sequences."""
-    assert is_regular_1d(case.input) is case.output
-    assert is_regular_1d(tuple(case.input)) is case.output
-
-
-@pytest.mark.parametrize(
-    "case",
-    [
-        Expect(input=[[10, 10, 10], [5, 5]], output=True, id="all-regular"),
-        Expect(input=[[10, 10, 7], [5, 5, 5, 3]], output=True, id="all-regular-with-boundary"),
-        Expect(input=[[10, 10, 10], [5, 8, 5]], output=False, id="second-dim-irregular"),
-        Expect(input=[[10, 5, 10], [5, 5]], output=False, id="first-dim-irregular"),
-        Expect(input=[], output=True, id="zero-dims"),
-    ],
-    ids=lambda c: c.id,
-)
-def test_is_regular_nd_sequence(case: Expect[list[list[int]], bool]) -> None:
-    """`is_regular_nd` returns True iff every per-dim spec is regular."""
-    assert is_regular_nd(case.input) is case.output
-
-
 def test_create_chunk_grid_metadata_unknown_dimension_type() -> None:
     """`create_chunk_grid_metadata` rejects dimension grids it does not recognize."""
     grid = ChunkGrid(dimensions=(object(),))  # type: ignore[arg-type]