Skip to content

Commit 6121499

Browse files
authored
Merge branch 'zarr-developers:main' into main
2 parents 43e4d9e + 123268a commit 6121499

16 files changed

Lines changed: 559 additions & 14 deletions

File tree

‎.github/workflows/codspeed.yml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ jobs:
3333
version: '1.16.5'
3434
- name: Run the benchmarks
3535
uses: CodSpeedHQ/action@f99becdce5e5d51fd556489ebef684f4ecfd6286 # v4.18.5
36+
env:
37+
ZARR_BENCHMARK_CLEAR_CACHE: '1'
3638
with:
3739
mode: walltime
3840
run: hatch run test.py3.12-minimal:pytest tests/benchmarks --codspeed

‎.github/workflows/test.yml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,8 @@ jobs:
178178
- name: Install Hatch
179179
run: python -m pip install hatch==1.16.5
180180
- name: Run Benchmarks
181+
env:
182+
ZARR_BENCHMARK_CLEAR_CACHE: '1'
181183
run: |
182184
hatch env run --env "test.py3.13-minimal" run-benchmark
183185

‎changes/4172.misc.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
Improved `CoordinateIndexer` construction for large, sorted, in-bounds, one-dimensional integer
2+
coordinate selections over regular chunk grids (e.g. `arr.get_coordinate_selection(sorted_idx)`,
3+
`arr.vindex[sorted_idx]`, and the gather behind sparse/CSR row selections). When boundary searching
4+
is estimated to be cheaper than processing every coordinate, per-chunk projections are now built
5+
with `searchsorted`, making index construction ~15x faster for large gathers. Sparse sorted
6+
selections spanning many chunks relative to their coordinate count, as well as unsorted, negative,
7+
multi-dimensional, and irregular-grid selections, continue to use the existing implementation.

‎changes/4179.bugfix.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fixed the opt-in `FusedCodecPipeline` for sharded arrays whose inner or index codec chain contains a codec implementing only the async codec interface (no `SupportsSyncCodec`). Such arrays previously raised `TypeError: All codecs must implement SupportsSyncCodec` on both read and write; the pipeline now declines its synchronous fast path for them and falls back to the async path, matching the behavior of the default `BatchedCodecPipeline`. Fully sync-capable codec chains keep the fast path unchanged.

‎changes/4183.bugfix.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Fixed `TypeError: unhashable type: 'writeable void-scalar'` when writing to sharded arrays whose fill value is a `np.void` scalar, e.g. arrays with a structured dtype.
2+
3+
`ArraySpec` equality and hashing now compare the fill value by its byte representation rather than numeric equality. As a result, two specs with a `NaN` (or `NaT`) fill value now compare equal, while fill values of `-0.0` and `0.0` now compare unequal. This also restores the sharding codec's per-chunk spec cache, which had been disabled because of this bug.

‎changes/4199.bugfix.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
The end-to-end benchmarks no longer invoke `sudo` to drop the OS page cache during a regular `pytest` run. Cache clearing is now opt-in via the `ZARR_BENCHMARK_CLEAR_CACHE` environment variable, which the benchmark CI jobs set.

‎src/zarr/abc/codec.py‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,20 @@ def _decode_sync(self, chunk_data: CO, chunk_spec: ArraySpec) -> CI: ...
8282
def _encode_sync(self, chunk_data: CI, chunk_spec: ArraySpec) -> CO | None: ...
8383

8484

