Create read only copy if needed when opening a store path

Author: maxrjones

src/zarr/storage/_common.py

@@ -2,8 +2,9 @@
 
 import importlib.util
 import json
+import warnings
 from pathlib import Path
-from typing import TYPE_CHECKING, Any, Literal, Self, TypeAlias
+from typing import TYPE_CHECKING, Any, Literal, Self, TypeAlias, get_args
 
 from zarr.abc.store import ByteRequest, Store
 from zarr.core.buffer import Buffer, default_buffer_prototype
@@ -54,59 +55,88 @@ def __init__(self, store: Store, path: str = "") -> None:
     def read_only(self) -> bool:
         return self.store.read_only
 
+    @classmethod
+    async def _create_open_instance(cls, store: Store, path: str) -> Self:
+        """Helper to create and return a StorePath instance."""
+        await store._ensure_open()
+        return cls(store, path)
+
     @classmethod
     async def open(cls, store: Store, path: str, mode: AccessModeLiteral | None = None) -> Self:
         """
         Open StorePath based on the provided mode.
 
-        * If the mode is 'w-' and the StorePath contains keys, raise a FileExistsError.
-        * If the mode is 'w', delete all keys nested within the StorePath
-        * If the mode is 'a', 'r', or 'r+', do nothing
+        * If the mode is None, return an opened version of the store with no changes.
+        * If the mode is 'r+', 'w-', 'w', or 'a' and the store is read-only, raise a ValueError.
+        * If the mode is 'r' and the store is not read-only, return a copy of the store with read_only set to True.
+        * If the mode is 'w-' and the store is not read-only and the StorePath contains keys, raise a FileExistsError.
+        * If the mode is 'w'  and the store is not read-only, delete all keys nested within the StorePath.
 
         Parameters
         ----------
         mode : AccessModeLiteral
             The mode to use when initializing the store path.
 
+            The accepted values are:
+
+            - ``'r'``: read only (must exist)
+            - ``'r+'``: read/write (must exist)
+            - ``'a'``: read/write (create if doesn't exist)
+            - ``'w'``: read/write (overwrite if exists)
+            - ``'w-'``: read/write (create if doesn't exist).
+
         Raises
         ------
         FileExistsError
             If the mode is 'w-' and the store path already exists.
-        ValueError
-            If the mode is not "r" and the store is read-only, or
-            if the mode is "r" and the store is not read-only.
         """
 
-        await store._ensure_open()
-        self = cls(store, path)
-
         # fastpath if mode is None
         if mode is None:
-            return self
+            return await cls._create_open_instance(store, path)
 
-        if store.read_only and mode != "r":
-            raise ValueError(f"Store is read-only but mode is '{mode}'")
-        if not store.read_only and mode == "r":
-            raise ValueError(f"Store is not read-only but mode is '{mode}'")
+        if mode not in get_args(AccessModeLiteral):
+            raise ValueError(f"Invalid mode: {mode}, expected one of {AccessModeLiteral}")
 
+        if store.read_only:
+            # Don't allow write operations on a read-only store
+            if mode != "r":
+                raise ValueError(
+                    f"Store is read-only but mode is '{mode}'. Create a writable store or use 'r' mode."
+                )
+            self = await cls._create_open_instance(store, path)
+        elif mode == "r":
+            # Create read-only copy for read mode on writable store
+            try:
+                warnings.warn(
+                    "Store is not read-only but mode is 'r'. Creating a read-only copy. "
+                    "This behavior may change in the future with a more granular permissions model.",
+                    UserWarning,
+                    stacklevel=2,
+                )
+                self = await cls._create_open_instance(store.with_read_only(True), path)
+            except NotImplementedError as e:
+                raise ValueError(
+                    "Store is not read-only but mode is 'r'. Unable to create a read-only copy of the store."
+                ) from e
+        else:
+            # writable store and writable mode
+            await store._ensure_open()
+            self = await cls._create_open_instance(store, path)
+
+        # Handle mode-specific operations
         match mode:
             case "w-":
                 if not await self.is_empty():
-                    msg = (
-                        f"{self} is not empty, but `mode` is set to 'w-'."
-                        "Either remove the existing objects in storage,"
-                        "or set `mode` to a value that handles pre-existing objects"
-                        "in storage, like `a` or `w`."
+                    raise FileExistsError(
+                        f"Cannot create '{path}' with mode 'w-' because it already contains data. "
+                        f"Use mode 'w' to overwrite or 'a' to append."
                     )
-                    raise FileExistsError(msg)
             case "w":
                 await self.delete_dir()
             case "a" | "r" | "r+":
                 # No init action
                 pass
-            case _:
-                raise ValueError(f"Invalid mode: {mode}")
-
         return self
 
     async def get(

tests/test_api.py

@@ -1318,7 +1318,7 @@ def test_no_overwrite_open(tmp_path: Path, open_func: Callable, mode: str) -> No
     existing_fpath = add_empty_file(tmp_path)
 
     assert existing_fpath.exists()
-    with contextlib.suppress(FileExistsError, FileNotFoundError, ValueError):
+    with contextlib.suppress(FileExistsError, FileNotFoundError, UserWarning):
         open_func(store=store, mode=mode)
     if mode == "w":
         assert not existing_fpath.exists()

tests/test_store/test_core.py

@@ -263,8 +263,11 @@ def test_relativize_path_invalid() -> None:
         _relativize_path(path="a/b/c", prefix="b")
 
 
-def test_invalid_open_mode() -> None:
+def test_different_open_mode() -> None:
     store = MemoryStore()
     zarr.create((100,), store=store, zarr_format=2, path="a")
-    with pytest.raises(ValueError, match="Store is not read-only but mode is 'r'"):
+    with pytest.warns(
+        UserWarning,
+        match="Store is not read-only but mode is 'r'. Attempting to create a read-only copy. This behavior may change in the future with a more granular permissions model.",
+    ):
         zarr.open_array(store=store, path="a", zarr_format=2, mode="r")