Error on invalid store mode (#3068)

* Error on invalid store mode

* Fix tests

* Add release note

* Fix test

Author: dstansby

changes/3068.bugfix.rst

@@ -0,0 +1 @@
+Trying to open an array with ``mode='r'`` when the store is not read-only now raises an error.

src/zarr/storage/_common.py

@@ -2,7 +2,7 @@
 
 import json
 from pathlib import Path
-from typing import TYPE_CHECKING, Any, Literal
+from typing import TYPE_CHECKING, Any, Literal, Self
 
 from zarr.abc.store import ByteRequest, Store
 from zarr.core.buffer import Buffer, default_buffer_prototype
@@ -48,9 +48,7 @@ def read_only(self) -> bool:
         return self.store.read_only
 
     @classmethod
-    async def open(
-        cls, store: Store, path: str, mode: AccessModeLiteral | None = None
-    ) -> StorePath:
+    async def open(cls, store: Store, path: str, mode: AccessModeLiteral | None = None) -> Self:
         """
         Open StorePath based on the provided mode.
 
@@ -67,6 +65,9 @@ async def open(
         ------
         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()
@@ -78,6 +79,8 @@ async def open(
 
         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}'")
 
         match mode:
             case "w-":

tests/test_api.py

@@ -171,7 +171,7 @@ def test_v2_and_v3_exist_at_same_path(store: Store) -> None:
     zarr.create_array(store, shape=(10,), dtype="uint8", zarr_format=2)
     msg = f"Both zarr.json (Zarr format 3) and .zarray (Zarr format 2) metadata objects exist at {store}. Zarr v3 will be used."
     with pytest.warns(UserWarning, match=re.escape(msg)):
-        zarr.open(store=store, mode="r")
+        zarr.open(store=store)
 
 
 @pytest.mark.parametrize("store", ["memory"], indirect=True)
@@ -1285,7 +1285,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):
+    with contextlib.suppress(FileExistsError, FileNotFoundError, ValueError):
         open_func(store=store, mode=mode)
     if mode == "w":
         assert not existing_fpath.exists()

tests/test_array.py

@@ -1473,7 +1473,7 @@ async def test_name(store: Store, zarr_format: ZarrFormat, path: str | None) ->
             for parent_path in parents:
                 # this will raise if these groups were not created
                 _ = await zarr.api.asynchronous.open_group(
-                    store=store, path=parent_path, mode="r", zarr_format=zarr_format
+                    store=store, path=parent_path, zarr_format=zarr_format
                 )
 
 
@@ -1661,7 +1661,7 @@ def test_roundtrip_numcodecs() -> None:
 
     BYTES_CODEC = {"name": "bytes", "configuration": {"endian": "little"}}
     # Read in the array again and check compressor config
-    root = zarr.open_group(store, mode="r")
+    root = zarr.open_group(store)
     metadata = root["test"].metadata.to_dict()
     expected = (*filters, BYTES_CODEC, *compressors)
     assert metadata["codecs"] == expected

tests/test_store/test_core.py

@@ -4,6 +4,7 @@
 import pytest
 from _pytest.compat import LEGACY_PATH
 
+import zarr
 from zarr import Group
 from zarr.core.common import AccessModeLiteral, ZarrFormat
 from zarr.storage import FsspecStore, LocalStore, MemoryStore, StoreLike, StorePath
@@ -251,3 +252,10 @@ def test_relativize_path_invalid() -> None:
     msg = f"The first component of {path} does not start with {prefix}."
     with pytest.raises(ValueError, match=msg):
         _relativize_path(path="a/b/c", prefix="b")
+
+
+def test_invalid_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'"):
+        zarr.open_array(store=store, path="a", zarr_format=2, mode="r")