fix file modes (#2000)

* fix file modes

* change store mode from literal to class of properties

* raise FileNotFoundError instead of KeyError

* rename OpenMode to AccessMode

* rename AccessMode parameters

* enforce AccessMode for MemoryStore and RemoteStore

* fix RemoteStore

* fix RemoteStore

* formatting

* fix RemoteStore._exists

* Revert "fix RemoteStore._exists"

This reverts commit 5f876d2.

* create async Store.open()

* make Store.open() classmethod

* async clear and root_exists in Store

* fix test_remote.py:test_basic

* fix RemoteStore.open

* remove unnecessary import zarr in tests

* rename root_exists to (not) empty, test and fix store.empty, store.clear

* mypy

* incorporate feedback on store._open()

Co-authored-by: Joe Hamman <[email protected]>

* rename store.ensure_open to store._ensure_open

---------

Co-authored-by: Joe Hamman <[email protected]>

Author: brokkoli71

src/zarr/abc/store.py

@@ -1,31 +1,74 @@
 from abc import ABC, abstractmethod
 from collections.abc import AsyncGenerator
-from typing import Protocol, runtime_checkable
+from typing import Any, NamedTuple, Protocol, runtime_checkable
+
+from typing_extensions import Self
 
 from zarr.buffer import Buffer, BufferPrototype
-from zarr.common import BytesLike, OpenMode
+from zarr.common import AccessModeLiteral, BytesLike
+
+
+class AccessMode(NamedTuple):
+    readonly: bool
+    overwrite: bool
+    create: bool
+    update: bool
+
+    @classmethod
+    def from_literal(cls, mode: AccessModeLiteral) -> Self:
+        if mode in ("r", "r+", "a", "w", "w-"):
+            return cls(
+                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):
-    _mode: OpenMode
+    _mode: AccessMode
+    _is_open: bool
+
+    def __init__(self, mode: AccessModeLiteral = "r", *args: Any, **kwargs: Any):
+        self._is_open = False
+        self._mode = AccessMode.from_literal(mode)
+
+    @classmethod
+    async def open(cls, *args: Any, **kwargs: Any) -> Self:
+        store = cls(*args, **kwargs)
+        await store._open()
+        return store
+
+    async def _open(self) -> None:
+        if self._is_open:
+            raise ValueError("store is already open")
+        if not await self.empty():
+            if self.mode.update or self.mode.readonly:
+                pass
+            elif self.mode.overwrite:
+                await self.clear()
+            else:
+                raise FileExistsError("Store already exists")
+        self._is_open = True
+
+    async def _ensure_open(self) -> None:
+        if not self._is_open:
+            await self._open()
 
-    def __init__(self, mode: OpenMode = "r"):
-        if mode not in ("r", "r+", "w", "w-", "a"):
-            raise ValueError("mode must be one of 'r', 'r+', 'w', 'w-', 'a'")
-        self._mode = mode
+    @abstractmethod
+    async def empty(self) -> bool: ...
+
+    @abstractmethod
+    async def clear(self) -> None: ...
 
     @property
-    def mode(self) -> OpenMode:
+    def mode(self) -> AccessMode:
         """Access mode of the store."""
         return self._mode
 
-    @property
-    def writeable(self) -> bool:
-        """Is the store writeable?"""
-        return self.mode in ("a", "w", "w-")
-
     def _check_writable(self) -> None:
-        if not self.writeable:
+        if self.mode.readonly:
             raise ValueError("store mode does not support writing")
 
     @abstractmethod
@@ -173,8 +216,9 @@ def list_dir(self, prefix: str) -> AsyncGenerator[str, None]:
         """
         ...
 
-    def close(self) -> None:  # noqa: B027
+    def close(self) -> None:
         """Close the store."""
+        self._is_open = False
         pass
 
 

src/zarr/api/asynchronous.py

@@ -12,7 +12,7 @@
 from zarr.array import Array, AsyncArray
 from zarr.buffer import NDArrayLike
 from zarr.chunk_key_encodings import ChunkKeyEncoding
-from zarr.common import JSON, ChunkCoords, MemoryOrder, OpenMode, ZarrFormat
+from zarr.common import JSON, AccessModeLiteral, ChunkCoords, MemoryOrder, ZarrFormat
 from zarr.group import AsyncGroup
 from zarr.metadata import ArrayV2Metadata, ArrayV3Metadata
 from zarr.store import (
@@ -158,7 +158,7 @@ async def load(
 async def open(
     *,
     store: StoreLike | None = None,
-    mode: OpenMode | None = None,  # type and value changed
+    mode: AccessModeLiteral | None = None,  # type and value changed
     zarr_version: ZarrFormat | None = None,  # deprecated
     zarr_format: ZarrFormat | None = None,
     path: str | None = None,
@@ -189,15 +189,15 @@ async def open(
         Return type depends on what exists in the given store.
     """
     zarr_format = _handle_zarr_version_or_format(zarr_version=zarr_version, zarr_format=zarr_format)
-    store_path = make_store_path(store, mode=mode)
+    store_path = await make_store_path(store, mode=mode)
 
     if path is not None:
         store_path = store_path / path
 
     try:
-        return await open_array(store=store_path, zarr_format=zarr_format, **kwargs)
+        return await open_array(store=store_path, zarr_format=zarr_format, mode=mode, **kwargs)
     except KeyError:
-        return await open_group(store=store_path, zarr_format=zarr_format, **kwargs)
+        return await open_group(store=store_path, zarr_format=zarr_format, mode=mode, **kwargs)
 
 
 async def open_consolidated(*args: Any, **kwargs: Any) -> AsyncGroup:
@@ -267,7 +267,7 @@ async def save_array(
         or _default_zarr_version()
     )
 
-    store_path = make_store_path(store, mode="w")
+    store_path = await make_store_path(store, mode="w")
     if path is not None:
         store_path = store_path / path
     new = await AsyncArray.create(
@@ -421,7 +421,7 @@ async def group(
         or _default_zarr_version()
     )
 
-    store_path = make_store_path(store)
+    store_path = await make_store_path(store)
     if path is not None:
         store_path = store_path / path
 
@@ -451,7 +451,7 @@ async def group(
 async def open_group(
     *,  # Note: this is a change from v2
     store: StoreLike | None = None,
-    mode: OpenMode | None = None,  # not used
+    mode: AccessModeLiteral | None = None,  # not used
     cache_attrs: bool | None = None,  # not used, default changed
     synchronizer: Any = None,  # not used
     path: str | None = None,
@@ -512,7 +512,7 @@ async def open_group(
     if storage_options is not None:
         warnings.warn("storage_options is not yet implemented", RuntimeWarning, stacklevel=2)
 
-    store_path = make_store_path(store, mode=mode)
+    store_path = await make_store_path(store, mode=mode)
     if path is not None:
         store_path = store_path / path
 
@@ -682,8 +682,8 @@ async def create(
     if meta_array is not None:
         warnings.warn("meta_array is not yet implemented", RuntimeWarning, stacklevel=2)
 
-    mode = cast(OpenMode, "r" if read_only else "w")
-    store_path = make_store_path(store, mode=mode)
+    mode = kwargs.pop("mode", cast(AccessModeLiteral, "r" if read_only else "w"))
+    store_path = await make_store_path(store, mode=mode)
     if path is not None:
         store_path = store_path / path
 
@@ -854,22 +854,24 @@ async def open_array(
         The opened array.
     """
 
-    store_path = make_store_path(store)
+    store_path = await make_store_path(store)
     if path is not None:
         store_path = store_path / path
 
     zarr_format = _handle_zarr_version_or_format(zarr_version=zarr_version, zarr_format=zarr_format)
 
     try:
         return await AsyncArray.open(store_path, zarr_format=zarr_format)
-    except KeyError as e:
-        if store_path.store.writeable:
-            pass
-        else:
-            raise e
-
-    # if array was not found, create it
-    return await create(store=store, path=path, zarr_format=zarr_format, **kwargs)
+    except FileNotFoundError as e:
+        if store_path.store.mode.create:
+            return await create(
+                store=store_path,
+                path=path,
+                zarr_format=zarr_format,
+                overwrite=store_path.store.mode.overwrite,
+                **kwargs,
+            )
+        raise e
 
 
 async def open_like(a: ArrayLike, path: str, **kwargs: Any) -> AsyncArray:

src/zarr/api/synchronous.py

@@ -5,7 +5,7 @@
 import zarr.api.asynchronous as async_api
 from zarr.array import Array, AsyncArray
 from zarr.buffer import NDArrayLike
-from zarr.common import JSON, ChunkCoords, OpenMode, ZarrFormat
+from zarr.common import JSON, AccessModeLiteral, ChunkCoords, ZarrFormat
 from zarr.group import Group
 from zarr.store import StoreLike
 from zarr.sync import sync
@@ -36,7 +36,7 @@ def load(
 def open(
     *,
     store: StoreLike | None = None,
-    mode: OpenMode | None = None,  # type and value changed
+    mode: AccessModeLiteral | None = None,  # type and value changed
     zarr_version: ZarrFormat | None = None,  # deprecated
     zarr_format: ZarrFormat | None = None,
     path: str | None = None,
@@ -161,7 +161,7 @@ def group(
 def open_group(
     *,  # Note: this is a change from v2
     store: StoreLike | None = None,
-    mode: OpenMode | None = None,  # not used in async api
+    mode: AccessModeLiteral | None = None,  # not used in async api
     cache_attrs: bool | None = None,  # default changed, not used in async api
     synchronizer: Any = None,  # not used in async api
     path: str | None = None,

src/zarr/array.py

@@ -142,7 +142,7 @@ async def create(
         exists_ok: bool = False,
         data: npt.ArrayLike | None = None,
     ) -> AsyncArray:
-        store_path = make_store_path(store)
+        store_path = await make_store_path(store)
 
         if chunk_shape is None:
             if chunks is None:
@@ -334,18 +334,18 @@ async def open(
         store: StoreLike,
         zarr_format: ZarrFormat | None = 3,
     ) -> AsyncArray:
-        store_path = make_store_path(store)
+        store_path = await make_store_path(store)
 
         if zarr_format == 2:
             zarray_bytes, zattrs_bytes = await gather(
                 (store_path / ZARRAY_JSON).get(), (store_path / ZATTRS_JSON).get()
             )
             if zarray_bytes is None:
-                raise KeyError(store_path)  # filenotfounderror?
+                raise FileNotFoundError(store_path)
         elif zarr_format == 3:
             zarr_json_bytes = await (store_path / ZARR_JSON).get()
             if zarr_json_bytes is None:
-                raise KeyError(store_path)  # filenotfounderror?
+                raise FileNotFoundError(store_path)
         elif zarr_format is None:
             zarr_json_bytes, zarray_bytes, zattrs_bytes = await gather(
                 (store_path / ZARR_JSON).get(),
@@ -357,7 +357,7 @@ async def open(
                 # alternatively, we could warn and favor v3
                 raise ValueError("Both zarr.json and .zarray objects exist")
             if zarr_json_bytes is None and zarray_bytes is None:
-                raise KeyError(store_path)  # filenotfounderror?
+                raise FileNotFoundError(store_path)
             # set zarr_format based on which keys were found
             if zarr_json_bytes is not None:
                 zarr_format = 3
@@ -412,7 +412,7 @@ def attrs(self) -> dict[str, JSON]:
 
     @property
     def read_only(self) -> bool:
-        return bool(not self.store_path.store.writeable)
+        return self.store_path.store.mode.readonly
 
     @property
     def path(self) -> str:

src/zarr/common.py

@@ -33,7 +33,7 @@
 ZarrFormat = Literal[2, 3]
 JSON = None | str | int | float | Enum | dict[str, "JSON"] | list["JSON"] | tuple["JSON", ...]
 MemoryOrder = Literal["C", "F"]
-OpenMode = Literal["r", "r+", "a", "w", "w-"]
+AccessModeLiteral = Literal["r", "r+", "a", "w", "w-"]
 
 
 def product(tup: ChunkCoords) -> int:

src/zarr/group.py

@@ -129,7 +129,7 @@ async def create(
         exists_ok: bool = False,
         zarr_format: ZarrFormat = 3,
     ) -> AsyncGroup:
-        store_path = make_store_path(store)
+        store_path = await make_store_path(store)
         if not exists_ok:
             await ensure_no_existing_node(store_path, zarr_format=zarr_format)
         attributes = attributes or {}
@@ -146,7 +146,7 @@ async def open(
         store: StoreLike,
         zarr_format: Literal[2, 3, None] = 3,
     ) -> AsyncGroup:
-        store_path = make_store_path(store)
+        store_path = await make_store_path(store)
 
         if zarr_format == 2:
             zgroup_bytes, zattrs_bytes = await asyncio.gather(
@@ -169,7 +169,7 @@ async def open(
                 # alternatively, we could warn and favor v3
                 raise ValueError("Both zarr.json and .zgroup objects exist")
             if zarr_json_bytes is None and zgroup_bytes is None:
-                raise KeyError(store_path)  # filenotfounderror?
+                raise FileNotFoundError(store_path)
             # set zarr_format based on which keys were found
             if zarr_json_bytes is not None:
                 zarr_format = 3

src/zarr/store/core.py

@@ -4,9 +4,9 @@
 from pathlib import Path
 from typing import Any, Literal
 
-from zarr.abc.store import Store
+from zarr.abc.store import AccessMode, Store
 from zarr.buffer import Buffer, BufferPrototype, default_buffer_prototype
-from zarr.common import ZARR_JSON, ZARRAY_JSON, ZGROUP_JSON, OpenMode, ZarrFormat
+from zarr.common import ZARR_JSON, ZARRAY_JSON, ZGROUP_JSON, AccessModeLiteral, ZarrFormat
 from zarr.errors import ContainsArrayAndGroupError, ContainsArrayError, ContainsGroupError
 from zarr.store.local import LocalStore
 from zarr.store.memory import MemoryStore
@@ -68,23 +68,26 @@ def __eq__(self, other: Any) -> bool:
 StoreLike = Store | StorePath | Path | str
 
 
-def make_store_path(store_like: StoreLike | None, *, mode: OpenMode | None = None) -> StorePath:
+async def make_store_path(
+    store_like: StoreLike | None, *, mode: AccessModeLiteral | None = None
+) -> StorePath:
     if isinstance(store_like, StorePath):
         if mode is not None:
-            assert mode == store_like.store.mode
+            assert AccessMode.from_literal(mode) == store_like.store.mode
         return store_like
     elif isinstance(store_like, Store):
         if mode is not None:
-            assert mode == store_like.mode
+            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:
             mode = "w"  # exception to the default mode = 'r'
-        return StorePath(MemoryStore(mode=mode))
+        return StorePath(await MemoryStore.open(mode=mode))
     elif isinstance(store_like, Path):
-        return StorePath(LocalStore(store_like, mode=mode or "r"))
+        return StorePath(await LocalStore.open(root=store_like, mode=mode or "r"))
     elif isinstance(store_like, str):
-        return StorePath(LocalStore(Path(store_like), mode=mode or "r"))
+        return StorePath(await LocalStore.open(root=Path(store_like), mode=mode or "r"))
     raise TypeError