Skip to content

Commit 259525d

Browse files
authored
Merge branch 'main' into claude/structured-dtype-bugfixes
2 parents d4a4b01 + 0de6077 commit 259525d

13 files changed

Lines changed: 286 additions & 109 deletions

File tree

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Validate inclusive `index_array_bounds` against all raw index values when loading transforms and output maps from JSON. Accept valid finite and one-sided constraints, and reject out-of-bounds values eagerly before offset, stride, or map simplification. Validated immutable maps need not retain the constraints; message normalization preserves the original bounds.
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Delegate LazyArray source tokenization to Dask, honoring its registered normalizers, source hooks, and deterministic-token requirements. Remove local content-hashing and UUID fallbacks. Dask remains optional for indexing and reading.

packages/zarr-indexing/docs/guide/integrations.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,3 +243,14 @@ that implementation or reproduce its full worker/GPU lifecycle.
243243
·
244244
**API:** [API reference](../api/index.md)
245245
</nav>
246+
247+
## Dask tokenization
248+
249+
`LazyArray.__dask_tokenize__()` combines Dask's token for the wrapped source
250+
with the serialized view transform. Dask owns source hashing, registered
251+
normalizers, custom source hooks, and deterministic-token requirements.
252+
Tokenization may read or hash source values. Dask is optional for indexing
253+
and reading, but required when requesting a Dask token.
254+
255+
The reader and partitioning are omitted because they must preserve values.
256+
Changing a source after graph construction does not update existing Dask keys.

packages/zarr-indexing/docs/ndsel.md

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,22 @@ assert t.to_json() == canonical
9999
map kind has a `to_json`; `output_index_map_from_json` dispatches the wire's
100100
structurally discriminated union back to the right kind. Exact JSON equality
101101
in this example is not a general round-trip guarantee: implicit flags are
102-
removed, finite `index_array_bounds` are not retained or enforced by the
103-
engine, and degenerate array maps are collapsed.
102+
removed and degenerate array maps are collapsed.
103+
104+
`index_array_bounds` constrains raw index-array values before the map's offset
105+
and stride are applied. Both `IndexTransform.from_json` and
106+
`output_index_map_from_json` validate every supplied value against the inclusive
107+
bounds when loading. Finite and one-sided bounds are supported; omitted bounds
108+
and `["-inf", "+inf"]` impose no additional constraint. Values outside the
109+
bounds raise `NdselError("invalid_json", ...)`, including in singleton arrays
110+
and zero-stride maps. Empty arrays satisfy any well-formed, ordered bounds.
111+
112+
Validation is eager: an invalid entry rejects the entire map even if a later
113+
selection would avoid that entry. After validation the engine owns immutable
114+
index coordinates, so it need not retain the bounds; serialization emits
115+
unbounded constraints for non-degenerate maps. Message normalization preserves
116+
the original bounds without checking array contents. This implementation does
117+
not defer bounds errors until individual positions are accessed.
104118

105119
A canonical body carrying a
106120
`"-inf"` or `"+inf"` bound cannot be lowered — an `IndexDomain` addresses a

packages/zarr-indexing/pyproject.toml

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -99,9 +99,8 @@ extend = "../../pyproject.toml"
9999
target-version = "py312"
100100

101101
[tool.ruff.lint.per-file-ignores]
102-
# Chunk discovery and __dask_tokenize__ deliberately catch Exception: a
103-
# foreign source's attributes or token hooks may fail, so these paths provide
104-
# fallback metadata or tokens when ordinary exceptions occur. Configured here (not as
102+
# Chunk discovery deliberately catches Exception: a foreign source's
103+
# attributes may fail, so this path provides fallback metadata. Configured here (not as
105104
# noqa comments) because different ruff versions
106105
# have differed on whether these rules fire; RUF100 can remove unused noqa comments.
107106
"src/zarr_indexing/lazy_array.py" = ["BLE001", "S110"]