85+
def _codec_supports_sync(codec: object) -> bool:
86+
"""Whether `codec` can actually run on a synchronous (no event loop) path.
87+
88+
Structural membership in `SupportsSyncCodec` is necessary but not always
89+
sufficient: a codec can provide `_decode_sync`/`_encode_sync` whose ability
90+
to run depends on runtime configuration the type system cannot see.
91+
`ShardingCodec` is the canonical case — its sync methods delegate to its
92+
configured inner and index codec chains, so they only work when every codec
93+
in those chains is itself sync-capable. Such codecs opt out dynamically via
94+
a `_sync_capable` attribute/property (absent means capable).
95+
"""
96+
return isinstance(codec, SupportsSyncCodec) and getattr(codec, "_sync_capable", True)
97+
98+
8599
class BaseCodec[CI: CodecInput, CO: CodecOutput](Metadata):
86100
"""Generic base class for codecs.
87101

‎src/zarr/codecs/sharding.py‎

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
ArrayBytesCodecPartialEncodeMixin,
1515
Codec,
1616
CodecPipeline,
17-
SupportsSyncCodec,
17+
_codec_supports_sync,
1818
)
1919
from zarr.abc.store import (
2020
ByteGetter,
@@ -411,11 +411,7 @@ def __init__(
411411
object.__setattr__(self, "subchunk_write_order", subchunk_write_order)
412412

413413
# Use instance-local lru_cache to avoid memory leaks
414-
415-
# numpy void scalars are not hashable, which means an array spec with a fill value that is
416-
# a numpy void scalar will break the lru_cache. This is commented for now but should be
417-
# fixed. See https://github.com/zarr-developers/zarr-python/issues/3054
418-
# object.__setattr__(self, "_get_chunk_spec", lru_cache()(self._get_chunk_spec))
414+
object.__setattr__(self, "_get_chunk_spec", lru_cache()(self._get_chunk_spec))
419415
object.__setattr__(self, "_get_index_chunk_spec", lru_cache()(self._get_index_chunk_spec))
420416
object.__setattr__(self, "_get_chunks_per_shard", lru_cache()(self._get_chunks_per_shard))
421417
object.__setattr__(self, "_shard_index_size", lru_cache()(self._shard_index_size))
@@ -441,7 +437,7 @@ def __setstate__(self, state: dict[str, Any]) -> None:
441437
object.__setattr__(self, "subchunk_write_order", state["subchunk_write_order"])
442438

443439
# Use instance-local lru_cache to avoid memory leaks
444-
# object.__setattr__(self, "_get_chunk_spec", lru_cache()(self._get_chunk_spec))
440+
object.__setattr__(self, "_get_chunk_spec", lru_cache()(self._get_chunk_spec))
445441
object.__setattr__(self, "_get_index_chunk_spec", lru_cache()(self._get_index_chunk_spec))
446442
object.__setattr__(self, "_get_chunks_per_shard", lru_cache()(self._get_chunks_per_shard))
447443
object.__setattr__(self, "_shard_index_size", lru_cache()(self._shard_index_size))
@@ -1406,8 +1402,30 @@ def _is_complete_shard_write(
14061402
is_complete_chunk for *_, is_complete_chunk in indexed_chunks
14071403
)
14081404

1405+
@property
1406+
def _sync_capable(self) -> bool:
1407+
"""Dynamic opt-out consulted by `_codec_supports_sync` / `ChunkTransform`.
1408+
1409+
This codec structurally satisfies `SupportsSyncCodec`, but every sync
1410+
method (`_decode_sync`, `_encode_sync`, `_decode_partial_sync`,
1411+
`_encode_partial_sync`) delegates to the inner and index codec chains
1412+
through `ChunkTransform`, so it can only run synchronously when every
1413+
codec in BOTH chains is itself sync-capable. Reporting False here makes
1414+
`ChunkTransform` construction raise, which in turn makes
1415+
`FusedCodecPipeline.evolve_from_array_spec` set `sync_transform=None` —
1416+
the whole pipeline then declines the sync fast path and routes through
1417+
the async paths (partial shard decode / async fallback write), exactly
1418+
as it does for an async-only TOP-level codec or a non-sync store.
1419+
"""
1420+
return self._inner_codecs_sync_capable() and self._index_codecs_sync_capable()
1421+
1422+
def _inner_codecs_sync_capable(self) -> bool:
1423+
# _codec_supports_sync (not bare isinstance) so a nested sharding codec
1424+
# with an async-only inner chain propagates its opt-out outward.
1425+
return all(_codec_supports_sync(c) for c in self.codecs)
1426+
14091427
def _index_codecs_sync_capable(self) -> bool:
1410-
return all(isinstance(c, SupportsSyncCodec) for c in self.index_codecs)
1428+
return all(_codec_supports_sync(c) for c in self.index_codecs)
14111429

14121430
async def _decode_shard_index(
14131431
self, index_bytes: Buffer, chunks_per_shard: tuple[int, ...]

‎src/zarr/core/array_spec.py‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
from dataclasses import dataclass, fields
44
from typing import TYPE_CHECKING, Any, Literal, Self, TypedDict, cast
55

6+
import numpy as np
7+
68
from zarr.core.common import (
79
MemoryOrder,
810
parse_bool,
@@ -132,7 +134,7 @@ def parse_array_config(data: ArrayConfigLike | None) -> ArrayConfig:
132134
return ArrayConfig.from_dict(data)
133135

134136

135-
@dataclass(frozen=True)
137+
@dataclass(frozen=True, eq=False)
136138
class ArraySpec:
137139
shape: tuple[int, ...]
138140
dtype: ZDType[TBaseDType, TBaseScalar]
@@ -157,6 +159,24 @@ def __init__(
157159
object.__setattr__(self, "config", config)
158160
object.__setattr__(self, "prototype", prototype)
159161

162+
def _key(self) -> tuple[object, ...]:
163+
"""Returns the tuple used for equality/hash identity."""
164+
fill_value = self.fill_value
165+
if isinstance(fill_value, np.generic):
166+
# fill_values should be byte-identical, otherwise they correspond to different values in memory / on disk.
167+
# Importantly, this ensures np.nan == np.nan, NaT == NaT, and -0.0 != 0.0.
168+
# It also fixes np.void fill_values being unhashable (#3054).
169+
fill_value = fill_value.tobytes()
170+
return (self.shape, self.dtype, fill_value, self.config, self.prototype)
171+
172+
def __eq__(self, other: object) -> bool:
173+
if not isinstance(other, ArraySpec):
174+
return NotImplemented
175+
return self._key() == other._key()
176+
177+
def __hash__(self) -> int:
178+
return hash(self._key())
179+
160180
@property
161181
def ndim(self) -> int:
162182
return len(self.shape)

‎src/zarr/core/chunk_utils.py‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
from dataclasses import dataclass, field
44
from typing import TYPE_CHECKING, Any, cast
55

6-
from zarr.abc.codec import GetResult, SupportsSyncCodec
6+
from zarr.abc.codec import GetResult, SupportsSyncCodec, _codec_supports_sync
77
from zarr.core.indexing import is_scalar
88

99
if TYPE_CHECKING:
@@ -240,7 +240,13 @@ class ChunkTransform:
240240
def __post_init__(self) -> None:
241241
from zarr.core.codec_pipeline import codecs_from_list
242242

243-
non_sync = [c for c in self.codecs if not isinstance(c, SupportsSyncCodec)]
243+
# _codec_supports_sync, not a bare isinstance check: a codec can satisfy
244+
# the SupportsSyncCodec protocol structurally yet be unable to run
245+
# synchronously (ShardingCodec whose inner/index chain contains an
246+
# async-only codec). Such codecs opt out via `_sync_capable`, and the
247+
# TypeError here is what makes FusedCodecPipeline.evolve_from_array_spec
248+
# decline the sync fast path and fall back to the async pipeline.
249+
non_sync = [c for c in self.codecs if not _codec_supports_sync(c)]
244250
if non_sync:
245251
names = ", ".join(type(c).__name__ for c in non_sync)
246252
raise TypeError(

0 commit comments

Comments
 (0)