Skip to content

Commit

Permalink
respond to reviews
Browse files Browse the repository at this point in the history
  • Loading branch information
jhamman committed Nov 12, 2024
1 parent ebe8b67 commit f9dbd5b
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 22 deletions.
4 changes: 2 additions & 2 deletions docs/guide/storage.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand Down
17 changes: 11 additions & 6 deletions src/zarr/abc/store.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -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:
Expand Down Expand Up @@ -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)
Expand Down
25 changes: 17 additions & 8 deletions src/zarr/api/asynchronous.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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,
)
Expand Down Expand Up @@ -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}")
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
-------
Expand All @@ -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
Expand Down
8 changes: 5 additions & 3 deletions src/zarr/testing/store.py
Original file line number Diff line number Diff line change
Expand Up @@ -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/")

Expand All @@ -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()) == ()
Expand Down
4 changes: 2 additions & 2 deletions tests/test_store/test_local.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
2 changes: 1 addition & 1 deletion tests/test_store/test_remote.py
Original file line number Diff line number Diff line change
Expand Up @@ -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("")

0 comments on commit f9dbd5b

Please sign in to comment.