fixups

Author: jhamman

src/zarr/abc/store.py

@@ -82,7 +82,7 @@ async def _ensure_open(self) -> None:
         if not self._is_open:
             await self._open()
 
-    async def empty(self, prefix: str = "") -> bool:
+    async def empty_dir(self, prefix: str = "") -> bool:
         """
         Check if the directory is empty.
 
@@ -310,7 +310,7 @@ async def delete_dir(self, prefix: str) -> None:
         if not self.supports_listing:
             raise NotImplementedError
         self._check_writable()
-        if not prefix.endswith("/"):
+        if prefix and not prefix.endswith("/"):
             prefix += "/"
         async for key in self.list_prefix(prefix):
             await self.delete(key)

src/zarr/storage/common.py

@@ -45,6 +45,10 @@ def __init__(self, store: Store, path: str = "") -> None:
         self.store = store
         self.path = path
 
+    @property
+    def readonly(self) -> bool:
+        return self.store.readonly
+
     @classmethod
     async def open(
         cls, store: Store, path: str, mode: AccessModeLiteral | None = None
@@ -79,7 +83,7 @@ async def open(
 
         match mode:
             case "w-":
-                if not await self.empty():
+                if not await self.empty_dir():
                     msg = (
                         f"{self} is not empty, but `mode` is set to 'w-'."
                         "Either remove the existing objects in storage,"
@@ -183,7 +187,7 @@ async def exists(self) -> bool:
         """
         return await self.store.exists(self.path)
 
-    async def empty(self) -> bool:
+    async def empty_dir(self) -> bool:
         """
         Check if any keys exist in the store with the given prefix.
 
@@ -192,7 +196,7 @@ async def empty(self) -> bool:
         bool
             True if no keys exist in the store with the given prefix, False otherwise.
         """
-        return await self.store.empty(self.path)
+        return await self.store.empty_dir(self.path)
 
     def __truediv__(self, other: str) -> StorePath:
         """Combine this store path with another path"""

src/zarr/storage/logging.py

@@ -135,10 +135,10 @@ async def _ensure_open(self) -> None:
         with self.log():
             return await self._store._ensure_open()
 
-    async def empty(self, prefix: str = "") -> bool:
+    async def empty_dir(self, prefix: str = "") -> bool:
         # docstring inherited
         with self.log():
-            return await self._store.empty(prefix=prefix)
+            return await self._store.empty_dir(prefix=prefix)
 
     async def clear(self) -> None:
         # docstring inherited

src/zarr/storage/memory.py

@@ -17,9 +17,6 @@
 logger = getLogger(__name__)
 
 
