Skip to content

Commit 2ac5f75

Browse files
d-v-bd-v-b
committed
test: address review — type mode params and indexers return; consistent mode collections
- Type the `mode` parameter as the `IndexMode` literal (not `str`) across the test helpers (`_get`, `_async_get`, `_setitem`, `_eligible`, `assert_read_matches_numpy`). - Make `_VECTORIZED_MODES` a tuple like `_INDEX_MODES` (was a `frozenset`) for consistency; `_INDEX_MODES` must stay an ordered sequence for `sampled_from`. - `indexers(...)` now returns `tuple[Selection, Selection]` (the real `zarr.core.indexing.Selection` type) instead of `tuple[Any, Any]`. Assisted-by: ClaudeCode:claude-opus-4.8
1 parent efe3056 commit 2ac5f75

3 files changed

Lines changed: 114 additions & 8 deletions

File tree

‎.claude/hooks/.logs/hook-log.jsonl‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
{"ts":"2026-06-30T08:02:48.679Z","hook":"stop-notify","projectName":"hopeful-euclid-d75809","duration":"2m 17s","iterations":0}
2+
{"ts":"2026-06-30T08:07:46.769Z","hook":"session-init","projectRoot":"/Users/d-v-b/dev/zarr-python/.claude/worktrees/hopeful-euclid-d75809","gitBranch":"review-perf-3906"}
3+
{"ts":"2026-06-30T08:07:47.141Z","hook":"iteration-context","action":"skip","iterationCount":1}
4+
{"ts":"2026-06-30T08:07:47.141Z","hook":"dev-rules-reminder","action":"inject","iterationCount":0}
5+
{"ts":"2026-06-30T08:15:43.914Z","hook":"iteration-context","action":"skip","iterationCount":7}
6+
{"ts":"2026-06-30T08:16:03.013Z","hook":"session-init","projectRoot":"/Users/d-v-b/dev/zarr-python/.claude/worktrees/hopeful-euclid-d75809","gitBranch":"review-perf-3906"}
7+
{"ts":"2026-06-30T08:16:03.435Z","hook":"iteration-context","action":"skip","iterationCount":1}
8+
{"ts":"2026-06-30T08:16:03.434Z","hook":"dev-rules-reminder","action":"inject","iterationCount":0}
9+
{"ts":"2026-06-30T08:17:39.013Z","hook":"stop-notify","projectName":"hopeful-euclid-d75809","duration":"9m 52s","iterations":0}
10+
{"ts":"2026-06-30T08:20:19.696Z","hook":"stop-notify","projectName":"hopeful-euclid-d75809","duration":"4m 16s","iterations":0}
11+
{"ts":"2026-06-30T08:20:23.991Z","hook":"iteration-context","action":"skip","iterationCount":8}
12+
{"ts":"2026-06-30T08:23:28.853Z","hook":"iteration-context","action":"skip","iterationCount":9}
13+
{"ts":"2026-06-30T08:25:40.928Z","hook":"iteration-context","action":"skip","iterationCount":10,"reason":"no-tsv"}
14+
{"ts":"2026-06-30T08:39:37.207Z","hook":"session-init","projectRoot":"/Users/d-v-b/dev/zarr-python/.claude/worktrees/hopeful-euclid-d75809","gitBranch":"review-perf-3906"}
15+
{"ts":"2026-06-30T08:39:37.551Z","hook":"iteration-context","action":"skip","iterationCount":1}
16+
{"ts":"2026-06-30T08:39:37.551Z","hook":"dev-rules-reminder","action":"inject","iterationCount":0}
17+
{"ts":"2026-06-30T08:40:05.715Z","hook":"session-init","projectRoot":"/Users/d-v-b/dev/zarr-python/.claude/worktrees/hopeful-euclid-d75809","gitBranch":"review-perf-3906"}
18+
{"ts":"2026-06-30T08:40:06.019Z","hook":"iteration-context","action":"skip","iterationCount":1}
19+
{"ts":"2026-06-30T08:40:06.019Z","hook":"dev-rules-reminder","action":"inject","iterationCount":0}
20+
{"ts":"2026-06-30T08:40:26.532Z","hook":"dev-rules-reminder","action":"inject","iterationCount":10}
21+
{"ts":"2026-06-30T08:40:26.534Z","hook":"iteration-context","action":"skip","iterationCount":11}
22+
{"ts":"2026-06-30T08:41:43.557Z","hook":"stop-notify","projectName":"hopeful-euclid-d75809","duration":"2m 6s","iterations":0}
23+
{"ts":"2026-06-30T08:52:58.718Z","hook":"session-init","projectRoot":"/Users/d-v-b/dev/zarr-python/.claude/worktrees/hopeful-euclid-d75809","gitBranch":"review-perf-3906"}
24+
{"ts":"2026-06-30T08:52:58.718Z","hook":"session-init","projectRoot":"/Users/d-v-b/dev/zarr-python/.claude/worktrees/hopeful-euclid-d75809","gitBranch":"review-perf-3906"}
25+
{"ts":"2026-06-30T08:52:59.083Z","hook":"iteration-context","action":"skip","iterationCount":1}
26+
{"ts":"2026-06-30T08:52:59.083Z","hook":"dev-rules-reminder","action":"inject","iterationCount":0}
27+
{"ts":"2026-06-30T08:52:59.145Z","hook":"iteration-context","action":"skip","iterationCount":1}
28+
{"ts":"2026-06-30T08:52:59.145Z","hook":"dev-rules-reminder","action":"inject","iterationCount":0}
29+
{"ts":"2026-06-30T08:53:43.711Z","hook":"stop-notify","projectName":"hopeful-euclid-d75809","duration":"0m 45s","iterations":0}
30+
{"ts":"2026-06-30T08:54:00.992Z","hook":"stop-notify","projectName":"hopeful-euclid-d75809","duration":"1m 2s","iterations":0}
31+
{"ts":"2026-06-30T08:55:15.551Z","hook":"iteration-context","action":"skip","iterationCount":12}
32+
{"ts":"2026-06-30T09:00:50.327Z","hook":"session-init","projectRoot":"/Users/d-v-b/dev/zarr-python/.claude/worktrees/hopeful-euclid-d75809","gitBranch":"review-perf-3906"}
33+
{"ts":"2026-06-30T09:00:50.784Z","hook":"iteration-context","action":"skip","iterationCount":1}
34+
{"ts":"2026-06-30T09:00:50.783Z","hook":"dev-rules-reminder","action":"inject","iterationCount":0}
35+
{"ts":"2026-06-30T09:00:55.351Z","hook":"stop-notify","projectName":"hopeful-euclid-d75809","duration":"0m 5s","iterations":0}
36+
{"ts":"2026-06-30T09:02:52.047Z","hook":"iteration-context","action":"skip","iterationCount":13}
37+
{"ts":"2026-06-30T09:13:17.174Z","hook":"session-init","projectRoot":"/Users/d-v-b/dev/zarr-python/.claude/worktrees/hopeful-euclid-d75809","gitBranch":"review-perf-3906"}
38+
{"ts":"2026-06-30T09:13:17.566Z","hook":"dev-rules-reminder","action":"inject","iterationCount":0}
39+
{"ts":"2026-06-30T09:13:17.569Z","hook":"iteration-context","action":"skip","iterationCount":1}
40+
{"ts":"2026-06-30T09:13:27.833Z","hook":"session-init","projectRoot":"/Users/d-v-b/dev/zarr-python/.claude/worktrees/hopeful-euclid-d75809","gitBranch":"review-perf-3906"}
41+
{"ts":"2026-06-30T09:13:28.168Z","hook":"iteration-context","action":"skip","iterationCount":1}
42+
{"ts":"2026-06-30T09:13:28.167Z","hook":"dev-rules-reminder","action":"inject","iterationCount":0}
43+
{"ts":"2026-06-30T09:17:00.160Z","hook":"session-init","projectRoot":"/Users/d-v-b/dev/zarr-python/.claude/worktrees/hopeful-euclid-d75809","gitBranch":"review-perf-3906"}
44+
{"ts":"2026-06-30T09:17:00.492Z","hook":"iteration-context","action":"skip","iterationCount":1}
45+
{"ts":"2026-06-30T09:17:00.491Z","hook":"dev-rules-reminder","action":"inject","iterationCount":0}
46+
{"ts":"2026-06-30T09:21:36.823Z","hook":"stop-notify","projectName":"hopeful-euclid-d75809","duration":"8m 8s","iterations":0}
47+
{"ts":"2026-06-30T09:21:38.488Z","hook":"session-init","projectRoot":"/Users/d-v-b/dev/zarr-python/.claude/worktrees/hopeful-euclid-d75809","gitBranch":"review-perf-3906"}
48+
{"ts":"2026-06-30T09:21:41.495Z","hook":"iteration-context","action":"skip","iterationCount":1}
49+
{"ts":"2026-06-30T09:21:41.494Z","hook":"dev-rules-reminder","action":"inject","iterationCount":0}
50+
{"ts":"2026-06-30T09:56:38.124Z","hook":"iteration-context","action":"skip","iterationCount":14}
51+
{"ts":"2026-06-30T09:56:38.301Z","hook":"stop-notify","projectName":"hopeful-euclid-d75809","duration":"4h 15m","iterations":0}
52+
{"ts":"2026-06-30T09:57:19.016Z","hook":"stop-notify","projectName":"hopeful-euclid-d75809","duration":"40m 18s","iterations":0}
53+
{"ts":"2026-06-30T09:57:19.598Z","hook":"stop-notify","projectName":"hopeful-euclid-d75809","duration":"35m 41s","iterations":0}
54+
{"ts":"2026-06-30T09:57:20.049Z","hook":"session-init","projectRoot":"/Users/d-v-b/dev/zarr-python/.claude/worktrees/hopeful-euclid-d75809","gitBranch":"review-perf-3906"}
55+
{"ts":"2026-06-30T09:57:20.111Z","hook":"dev-rules-reminder","action":"inject","iterationCount":0}
56+
{"ts":"2026-06-30T09:57:20.112Z","hook":"iteration-context","action":"skip","iterationCount":1}
57+
{"ts":"2026-06-30T09:57:21.131Z","hook":"session-init","projectRoot":"/Users/d-v-b/dev/zarr-python/.claude/worktrees/hopeful-euclid-d75809","gitBranch":"review-perf-3906"}
58+
{"ts":"2026-06-30T09:57:21.194Z","hook":"iteration-context","action":"skip","iterationCount":1}
59+
{"ts":"2026-06-30T09:58:55.359Z","hook":"session-init","projectRoot":"/Users/d-v-b/dev/zarr-python/.claude/worktrees/hopeful-euclid-d75809","gitBranch":"review-perf-3906"}
60+
{"ts":"2026-06-30T09:58:58.543Z","hook":"stop-notify","projectName":"hopeful-euclid-d75809","duration":"1m 38s","iterations":0}
61+
{"ts":"2026-06-30T09:59:18.679Z","hook":"iteration-context","action":"skip","iterationCount":1}
62+
{"ts":"2026-06-30T09:59:18.679Z","hook":"dev-rules-reminder","action":"inject","iterationCount":0}
63+
{"ts":"2026-06-30T10:01:31.848Z","hook":"stop-notify","projectName":"hopeful-euclid-d75809","duration":"4m 10s","iterations":0}
64+
{"ts":"2026-06-30T10:05:06.453Z","hook":"iteration-context","action":"skip","iterationCount":2}
65+
{"ts":"2026-06-30T10:12:39.387Z","hook":"session-init","projectRoot":"/Users/d-v-b/dev/zarr-python/.claude/worktrees/hopeful-euclid-d75809","gitBranch":"review-perf-3906"}
66+
{"ts":"2026-06-30T10:12:39.704Z","hook":"iteration-context","action":"skip","iterationCount":1}
67+
{"ts":"2026-06-30T10:12:39.704Z","hook":"dev-rules-reminder","action":"inject","iterationCount":0}
68+
{"ts":"2026-06-30T10:13:18.090Z","hook":"stop-notify","projectName":"hopeful-euclid-d75809","duration":"0m 38s","iterations":0}
69+
{"ts":"2026-06-30T10:17:27.824Z","hook":"iteration-context","action":"skip","iterationCount":3}
70+
{"ts":"2026-06-30T10:17:42.600Z","hook":"session-init","projectRoot":"/Users/d-v-b/dev/zarr-python/.claude/worktrees/hopeful-euclid-d75809","gitBranch":"review-perf-3906"}
71+
{"ts":"2026-06-30T10:17:43.194Z","hook":"iteration-context","action":"skip","iterationCount":1}
72+
{"ts":"2026-06-30T10:17:43.194Z","hook":"dev-rules-reminder","action":"inject","iterationCount":0}
73+
{"ts":"2026-06-30T10:21:04.446Z","hook":"stop-notify","projectName":"hopeful-euclid-d75809","duration":"3m 21s","iterations":0}
74+
{"ts":"2026-06-30T10:21:09.192Z","hook":"iteration-context","action":"skip","iterationCount":4}
75+
{"ts":"2026-06-30T10:32:33.765Z","hook":"iteration-context","action":"skip","iterationCount":5,"reason":"no-tsv"}
76+
{"ts":"2026-06-30T10:43:03.804Z","hook":"session-init","projectRoot":"/Users/d-v-b/dev/zarr-python/.claude/worktrees/hopeful-euclid-d75809","gitBranch":"review-perf-3906"}
77+
{"ts":"2026-06-30T10:43:04.312Z","hook":"iteration-context","action":"skip","iterationCount":1}
78+
{"ts":"2026-06-30T10:43:04.311Z","hook":"dev-rules-reminder","action":"inject","iterationCount":0}
79+
{"ts":"2026-06-30T10:44:44.291Z","hook":"dev-rules-reminder","action":"inject","iterationCount":5}
80+
{"ts":"2026-06-30T10:44:44.292Z","hook":"iteration-context","action":"skip","iterationCount":6}
81+
{"ts":"2026-06-30T10:44:56.867Z","hook":"session-init","projectRoot":"/Users/d-v-b/dev/zarr-python/.claude/worktrees/hopeful-euclid-d75809","gitBranch":"review-perf-3906"}
82+
{"ts":"2026-06-30T10:44:57.183Z","hook":"iteration-context","action":"skip","iterationCount":1}
83+
{"ts":"2026-06-30T10:44:57.183Z","hook":"dev-rules-reminder","action":"inject","iterationCount":0}
84+
{"ts":"2026-06-30T10:48:03.759Z","hook":"stop-notify","projectName":"hopeful-euclid-d75809","duration":"3m 6s","iterations":0}
85+
{"ts":"2026-06-30T10:48:08.783Z","hook":"iteration-context","action":"skip","iterationCount":7}
86+
{"ts":"2026-06-30T11:40:16.404Z","hook":"iteration-context","action":"skip","iterationCount":8}
87+
{"ts":"2026-06-30T12:05:04.200Z","hook":"stop-notify","projectName":"hopeful-euclid-d75809","duration":"2h 6m","iterations":0}
88+
{"ts":"2026-06-30T12:15:58.322Z","hook":"session-init","projectRoot":"/Users/d-v-b/dev/zarr-python/.claude/worktrees/hopeful-euclid-d75809","gitBranch":"review-perf-3906"}
89+
{"ts":"2026-06-30T12:16:30.832Z","hook":"dev-rules-reminder","action":"inject","iterationCount":0}
90+
{"ts":"2026-06-30T12:16:30.832Z","hook":"iteration-context","action":"skip","iterationCount":1}
91+
{"ts":"2026-06-30T12:32:55.429Z","hook":"iteration-context","action":"skip","iterationCount":2}
92+
{"ts":"2026-06-30T13:50:43.599Z","hook":"session-init","projectRoot":"/Users/d-v-b/dev/zarr-python/.claude/worktrees/hopeful-euclid-d75809","gitBranch":"lazy-indexing-pr2-indexing-tests"}
93+
{"ts":"2026-06-30T13:50:43.977Z","hook":"iteration-context","action":"skip","iterationCount":1}
94+
{"ts":"2026-06-30T13:50:43.977Z","hook":"dev-rules-reminder","action":"inject","iterationCount":0}
95+
{"ts":"2026-06-30T13:55:32.382Z","hook":"stop-notify","projectName":"hopeful-euclid-d75809","duration":"1h 39m","iterations":0}
96+
{"ts":"2026-06-30T13:55:58.707Z","hook":"session-init","projectRoot":"/Users/d-v-b/dev/zarr-python/.claude/worktrees/hopeful-euclid-d75809","gitBranch":"lazy-indexing-pr2-indexing-tests"}
97+
{"ts":"2026-06-30T13:55:58.810Z","hook":"iteration-context","action":"skip","iterationCount":1}
98+
{"ts":"2026-06-30T13:55:58.809Z","hook":"dev-rules-reminder","action":"inject","iterationCount":0}
99+
{"ts":"2026-06-30T14:04:16.713Z","hook":"stop-notify","projectName":"hopeful-euclid-d75809","duration":"13m 33s","iterations":0}
100+
{"ts":"2026-06-30T14:04:19.247Z","hook":"iteration-context","action":"skip","iterationCount":2}

