Skip to content

Commit

Permalink
raise RuntimeError when attempting to set or get a closed store
Browse files Browse the repository at this point in the history
  • Loading branch information
d-v-b committed Aug 6, 2024
1 parent 8dceef2 commit acdb9ef
Show file tree
Hide file tree
Showing 9 changed files with 26 additions and 25 deletions.
4 changes: 0 additions & 4 deletions src/zarr/abc/store.py
Original file line number Diff line number Diff line change
Expand Up @@ -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: ...

Expand Down
1 change: 0 additions & 1 deletion src/zarr/store/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion src/zarr/store/local.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
6 changes: 3 additions & 3 deletions src/zarr/store/memory.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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]
Expand All @@ -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):
Expand Down
3 changes: 2 additions & 1 deletion src/zarr/store/remote.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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:
Expand Down
14 changes: 8 additions & 6 deletions tests/v3/test_buffer.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
from __future__ import annotations

from typing import Any

import numpy as np
import pytest

Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
11 changes: 6 additions & 5 deletions tests/v3/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
register_ndbuffer,
register_pipeline,
)
from zarr.sync import sync
from zarr.testing.buffer import (
NDBufferUsingTestNDArrayLike,
StoreExpectingTestBuffer,
Expand Down Expand Up @@ -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):
Expand All @@ -213,15 +214,15 @@ 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
assert np.array_equal(arr_sharding[:], data2d)

arr_Crc32c = zeros(
shape=(100, 10),
store=StoreExpectingTestBuffer(mode="w"),
store=sync(StoreExpectingTestBuffer.open(mode="w")),
codecs=[BytesCodec(), Crc32cCodec()],
)
arr_Crc32c[:] = data2d
Expand Down
6 changes: 4 additions & 2 deletions tests/v3/test_indexing.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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
4 changes: 2 additions & 2 deletions tests/v3/test_store/test_memory.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)}"
Expand Down

0 comments on commit acdb9ef

Please sign in to comment.