packages/zarr-indexing/src/zarr_indexing/_wire.py

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@
22
33
Package-private: the types that serialize themselves (`IndexDomain`,
44
`IndexTransform`, the output map kinds) all need these, so they cannot live in
5-
any one of them, and they are not API. The three engine constraints named in
6-
[`zarr_indexing.json`][zarr_indexing.json] — finite bounds, implicit bounds
7-
lowering by value, integer `index_array` content — are enforced here.
5+
any one of them, and they are not API. Domain bounds, index-array bounds,
6+
implicit bounds lowering by value, and integer `index_array` content are
7+
handled here, as described in [`zarr_indexing.json`][zarr_indexing.json].
88
"""
99

1010
from __future__ import annotations
@@ -13,13 +13,37 @@
1313

1414
import numpy as np
1515

16-
from zarr_indexing.messages import NdselError
16+
from zarr_indexing.messages import NdselError, validate_index_array_bounds
1717

1818
if TYPE_CHECKING:
1919
from zarr_indexing.domain import IndexDomain
2020
from zarr_indexing.json import BoundJSON
2121

2222

23+
def check_index_array_bounds(array: np.ndarray[Any, Any], bounds: Any, where: str) -> None:
24+
"""Validate every raw index value against an inclusive interval.
25+
26+
Validate interval syntax even for empty arrays. Once checked, immutable
27+
index coordinates need no retained constraint. Use Python integer extrema
28+
to avoid overflow or floating-point rounding at integer limits.
29+
"""
30+
lo, hi = validate_index_array_bounds(bounds, where)
31+
if array.size == 0 or (lo == "-inf" and hi == "+inf"):
32+
return
33+
minimum, maximum = int(array.min()), int(array.max())
34+
if (
35+
lo == "+inf"
36+
or hi == "-inf"
37+
or (isinstance(lo, int) and minimum < lo)
38+
or (isinstance(hi, int) and maximum > hi)
39+
):
40+
raise NdselError(
41+
"invalid_json",
42+
f"{where}.index_array values [{minimum}, {maximum}] are outside "
43+
f"index_array_bounds {bounds!r}",
44+
)
45+
46+
2347
def lower_bound(bound: BoundJSON, where: str) -> int:
2448
"""Lower a canonical bound to a finite integer, rejecting infinities.
2549

packages/zarr-indexing/src/zarr_indexing/json.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@
1414
1515
The engine lowering rules include:
1616
17+
- **Validated index arrays.** Raw index values must satisfy the inclusive
18+
`index_array_bounds` before offset and stride are applied. Lowering checks
19+
all supplied values eagerly; validated immutable maps need not retain bounds.
1720
- **Finite bounds.** An `IndexDomain` addresses a finite array, so a canonical
1821
body carrying a `"-inf"`/`"+inf"` bound cannot be lowered; `from_json` raises.
1922
- **Implicit bounds lower by value.** The `[n]`-bracket implicit/explicit flag
@@ -132,8 +135,9 @@ class OutputIndexMapJSON(TypedDict, total=False):
132135
index_array_bounds: list[IndexValueJSON]
133136
"""Wire bounds on index-array values; `["-inf", "+inf"]` if unconstrained.
134137
135-
The engine currently discards this field on load and does not enforce
136-
finite bounds against the array values. Serialization emits unconstrained bounds.
138+
The message layer preserves these inclusive constraints on raw index values.
139+
Engine lowering validates all values eagerly before offset and stride.
140+
Serialization emits unconstrained bounds for validated non-degenerate maps.
137141
"""
138142

139143

packages/zarr-indexing/src/zarr_indexing/lazy_array.py

