respond to reviews

Author: jhamman

docs/guide/storage.rst

@@ -43,7 +43,7 @@ filesystem.
 .. code-block:: python
 
    >>> import zarr
-   >>> store = zarr.storage.LocalStore("data/foo/bar", readonly=True)
+   >>> store = zarr.storage.LocalStore("data/foo/bar", read_only=True)
    >>> zarr.open(store=store)
    <Group file://data/foo/bar>
 
@@ -72,7 +72,7 @@ that implements the `AbstractFileSystem` API,
 .. code-block:: python
 
    >>> import zarr
-   >>> store = zarr.storage.RemoteStore.from_url("gs://foo/bar", readonly=True)
+   >>> store = zarr.storage.RemoteStore.from_url("gs://foo/bar", read_only=True)
    >>> zarr.open(store=store)
    <Array <RemoteStore(GCSFileSystem, foo/bar)> shape=(10, 20) dtype=float32>
 

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_dir(self, prefix: str = "") -> bool:
+    async def empty_dir(self, prefix: str) -> bool:
         """
         Check if the directory is empty.
 
@@ -96,8 +96,9 @@ async def empty_dir(self, prefix: str = "") -> bool:
         bool
             True if the store is empty, False otherwise.
         """
-
-        if prefix and not prefix.endswith("/"):
+        if not self.supports_listing:
+            raise NotImplementedError
+        if prefix != "" and not prefix.endswith("/"):
             prefix += "/"
         async for _ in self.list_prefix(prefix):
             return False
@@ -109,8 +110,12 @@ async def clear(self) -> None:
 
         Remove all keys and values from the store.
         """
-        async for key in self.list():
-            await self.delete(key)
+        if not self.supports_deletes:
+            raise NotImplementedError
+        if not self.supports_listing:
+            raise NotImplementedError
+        self._check_writable()
+        await self.delete_dir("")
 
     @property
     def read_only(self) -> bool:
@@ -319,7 +324,7 @@ async def delete_dir(self, prefix: str) -> None:
         if not self.supports_listing:
             raise NotImplementedError
         self._check_writable()
-        if prefix and not prefix.endswith("/"):
+        if prefix != "" and not prefix.endswith("/"):
             prefix += "/"
         async for key in self.list_prefix(prefix):
             await self.delete(key)

src/zarr/api/asynchronous.py

@@ -71,6 +71,9 @@
 
 
 def _infer_exists_ok(mode: AccessModeLiteral) -> bool:
+    """
+    Check that an ``AccessModeLiteral`` is compatible with overwriting an existing Zarr node.
+    """
     return mode in _OVERWRITE_MODES
 
 
@@ -410,13 +413,14 @@ async def save_array(
         arr = np.array(arr)
     shape = arr.shape
     chunks = getattr(arr, "chunks", None)  # for array-likes with chunks attribute
+    exists_ok = kwargs.pop("exists_ok", None) or _infer_exists_ok(mode)
     new = await AsyncArray.create(
         store_path,
         zarr_format=zarr_format,
         shape=shape,
         dtype=arr.dtype,
         chunks=chunks,
-        exists_ok=kwargs.pop("exists_ok", None) or _infer_exists_ok(mode),
+        exists_ok=exists_ok,
         **kwargs,
     )
     await new.setitem(slice(None), arr)
@@ -617,9 +621,10 @@ async def group(
     try:
         return await AsyncGroup.open(store=store_path, zarr_format=zarr_format)
     except (KeyError, FileNotFoundError):
+        _zarr_format = zarr_format or _default_zarr_version()
         return await AsyncGroup.from_store(
             store=store_path,
-            zarr_format=zarr_format or _default_zarr_version(),
+            zarr_format=_zarr_format,
             exists_ok=overwrite,
             attributes=attributes,
         )
@@ -726,10 +731,12 @@ async def open_group(
     except (KeyError, FileNotFoundError):
         pass
     if mode in _CREATE_MODES:
+        exists_ok = _infer_exists_ok(mode)
+        _zarr_format = zarr_format or _default_zarr_version()
         return await AsyncGroup.from_store(
             store_path,
-            zarr_format=zarr_format or _default_zarr_version(),
-            exists_ok=_infer_exists_ok(mode),
+            zarr_format=_zarr_format,
+            exists_ok=exists_ok,
             attributes=attributes,
         )
     raise FileNotFoundError(f"Unable to find group: {store_path}")
@@ -904,7 +911,7 @@ async def create(
         dtype=dtype,
         compressor=compressor,
         fill_value=fill_value,
-        exists_ok=overwrite,  # TODO: name change
+        exists_ok=overwrite,
         filters=filters,
         dimension_separator=dimension_separator,
         zarr_format=zarr_format,
@@ -1074,7 +1081,7 @@ async def open_array(
         If using an fsspec URL to create the store, these will be passed to
         the backend implementation. Ignored otherwise.
     **kwargs
-        Any keyword arguments to pass to the ``create``.
+        Any keyword arguments to pass to ``create``.
 
     Returns
     -------
@@ -1091,10 +1098,12 @@ async def open_array(
         return await AsyncArray.open(store_path, zarr_format=zarr_format)
     except FileNotFoundError:
         if not store_path.read_only:
+            exists_ok = _infer_exists_ok(mode)
+            _zarr_format = zarr_format or _default_zarr_version()
             return await create(
                 store=store_path,
-                zarr_format=zarr_format or _default_zarr_version(),
-                overwrite=_infer_exists_ok(mode),
+                zarr_format=_zarr_format,
+                overwrite=exists_ok,
                 **kwargs,
             )
         raise

src/zarr/testing/store.py

@@ -231,11 +231,13 @@ async def test_delete_dir(self, store: S) -> None:
         assert not await store.exists("foo/c/0")
 
     async def test_empty_dir(self, store: S) -> None:
-        assert await store.empty_dir()
+        assert await store.empty_dir("")
         await self.set(
             store, "foo/bar", self.buffer_cls.from_bytes(bytes("something", encoding="utf-8"))
         )
-        assert not await store.empty_dir()
+        assert not await store.empty_dir("")
+        assert await store.empty_dir("fo")
+        assert not await store.empty_dir("foo/")
         assert not await store.empty_dir("foo")
         assert await store.empty_dir("spam/")
 
@@ -244,7 +246,7 @@ async def test_clear(self, store: S) -> None:
             store, "key", self.buffer_cls.from_bytes(bytes("something", encoding="utf-8"))
         )
         await store.clear()
-        assert await store.empty_dir()
+        assert await store.empty_dir("")
 
     async def test_list(self, store: S) -> None:
         assert await _collect_aiterator(store.list()) == ()

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_dir()
+        assert await store.empty_dir("")
         (store.root / "foo/bar").mkdir(parents=True)
-        assert await store.empty_dir()
+        assert await store.empty_dir("")
 
     def test_creates_new_directory(self, tmp_path: pathlib.Path):
         target = tmp_path.joinpath("a", "b", "c")

tests/test_store/test_remote.py

@@ -213,4 +213,4 @@ async def test_empty_nonexistent_path(self, store_kwargs) -> None:
         # regression test for https://github.com/zarr-developers/zarr-python/pull/2343
         store_kwargs["path"] += "/abc"
         store = await self.store_cls.open(**store_kwargs)
-        assert await store.empty_dir()
+        assert await store.empty_dir("")