Skip to content

Commit 103e58d

Browse files
committed
fix(tests,engine): guard zarrista tests with importorskip; engine reads return writable arrays
Two fixes from roborev review 371: - tests/zarrista lacked the importorskip guard its tests/zarrs siblings have, so any environment without the optional zarrista package failed at pytest collection (the default test env does not install zarrista). Guarded the module, and the obstore-dependent fixture/test likewise skip instead of erroring when obstore is absent. Verified by running the suite with the zarrista import blocked: clean skips, no collection errors. - Engine reads copied for parity with the native path: read_region returns a read-only view over the backend's immutable bytes, which leaked through arr[...] as an immutable result. Pinned by a writability test. Assisted-by: ClaudeCode:claude-opus-4.8
1 parent d7e2e22 commit 103e58d

4 files changed

Lines changed: 22 additions & 5 deletions

File tree

src/zarr/core/array.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5471,7 +5471,9 @@ async def _engine_get_selection(
54715471
)
54725472
if isinstance(indexer, BasicIndexer) and indexer.shape == ():
54735473
return cast("NDArrayLikeOrScalar", result[()])
5474-
return result
5474+
# `read_region` returns a read-only view over the backend's immutable bytes;
5475+
# the native path returns a writable buffer, so copy for parity.
5476+
return result.copy()
54755477

54765478

54775479
async def _get_selection(

tests/test_engine.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,19 @@ def test_getitem_via_engine_matches_native(local: LocalStore, engine: str) -> No
111111
np.testing.assert_array_equal(arr[2:6, 1:5], data[2:6, 1:5])
112112

113113

114+
@pytest.mark.parametrize("engine", ENGINES)
115+
def test_getitem_result_is_writable_under_engine(local: LocalStore, engine: str) -> None:
116+
"""Engine reads return a writable array, matching the native path (the crud
117+
facade's read-only view must not leak through `arr[...]`)."""
118+
native = zarr.create_array(store=local, name="a", shape=(8, 8), chunks=(4, 4), dtype="uint16")
119+
native[:] = _ramp()
120+
121+
arr = zarr.open_array(store=local, path="a", engine=engine)
122+
result = np.asarray(arr[2:6, 1:5])
123+
assert result.flags.writeable
124+
result[0, 0] = 42 # must not raise
125+
126+
114127
@pytest.mark.parametrize("engine", ENGINES)
115128
def test_scalar_getitem_via_engine(local: LocalStore, engine: str) -> None:
116129
"""A full-integer selection returns a scalar, matching native semantics."""

tests/zarrista/conftest.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,13 @@ async def memory_store() -> AsyncIterator[Store]:
3939
async def object_store(tmp_path: Path) -> AsyncIterator[Store]:
4040
"""A zarr ObjectStore wrapping an obstore LocalStore — ingested via
4141
zarrista's async API."""
42-
import obstore.store
42+
obstore_store = pytest.importorskip("obstore.store", reason="obstore is not installed")
4343

4444
from zarr.storage import ObjectStore
4545

4646
root = tmp_path / "obj"
4747
root.mkdir()
48-
s: Store = await ObjectStore.open(obstore.store.LocalStore(str(root)))
48+
s: Store = await ObjectStore.open(obstore_store.LocalStore(str(root)))
4949
try:
5050
yield s
5151
finally:

tests/zarrista/test_backend.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
import numpy as np
1414
import pytest
1515

16+
pytest.importorskip("zarrista", reason="zarrista is not installed", exc_type=ImportError)
17+
1618
from zarr.crud import ReferenceBackend
1719
from zarr.zarrista import UnsupportedStoreError, ZarristaBackend
1820

@@ -231,11 +233,11 @@ async def test_object_store_read_subset(object_store: Store) -> None:
231233
async def test_memory_backed_object_store_raises() -> None:
232234
"""An ObjectStore wrapping an obstore MemoryStore is rejected at the gate:
233235
zarrista cannot ingest memory-backed obstore stores."""
234-
import obstore.store
236+
obstore_store = pytest.importorskip("obstore.store", reason="obstore is not installed")
235237

236238
from zarr.storage import ObjectStore
237239

238-
store: Store = await ObjectStore.open(obstore.store.MemoryStore())
240+
store: Store = await ObjectStore.open(obstore_store.MemoryStore())
239241
backend = ZarristaBackend()
240242
with pytest.raises(UnsupportedStoreError):
241243
await backend.create_array(store, "a", array_metadata(), overwrite=False)

0 commit comments

Comments
 (0)