Refactor: store mode and initialization (zarr-developers#2442)

* fix(array): thread order parameter through to array __init__

* type fixes

* move defaults

* apply MemoryOrder type to ArrayV2Metadata

* more more

* more more more

* feature(store,group,array): stores learn to delete prefixes when
overwriting nodes

- add Store.delete_dir and Store.delete_prefix
- update array and group creation methods to call delete_dir
- change list_prefix to return absolue keys

* fixup

* fixup

* respond to review

* fixup

* fixup

* fix(store): refactor store initialization to include only r or w permissions

* Apply suggestions from code review

Co-authored-by: Davis Bennett <[email protected]>

* store -> readonly

* fixups

* fixups

* fixups

* fixup

* readonly -> read_only

* fixup

* Fix hypothesis

* fixup for windows

* respond to reviews

* is_empty

---------

Co-authored-by: Davis Bennett <[email protected]>
Co-authored-by: Deepak Cherian <[email protected]>

Author: 3 people

docs/guide/storage.rst

@@ -2,7 +2,7 @@ Storage
 =======
 
 Zarr-Python supports multiple storage backends, including: local file systems,
-Zip files, remote stores via ``fspec`` (S3, HTTP, etc.), and in-memory stores. In
+Zip files, remote stores via ``fsspec`` (S3, HTTP, etc.), and in-memory stores. In
 Zarr-Python 3, stores must implement the abstract store API from
 :class:`zarr.abc.store.Store`. 
 
@@ -19,9 +19,9 @@ to Zarr's top level API will result in the store being created automatically.
 .. code-block:: python
 
    >>> import zarr
-   >>> zarr.open("data/foo/bar", mode="r")  # implicitly creates a LocalStore
+   >>> zarr.open("data/foo/bar", mode="r")  # implicitly creates a read-only LocalStore
    <Group file://data/foo/bar>
-   >>> zarr.open("s3://foo/bar", mode="r")  # implicitly creates a RemoteStore
+   >>> zarr.open("s3://foo/bar", mode="r")  # implicitly creates a read-only RemoteStore
    <Group s3://foo/bar>
    >>> data = {}
    >>> zarr.open(data, mode="w")  # implicitly creates a MemoryStore
@@ -43,7 +43,7 @@ filesystem.
 .. code-block:: python
 
    >>> import zarr
-   >>> store = zarr.storage.LocalStore("data/foo/bar", mode="r")
+   >>> 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", mode="r")
+   >>> 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>
 
@@ -86,7 +86,7 @@ Zarr data (metadata and chunks) to a dictionary.
 
    >>> import zarr
    >>> data = {}
-   >>> store = zarr.storage.MemoryStore(data, mode="w")
+   >>> store = zarr.storage.MemoryStore(data)
    >>> zarr.open(store=store, shape=(2, ))
    <Array memory://4943638848 shape=(2,) dtype=float64>
 

src/zarr/abc/store.py

@@ -3,72 +3,32 @@
 from abc import ABC, abstractmethod
 from asyncio import gather
 from itertools import starmap
-from typing import TYPE_CHECKING, NamedTuple, Protocol, runtime_checkable
+from typing import TYPE_CHECKING, Protocol, runtime_checkable
 
 if TYPE_CHECKING:
     from collections.abc import AsyncGenerator, AsyncIterator, Iterable
     from types import TracebackType
     from typing import Any, Self, TypeAlias
 
     from zarr.core.buffer import Buffer, BufferPrototype
-    from zarr.core.common import AccessModeLiteral, BytesLike
+    from zarr.core.common import BytesLike
 
-__all__ = ["AccessMode", "ByteGetter", "ByteSetter", "Store", "set_or_delete"]
+__all__ = ["ByteGetter", "ByteSetter", "Store", "set_or_delete"]
 
 ByteRangeRequest: TypeAlias = tuple[int | None, int | None]
 
 
-class AccessMode(NamedTuple):
-    """Access mode flags."""
-
-    str: AccessModeLiteral
-    readonly: bool
-    overwrite: bool
-    create: bool
-    update: bool
-
-    @classmethod
-    def from_literal(cls, mode: AccessModeLiteral) -> Self:
-        """
-        Create an AccessMode instance from a literal.
-
-        Parameters
-        ----------
-        mode : AccessModeLiteral
-            One of 'r', 'r+', 'w', 'w-', 'a'.
-
-        Returns
-        -------
-        AccessMode
-            The created instance.
-
-        Raises
-        ------
-        ValueError
-            If mode is not one of 'r', 'r+', 'w', 'w-', 'a'.
-        """
-        if mode in ("r", "r+", "a", "w", "w-"):
-            return cls(
-                str=mode,
-                readonly=mode == "r",
-                overwrite=mode == "w",
-                create=mode in ("a", "w", "w-"),
-                update=mode in ("r+", "a"),
-            )
-        raise ValueError("mode must be one of 'r', 'r+', 'w', 'w-', 'a'")
-
-
 class Store(ABC):
     """
     Abstract base class for Zarr stores.
     """
 
-    _mode: AccessMode
+    _read_only: bool
     _is_open: bool
 
-    def __init__(self, *args: Any, mode: AccessModeLiteral = "r", **kwargs: Any) -> None:
+    def __init__(self, *, read_only: bool = False) -> None:
         self._is_open = False
-        self._mode = AccessMode.from_literal(mode)
+        self._read_only = read_only
 
     @classmethod
     async def open(cls, *args: Any, **kwargs: Any) -> Self:
@@ -112,81 +72,60 @@ async def _open(self) -> None:
         ------
         ValueError
             If the store is already open.
-        FileExistsError
-            If ``mode='w-'`` and the store already exists.
-
-        Notes
-        -----
-        * When ``mode='w'`` and the store already exists, it will be cleared.
         """
         if self._is_open:
             raise ValueError("store is already open")
-        if self.mode.str == "w":
-            await self.clear()
-        elif self.mode.str == "w-" and not await self.empty():
-            raise FileExistsError("Store already exists")
         self._is_open = True
 
     async def _ensure_open(self) -> None:
         """Open the store if it is not already open."""
         if not self._is_open:
             await self._open()
 
-    @abstractmethod
-    async def empty(self) -> bool:
+    async def is_empty(self, prefix: str) -> bool:
         """
-        Check if the store is empty.
+        Check if the directory is empty.
+
+        Parameters
+        ----------
+        prefix : str
+            Prefix of keys to check.
 
         Returns
         -------
         bool
             True if the store is empty, False otherwise.
         """
-        ...
+        if not self.supports_listing:
+            raise NotImplementedError
+        if prefix != "" and not prefix.endswith("/"):
+            prefix += "/"
+        async for _ in self.list_prefix(prefix):
+            return False
+        return True
 
-    @abstractmethod
     async def clear(self) -> None:
         """
         Clear the store.
 
         Remove all keys and values from the store.
         """
-        ...
-
-    @abstractmethod
-    def with_mode(self, mode: AccessModeLiteral) -> Self:
-        """
-        Return a new store of the same type pointing to the same location with a new mode.
-
-        The returned Store is not automatically opened. Call :meth:`Store.open` before
-        using.
-
-        Parameters
-        ----------
-        mode : AccessModeLiteral
-            The new mode to use.
-
-        Returns
-        -------
-        store
-            A new store of the same type with the new mode.
-
-        Examples
-        --------
-        >>> writer = zarr.store.MemoryStore(mode="w")
-        >>> reader = writer.with_mode("r")
-        """
-        ...
+        if not self.supports_deletes:
+            raise NotImplementedError
+        if not self.supports_listing:
+            raise NotImplementedError
+        self._check_writable()
+        await self.delete_dir("")
 
     @property
-    def mode(self) -> AccessMode:
-        """Access mode of the store."""
-        return self._mode
+    def read_only(self) -> bool:
+        """Is the store read-only?"""
+        return self._read_only
 
     def _check_writable(self) -> None:
         """Raise an exception if the store is not writable."""
-        if self.mode.readonly:
-            raise ValueError("store mode does not support writing")
+        if self.read_only:
+            raise ValueError("store was opened in read-only mode and does not support writing")
 
     @abstractmethod
     def __eq__(self, value: object) -> bool:
@@ -385,7 +324,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)