Lines changed: 10 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@
1212
```
1313
1414
Selection construction does not read source values: `result()`, `__array__`,
15-
and eager `__getitem__` perform reads. Tokenization can also read or hash source
16-
data, depending on the source and tokenization path. `.lazy` operations inspect
15+
and eager `__getitem__` perform reads. Source tokenization is delegated to
16+
Dask and may inspect source values. `.lazy` operations inspect
1717
selection metadata and may copy or process supplied index arrays. Composition
1818
does not accumulate wrapper layers: a view of a view is still a single transform
1919
and retains its reader.
@@ -138,7 +138,6 @@
138138
import json
139139
import math
140140
import operator
141-
import uuid
142141
from collections.abc import Sequence
143142
from dataclasses import dataclass, field
144143
from typing import TYPE_CHECKING, Any, Protocol, cast
@@ -173,10 +172,6 @@
173172

174173
__all__ = ["LazyArray", "Partition"]
175174

176-
# Above this declared byte count, the no-Dask token fallback adds a fresh UUID
177-
# instead of digesting contents. See `_wrapped_token`.
178-
_TOKEN_DIGEST_LIMIT = 1 << 20
179-
180175

181176
def _invoke_reader(
182177
reader: Reader,
@@ -525,66 +520,6 @@ def _validate_prepared_parts(parts: Sequence[Partition], out_shape: tuple[int, .
525520
raise ValueError("prepared parts do not tile the view exactly")
526521

527522

528-
# --------------------------------------------------------------------------- #
529-
# Tokenization
530-
# --------------------------------------------------------------------------- #
531-
532-
533-
def _wrapped_token(array: Any) -> Any:
534-
"""A token for the wrapped array.
535-
536-
In order of preference: the array's own `__dask_tokenize__`;
537-
`dask.base.tokenize` when dask is importable (imported lazily — this package
538-
never requires it); otherwise a local fallback that digests the contents of
539-
a small array.
540-
541-
Tokens can differ depending on whether Dask is available and on the source
542-
hook. This fallback does not provide a portable content identifier.
543-
544-
Above `_TOKEN_DIGEST_LIMIT`, or when conversion is unavailable, the local
545-
fallback adds a fresh UUID on each call, so repeated calls normally differ.
546-
The returned tuple is still equal to itself. Below the limit, conversion
547-
can read the source, and the hash is of its NumPy buffer bytes. Object-array
548-
buffer bytes contain object references, not a recursive content snapshot.
549-
"""
550-
hook = getattr(array, "__dask_tokenize__", None)
551-
if hook is not None:
552-
try:
553-
return hook()
554-
# A failing source hook falls through to the remaining tokenization paths.
555-
except Exception: # pragma: no cover - a hook that refuses to run
556-
pass
557-
try:
558-
# dask is an optional peer, never a dependency of this package, so it is
559-
# imported here and its absence is ordinary.
560-
from dask.base import tokenize # pyright: ignore[reportMissingImports]
561-
except ImportError:
562-
pass
563-
else:
564-
return tokenize(array)
565-
566-
shape = tuple(int(s) for s in getattr(array, "shape", ()))
567-
dtype = getattr(array, "dtype", None)
568-
structural = (type(array).__qualname__, shape, str(dtype))
569-
# A fresh identifier per call when contents cannot be identified. It
570-
# is the shape and dtype that would otherwise be mistaken for an identity,
571-
# so they are kept alongside it for a reader looking at a graph.
572-
unidentified = (*structural, "unidentified", uuid.uuid4().hex)
573-
574-
# Decide whether to digest the contents from the *declared* size. Measuring
575-
# it by converting first would read the whole array — a multi-gigabyte store
576-
# pulled into memory by a token call, which is the opposite of the point.
577-
itemsize = getattr(dtype, "itemsize", None)
578-
if not isinstance(itemsize, int) or itemsize * math.prod(shape) > _TOKEN_DIGEST_LIMIT:
579-
return unidentified
580-
try:
581-
contents = np.ascontiguousarray(array)
582-
# Failed NumPy conversion leaves this source unidentified.
583-
except Exception:
584-
return unidentified
585-
return (*structural, hashlib.sha256(contents.tobytes()).hexdigest())
586-
587-
588523
# --------------------------------------------------------------------------- #
589524
# The wrapper
590525
# --------------------------------------------------------------------------- #
@@ -1249,15 +1184,18 @@ def __dask_tokenize__(self) -> Any:
12491184
tokens produce equal tokens; arbitrary semantically equivalent mappings
12501185
are not guaranteed to serialize identically.
12511186
1252-
Source tokenization can read or hash data and need not be deterministic
1253-
on every fallback path; see `_wrapped_token`. The reader and partitioning
1254-
are omitted under the contract that they preserve values. Cache users
1255-
must also account for source mutation and the source's token semantics.
1187+
Dask tokenizes the wrapped source using its normal dispatch and
1188+
determinism policy. This may read or hash source values. Dask is
1189+
imported only when this method is called and is otherwise optional.
1190+
The reader and partitioning are omitted because they must preserve
1191+
values. Mutating a source does not update keys in existing Dask graphs.
12561192
"""
1193+
from dask.base import tokenize # pyright: ignore[reportMissingImports]
1194+
12571195
canonical = json.dumps(self._transform.to_json(), sort_keys=True)
12581196
return (
12591197
type(self).__qualname__,
1260-
_wrapped_token(self._array),
1198+
tokenize(self._array),
12611199
hashlib.sha256(canonical.encode()).hexdigest(),
12621200
)
12631201

packages/zarr-indexing/src/zarr_indexing/messages.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -556,7 +556,7 @@ def _normalize_output_map(raw: Any, where: str) -> dict[str, Any]:
556556
if has_index_array:
557557
stride = _check_int(raw["stride"], f"{where}.stride") if "stride" in raw else 1
558558
bounds = (
559-
_check_index_array_bounds(raw["index_array_bounds"], where)
559+
validate_index_array_bounds(raw["index_array_bounds"], where)
560560
if "index_array_bounds" in raw
561561
else ["-inf", "+inf"]
562562
)
@@ -585,7 +585,8 @@ def _normalize_output_map(raw: Any, where: str) -> dict[str, Any]:
585585
return {"offset": offset}
586586

587587

588-
def _check_index_array_bounds(value: Any, where: str) -> list[int | str]:
588+
def validate_index_array_bounds(value: Any, where: str) -> list[int | str]:
589+
"""Validate the syntax and ordering of an inclusive index-array interval."""
589590
if not isinstance(value, list) or len(value) != 2:
590591
raise NdselError(
591592
"invalid_json",

packages/zarr-indexing/src/zarr_indexing/output_map.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -357,18 +357,24 @@ def output_index_map_from_json(data: OutputIndexMapJSON) -> OutputIndexMap:
357357
selects an array map, `input_dimension` selects a dimension map, and
358358
neither selects a constant map.
359359
360+
Validate every raw index value against the inclusive `index_array_bounds`
361+
before applying offset and stride. Out-of-bounds values raise `NdselError`
362+
at load time. Validated immutable maps do not retain the bounds.
363+
360364
Examples
361365
--------
362366
>>> output_index_map_from_json({"offset": 5})
363367
ConstantMap(offset=5)
364368
>>> output_index_map_from_json({"offset": 0, "stride": 2, "input_dimension": 1})
365369
DimensionMap(input_dimension=1, offset=0, stride=2)
366370
"""
367-
from zarr_indexing._wire import lower_index_array
371+
from zarr_indexing._wire import check_index_array_bounds, lower_index_array
368372

369373
if "index_array" in data:
374+
array = lower_index_array(data["index_array"], "index_array")
375+
check_index_array_bounds(array, data.get("index_array_bounds", ["-inf", "+inf"]), "output")
370376
return ArrayMap(
371-
index_array=lower_index_array(data["index_array"], "index_array"),
377+
index_array=array,
372378
offset=data.get("offset", 0),
373379
stride=data.get("stride", 1),
374380
)

0 commit comments

Comments
 (0)