-logger = getLogger(__name__)
-
-
 class MemoryStore(Store):
     """
     In-memory store for testing purposes.

src/zarr/storage/zip.py

@@ -65,10 +65,13 @@ def __init__(
         path: Path | str,
         *,
         mode: ZipStoreAccessModeLiteral = "r",
-        readonly: bool = True,
+        readonly: bool | None = None,
         compression: int = zipfile.ZIP_STORED,
         allowZip64: bool = True,
     ) -> None:
+        if readonly is None:
+            readonly = mode == "r"
+
         super().__init__(readonly=readonly)
 
         if isinstance(path, str):
@@ -210,6 +213,8 @@ async def set_if_not_exists(self, key: str, value: Buffer) -> None:
                 self._set(key, value)
 
     async def delete_dir(self, prefix: str) -> None:
+        # only raise NotImplementedError if any keys are found
+        self._check_writable()
         if not prefix.endswith("/"):
             prefix += "/"
         async for _ in self.list_prefix(prefix):
@@ -219,6 +224,7 @@ async def delete(self, key: str) -> None:
         # docstring inherited
         # we choose to only raise NotImplementedError here if the key exists
         # this allows the array/group APIs to avoid the overhead of existence checks
+        self._check_writable()
         if await self.exists(key):
             raise NotImplementedError
 

src/zarr/testing/store.py

@@ -75,7 +75,7 @@ async def test_store_open_readonly(self, store_kwargs: dict[str, Any], readonly:
         assert store._is_open
         assert store.readonly == readonly
 
-    async def test_not_writable_store_raises(self, store_kwargs: dict[str, Any]) -> None:
+    async def test_readonly_store_raises(self, store_kwargs: dict[str, Any]) -> None:
         kwargs = {**store_kwargs, "readonly": True}
         store = await self.store_cls.open(**kwargs)
         assert store.readonly
@@ -228,19 +228,21 @@ async def test_delete_dir(self, store: S) -> None:
         assert not await store.exists("foo/zarr.json")
         assert not await store.exists("foo/c/0")
 
-    async def test_empty(self, store: S) -> None:
-        assert await store.empty()
+    async def test_empty_dir(self, store: S) -> None:
+        assert await store.empty_dir()
         await self.set(
-            store, "key", self.buffer_cls.from_bytes(bytes("something", encoding="utf-8"))
+            store, "foo/bar", self.buffer_cls.from_bytes(bytes("something", encoding="utf-8"))
         )
-        assert not await store.empty()
+        assert not await store.empty_dir()
+        assert not await store.empty_dir("foo")
+        assert await store.empty_dir("spam/")
 
     async def test_clear(self, store: S) -> None:
         await self.set(
             store, "key", self.buffer_cls.from_bytes(bytes("something", encoding="utf-8"))
         )
         await store.clear()
-        assert await store.empty()
+        assert await store.empty_dir()
 
     async def test_list(self, store: S) -> None:
         assert await _collect_aiterator(store.list()) == ()
@@ -297,40 +299,6 @@ async def test_list_dir(self, store: S) -> None:
         keys_observed = await _collect_aiterator(store.list_dir(root + "/"))
         assert sorted(keys_expected) == sorted(keys_observed)
 
-    # async def test_with_mode(self, store: S) -> None:
-    #     data = b"0000"
-    #     await self.set(store, "key", self.buffer_cls.from_bytes(data))
-    #     assert (await self.get(store, "key")).to_bytes() == data
-
-    #     modes: list[StoreAccessMode] = ["r", "w"]
-    #     for mode in modes:
-    #         clone = store.with_mode(mode)
-    #         await clone._ensure_open()
-    #         assert clone.mode == mode
-    #         assert isinstance(clone, type(store))
-
-    #         # earlier writes are visible
-    #         result = await clone.get("key", default_buffer_prototype())
-    #         assert result is not None
-    #         assert result.to_bytes() == data
-
-    #         # writes to original after with_mode is visible
-    #         await self.set(store, "key-2", self.buffer_cls.from_bytes(data))
-    #         result = await clone.get("key-2", default_buffer_prototype())
-    #         assert result is not None
-    #         assert result.to_bytes() == data
-
-    #         if mode == "w":
-    #             # writes to clone is visible in the original
-    #             await clone.set("key-3", self.buffer_cls.from_bytes(data))
-    #             result = await clone.get("key-3", default_buffer_prototype())
-    #             assert result is not None
-    #             assert result.to_bytes() == data
-
-    #         else:
-    #             with pytest.raises(ValueError, match="store mode"):
-    #                 await clone.set("key-3", self.buffer_cls.from_bytes(data))
-
     async def test_set_if_not_exists(self, store: S) -> None:
         key = "k"
         data_buf = self.buffer_cls.from_bytes(b"0000")

src/zarr/testing/strategies.py

@@ -8,8 +8,6 @@
 
 import zarr
 from zarr.core.array import Array
-
-# from zarr.core.group import Group
 from zarr.storage import MemoryStore, StoreLike
 
 # Copied from Xarray

tests/conftest.py

@@ -28,13 +28,13 @@ async def parse_store(
     store: Literal["local", "memory", "remote", "zip"], path: str
 ) -> LocalStore | MemoryStore | RemoteStore | ZipStore:
     if store == "local":
-        return await LocalStore.open(path, readonly=False)
+        return await LocalStore.open(path)
     if store == "memory":
         return await MemoryStore.open(readonly=False)
     if store == "remote":
-        return await RemoteStore.open(url=path, readonly=False)
+        return await RemoteStore.open(url=path)
     if store == "zip":
-        return await ZipStore.open(path + "/zarr.zip", readonly=False, mode="w")
+        return await ZipStore.open(path + "/zarr.zip", mode="w")
     raise AssertionError
 
 
@@ -46,18 +46,18 @@ def path_type(request: pytest.FixtureRequest) -> Any:
 # todo: harmonize this with local_store fixture
 @pytest.fixture
 async def store_path(tmpdir: LEGACY_PATH) -> StorePath:
-    store = await LocalStore.open(str(tmpdir), readonly=False)
+    store = await LocalStore.open(str(tmpdir))
     return StorePath(store)
 
 
 @pytest.fixture
 async def local_store(tmpdir: LEGACY_PATH) -> LocalStore:
-    return await LocalStore.open(str(tmpdir), readonly=False)
+    return await LocalStore.open(str(tmpdir))
 
 
 @pytest.fixture
 async def remote_store(url: str) -> RemoteStore:
-    return await RemoteStore.open(url, readonly=False)
+    return await RemoteStore.open(url)
 
 
 @pytest.fixture
@@ -67,7 +67,7 @@ async def memory_store() -> MemoryStore:
 
 @pytest.fixture
 async def zip_store(tmpdir: LEGACY_PATH) -> ZipStore:
-    return await ZipStore.open(str(tmpdir / "zarr.zip"), mode="w", readonly=False)
+    return await ZipStore.open(str(tmpdir / "zarr.zip"), mode="w")
 
 
 @pytest.fixture

tests/test_store/test_core.py

@@ -40,7 +40,7 @@ async def test_make_store_path_local(
     assert isinstance(store_path.store, LocalStore)
     assert Path(store_path.store.root) == Path(tmpdir)
     assert store_path.path == normalize_path(path)
-    assert store_path.store.readonly == (mode == "r")
+    assert store_path.readonly == (mode == "r")
 
 
 @pytest.mark.parametrize("path", [None, "", "bar"])
@@ -60,7 +60,7 @@ async def test_make_store_path_store_path(
     path_normalized = normalize_path(path)
     assert store_path.path == (store_like / path_normalized).path
 
-    assert store_path.store.readonly == (mode == "r")
+    assert store_path.readonly == ro
 
 
 async def test_make_store_path_invalid() -> None:

tests/test_store/test_local.py

@@ -43,9 +43,9 @@ def test_store_supports_listing(self, store: LocalStore) -> None:
         assert store.supports_listing
 
     async def test_empty_with_empty_subdir(self, store: LocalStore) -> None:
-        assert await store.empty()
+        assert await store.empty_dir()
         (store.root / "foo/bar").mkdir(parents=True)
-        assert await store.empty()
+        assert await store.empty_dir()
 
     def test_creates_new_directory(self, tmp_path: pathlib.Path):
         target = tmp_path.joinpath("a", "b", "c")