From acdb9ef02990d0cd26547fd37d970a244c52f865 Mon Sep 17 00:00:00 2001 From: Davis Vann Bennett Date: Tue, 6 Aug 2024 15:16:00 +0200 Subject: [PATCH] raise RuntimeError when attempting to set or get a closed store --- src/zarr/abc/store.py | 4 ---- src/zarr/store/core.py | 1 - src/zarr/store/local.py | 2 +- src/zarr/store/memory.py | 6 +++--- src/zarr/store/remote.py | 3 ++- tests/v3/test_buffer.py | 14 ++++++++------ tests/v3/test_config.py | 11 ++++++----- tests/v3/test_indexing.py | 6 ++++-- tests/v3/test_store/test_memory.py | 4 ++-- 9 files changed, 26 insertions(+), 25 deletions(-) diff --git a/src/zarr/abc/store.py b/src/zarr/abc/store.py index 449816209..ad1514289 100644 --- a/src/zarr/abc/store.py +++ b/src/zarr/abc/store.py @@ -52,10 +52,6 @@ async def _open(self) -> None: raise FileExistsError("Store already exists") self._is_open = True - async def _ensure_open(self) -> None: - if not self._is_open: - await self._open() - @abstractmethod async def empty(self) -> bool: ... diff --git a/src/zarr/store/core.py b/src/zarr/store/core.py index fa3587930..2aabe61a4 100644 --- a/src/zarr/store/core.py +++ b/src/zarr/store/core.py @@ -80,7 +80,6 @@ async def make_store_path( elif isinstance(store_like, Store): if mode is not None: assert AccessMode.from_literal(mode) == store_like.mode - await store_like._ensure_open() return StorePath(store_like) elif store_like is None: if mode is None: diff --git a/src/zarr/store/local.py b/src/zarr/store/local.py index 25fd9fc13..1e26de9e0 100644 --- a/src/zarr/store/local.py +++ b/src/zarr/store/local.py @@ -73,11 +73,11 @@ class LocalStore(Store): root: Path def __init__(self, root: Path | str, *, mode: AccessModeLiteral = "r"): - super().__init__(mode=mode) if isinstance(root, str): root = Path(root) assert isinstance(root, Path) self.root = root + super().__init__(mode=mode) async def clear(self) -> None: self._check_writable() diff --git a/src/zarr/store/memory.py b/src/zarr/store/memory.py index dd3e52e70..25fdbd41d 100644 --- a/src/zarr/store/memory.py +++ b/src/zarr/store/memory.py @@ -23,8 +23,8 @@ def __init__( *, mode: AccessModeLiteral = "r", ): - super().__init__(mode=mode) self._store_dict = store_dict or {} + super().__init__(mode=mode) async def empty(self) -> bool: return not self._store_dict @@ -45,7 +45,7 @@ async def get( byte_range: tuple[int | None, int | None] | None = None, ) -> Buffer | None: if not self._is_open: - await self._open() + raise RuntimeError("Store is closed. Cannot `get` from a closed store.") assert isinstance(key, str) try: value = self._store_dict[key] @@ -71,7 +71,7 @@ async def exists(self, key: str) -> bool: async def set(self, key: str, value: Buffer, byte_range: tuple[int, int] | None = None) -> None: if not self._is_open: - await self._open() + raise RuntimeError("Store is closed. Cannot `get` from a closed store.") self._check_writable() assert isinstance(key, str) if not isinstance(value, Buffer): diff --git a/src/zarr/store/remote.py b/src/zarr/store/remote.py index c742d9e56..83361afcd 100644 --- a/src/zarr/store/remote.py +++ b/src/zarr/store/remote.py @@ -50,7 +50,7 @@ def __init__( storage_options: passed on to fsspec to make the filesystem instance. If url is a UPath, this must not be used. """ - super().__init__(mode=mode) + if isinstance(url, str): self._url = url.rstrip("/") self._fs, _path = fsspec.url_to_fs(url, **storage_options) @@ -73,6 +73,7 @@ def __init__( # test instantiate file system if not self._fs.async_impl: raise TypeError("FileSystem needs to support async operations") + super().__init__(mode=mode) async def clear(self) -> None: try: diff --git a/tests/v3/test_buffer.py b/tests/v3/test_buffer.py index d53e98d42..1b72aaacb 100644 --- a/tests/v3/test_buffer.py +++ b/tests/v3/test_buffer.py @@ -1,5 +1,7 @@ from __future__ import annotations +from typing import Any + import numpy as np import pytest @@ -20,19 +22,19 @@ ) -def test_nd_array_like(xp): +def test_nd_array_like(xp: Any) -> None: ary = xp.arange(10) assert isinstance(ary, ArrayLike) assert isinstance(ary, NDArrayLike) @pytest.mark.asyncio -async def test_async_array_prototype(): +async def test_async_array_prototype() -> None: """Test the use of a custom buffer prototype""" expect = np.zeros((9, 9), dtype="uint16", order="F") a = await AsyncArray.create( - StorePath(StoreExpectingTestBuffer(mode="w")) / "test_async_array_prototype", + StorePath(await StoreExpectingTestBuffer.open(mode="w")) / "test_async_array_prototype", shape=expect.shape, chunk_shape=(5, 5), dtype=expect.dtype, @@ -53,10 +55,10 @@ async def test_async_array_prototype(): @pytest.mark.asyncio -async def test_codecs_use_of_prototype(): +async def test_codecs_use_of_prototype() -> None: expect = np.zeros((10, 10), dtype="uint16", order="F") a = await AsyncArray.create( - StorePath(StoreExpectingTestBuffer(mode="w")) / "test_codecs_use_of_prototype", + StorePath(await StoreExpectingTestBuffer.open(mode="w")) / "test_codecs_use_of_prototype", shape=expect.shape, chunk_shape=(5, 5), dtype=expect.dtype, @@ -84,7 +86,7 @@ async def test_codecs_use_of_prototype(): assert np.array_equal(expect, got) -def test_numpy_buffer_prototype(): +def test_numpy_buffer_prototype() -> None: buffer = numpy_buffer_prototype().buffer.create_zero_length() ndbuffer = numpy_buffer_prototype().nd_buffer.create(shape=(1, 2), dtype=np.dtype("int64")) assert isinstance(buffer.as_array_like(), np.ndarray) diff --git a/tests/v3/test_config.py b/tests/v3/test_config.py index 8e7b86852..956db79df 100644 --- a/tests/v3/test_config.py +++ b/tests/v3/test_config.py @@ -27,6 +27,7 @@ register_ndbuffer, register_pipeline, ) +from zarr.sync import sync from zarr.testing.buffer import ( NDBufferUsingTestNDArrayLike, StoreExpectingTestBuffer, @@ -191,11 +192,11 @@ def test_config_ndbuffer_implementation(store): assert isinstance(got, TestNDArrayLike) -def test_config_buffer_implementation(): +def test_config_buffer_implementation() -> None: # has default value assert fully_qualified_name(get_buffer_class()) == config.defaults[0]["buffer"] - - arr = zeros(shape=(100), store=StoreExpectingTestBuffer(mode="w")) + store = sync(StoreExpectingTestBuffer.open(mode="w")) + arr = zeros(shape=(100), store=store) # AssertionError of StoreExpectingTestBuffer when not using my buffer with pytest.raises(AssertionError): @@ -213,7 +214,7 @@ def test_config_buffer_implementation(): data2d = np.arange(1000).reshape(100, 10) arr_sharding = zeros( shape=(100, 10), - store=StoreExpectingTestBuffer(mode="w"), + store=sync(StoreExpectingTestBuffer.open(mode="w")), codecs=[ShardingCodec(chunk_shape=(10, 10))], ) arr_sharding[:] = data2d @@ -221,7 +222,7 @@ def test_config_buffer_implementation(): arr_Crc32c = zeros( shape=(100, 10), - store=StoreExpectingTestBuffer(mode="w"), + store=sync(StoreExpectingTestBuffer.open(mode="w")), codecs=[BytesCodec(), Crc32cCodec()], ) arr_Crc32c[:] = data2d diff --git a/tests/v3/test_indexing.py b/tests/v3/test_indexing.py index e170852e9..f913d824a 100644 --- a/tests/v3/test_indexing.py +++ b/tests/v3/test_indexing.py @@ -27,11 +27,12 @@ from zarr.registry import get_ndbuffer_class from zarr.store.core import StorePath from zarr.store.memory import MemoryStore +from zarr.sync import sync @pytest.fixture def store() -> Iterator[StorePath]: - yield StorePath(MemoryStore(mode="w")) + yield StorePath(sync(MemoryStore.open(mode="w"))) def zarr_array_from_numpy_array( @@ -1838,4 +1839,5 @@ def test_indexing_after_close(store: StorePath) -> None: a[:] = nparray assert a[:] == value store.store.close() - assert a[:] == value + with pytest.raises(RuntimeError, match="Store is closed"): + assert a[:] == value diff --git a/tests/v3/test_store/test_memory.py b/tests/v3/test_store/test_memory.py index 5b8f1ef87..63a631020 100644 --- a/tests/v3/test_store/test_memory.py +++ b/tests/v3/test_store/test_memory.py @@ -23,8 +23,8 @@ def store_kwargs( return {"store_dict": request.param, "mode": "r+"} @pytest.fixture(scope="function") - def store(self, store_kwargs: str | None | dict[str, Buffer]) -> MemoryStore: - return self.store_cls(**store_kwargs) + async def store(self, store_kwargs: str | None | dict[str, Buffer]) -> MemoryStore: + return await self.store_cls.open(**store_kwargs) def test_store_repr(self, store: MemoryStore) -> None: assert str(store) == f"memory://{id(store._store_dict)}"