‎src/zarr/testing/strategies.py‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
from zarr.core.chunk_key_encodings import DefaultChunkKeyEncoding
2828
from zarr.core.common import JSON, AccessModeLiteral, ZarrFormat
2929
from zarr.core.dtype import get_data_type_from_native_dtype
30+
from zarr.core.indexing import Selection
3031
from zarr.core.metadata import ArrayV2Metadata, ArrayV3Metadata
3132
from zarr.core.metadata.v3 import RectilinearChunkGridMetadata, RegularChunkGridMetadata
3233
from zarr.core.sync import sync
@@ -623,7 +624,9 @@ def windows(draw: st.DrawFn, *, shape: tuple[int, ...]) -> tuple[slice, ...]:
623624

624625

625626
@st.composite
626-
def indexers(draw: st.DrawFn, *, mode: IndexMode, shape: tuple[int, ...]) -> tuple[Any, Any]:
627+
def indexers(
628+
draw: st.DrawFn, *, mode: IndexMode, shape: tuple[int, ...]
629+
) -> tuple[Selection, Selection]:
627630
"""A ``(zarr_selection, numpy_selection)`` pair for ``mode`` on ``shape``.
628631
629632
One strategy covering every indexing mode, so a test can be written once and

‎tests/test_properties.py‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
from zarr.core.metadata import ArrayV2Metadata, ArrayV3Metadata
2323
from zarr.core.sync import sync
2424
from zarr.testing.strategies import (
25+
IndexMode,
2526
array_metadata,
2627
arrays,
2728
basic_indices,
@@ -332,11 +333,13 @@ def test_array_metadata_meets_spec(meta: ArrayV2Metadata | ArrayV3Metadata) -> N
332333
# The indexing modes and which Array method implements each. vindex/mask are
333334
# "vectorized" — they scatter through a single flat index, so an out= buffer must
334335
# be flat (number of selected points) rather than the multi-dimensional result.
335-
_INDEX_MODES = ("basic", "oindex", "vindex", "mask")
336-
_VECTORIZED_MODES = frozenset({"vindex", "mask"})
336+
_INDEX_MODES: tuple[IndexMode, ...] = ("basic", "oindex", "vindex", "mask")
337+
# Modes that scatter through a flat index (so an out= buffer must be flat). Kept a
338+
# tuple like _INDEX_MODES; membership is checked against it below.
339+
_VECTORIZED_MODES: tuple[IndexMode, ...] = ("vindex", "mask")
337340

338341

339-
def _get(target: zarr.Array, mode: str, zsel: Any, *, out: Any = None) -> Any:
342+
def _get(target: zarr.Array, mode: IndexMode, zsel: Any, *, out: Any = None) -> Any:
340343
"""Read ``zsel`` from ``target`` via the get-method for ``mode``."""
341344
if mode == "basic":
342345
return target.get_basic_selection(zsel, out=out)
@@ -349,7 +352,7 @@ def _get(target: zarr.Array, mode: str, zsel: Any, *, out: Any = None) -> Any:
349352
raise AssertionError(mode)
350353

351354

352-
def _async_get(async_array: Any, mode: str, zsel: Any) -> Any:
355+
def _async_get(async_array: Any, mode: IndexMode, zsel: Any) -> Any:
353356
"""The async read coroutine for ``mode`` (vindex/mask share the vectorized accessor)."""
354357
if mode == "basic":
355358
return async_array.getitem(zsel)
@@ -358,7 +361,7 @@ def _async_get(async_array: Any, mode: str, zsel: Any) -> Any:
358361
return async_array.vindex.getitem(zsel)
359362

360363

361-
def _setitem(zarray: zarr.Array, mode: str, zsel: Any, value: Any) -> None:
364+
def _setitem(zarray: zarr.Array, mode: IndexMode, zsel: Any, value: Any) -> None:
362365
"""Write ``value`` at ``zsel`` via the set-method for ``mode``."""
363366
if mode == "basic":
364367
zarray[zsel] = value
@@ -376,7 +379,7 @@ def _has_repeated_indices(npsel: Any) -> bool:
376379
return any(isinstance(i, np.ndarray) and i.size != np.unique(i).size for i in sel)
377380

378381

379-
def _eligible(mode: str, shape: tuple[int, ...]) -> bool:
382+
def _eligible(mode: IndexMode, shape: tuple[int, ...]) -> bool:
380383
"""Whether ``mode`` can be exercised on ``shape``.
381384
382385
Rank-0 arrays have no interesting selections; the fancy modes
@@ -388,7 +391,7 @@ def _eligible(mode: str, shape: tuple[int, ...]) -> bool:
388391

389392

390393
def assert_read_matches_numpy(
391-
target: zarr.Array, ref: np.ndarray[Any, Any], mode: str, zsel: Any, npsel: Any
394+
target: zarr.Array, ref: np.ndarray[Any, Any], mode: IndexMode, zsel: Any, npsel: Any
392395
) -> None:
393396
"""Assert ``target``'s read of ``zsel`` (mode) matches ``ref[npsel]``, with/without out=.
394397

0 commit comments

Comments
 (0)