Fix zarr.open default for argument mode when store is read_only (#3128)

* fix `zarr.open` default for argument `mode` when store is read_only

* document changes and format

* assert type in test_load_zip and test_load_local

* default mode=None for `zarr.synchronous.open`

* update docstring

* also check for read_only if store is a StorePath

Author: brokkoli71

changes/3128.bugfix.rst

@@ -0,0 +1 @@
+Fix `zarr.open` default for argument `mode` when `store` is `read_only`

src/zarr/api/asynchronous.py

@@ -9,6 +9,7 @@
 import numpy.typing as npt
 from typing_extensions import deprecated
 
+from zarr.abc.store import Store
 from zarr.core.array import (
     Array,
     AsyncArray,
@@ -40,6 +41,7 @@
 from zarr.core.metadata import ArrayMetadataDict, ArrayV2Metadata, ArrayV3Metadata
 from zarr.core.metadata.v2 import _default_compressor, _default_filters
 from zarr.errors import GroupNotFoundError, NodeTypeValidationError
+from zarr.storage import StorePath
 from zarr.storage._common import make_store_path
 
 if TYPE_CHECKING:
@@ -298,7 +300,7 @@ async def load(
 async def open(
     *,
     store: StoreLike | None = None,
-    mode: AccessModeLiteral = "a",
+    mode: AccessModeLiteral | None = None,
     zarr_version: ZarrFormat | None = None,  # deprecated
     zarr_format: ZarrFormat | None = None,
     path: str | None = None,
@@ -316,6 +318,7 @@ async def open(
         read/write (must exist); 'a' means read/write (create if doesn't
         exist); 'w' means create (overwrite if exists); 'w-' means create
         (fail if exists).
+        If the store is read-only, the default is 'r'; otherwise, it is 'a'.
     zarr_format : {2, 3, None}, optional
         The zarr format to use when saving.
     path : str or None, optional
@@ -333,7 +336,11 @@ 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)
-
+    if mode is None:
+        if isinstance(store, (Store, StorePath)) and store.read_only:
+            mode = "r"
+        else:
+            mode = "a"
     store_path = await make_store_path(store, mode=mode, path=path, storage_options=storage_options)
 
     # TODO: the mode check below seems wrong!

src/zarr/api/synchronous.py

@@ -162,7 +162,7 @@ def load(
 def open(
     store: StoreLike | None = None,
     *,
-    mode: AccessModeLiteral = "a",
+    mode: AccessModeLiteral | None = None,
     zarr_version: ZarrFormat | None = None,  # deprecated
     zarr_format: ZarrFormat | None = None,
     path: str | None = None,
@@ -180,6 +180,7 @@ def open(
         read/write (must exist); 'a' means read/write (create if doesn't
         exist); 'w' means create (overwrite if exists); 'w-' means create
         (fail if exists).
+        If the store is read-only, the default is 'r'; otherwise, it is 'a'.
     zarr_format : {2, 3, None}, optional
         The zarr format to use when saving.
     path : str or None, optional

tests/test_api.py

@@ -39,7 +39,7 @@
 )
 from zarr.core.buffer import NDArrayLike
 from zarr.errors import MetadataValidationError
-from zarr.storage import MemoryStore
+from zarr.storage import LocalStore, MemoryStore, ZipStore
 from zarr.storage._utils import normalize_path
 from zarr.testing.utils import gpu_test
 
@@ -401,6 +401,38 @@ def test_load_array(sync_store: Store) -> None:
             assert_array_equal(bar, array)
 
 
+@pytest.mark.parametrize("path", ["data", None])
+@pytest.mark.parametrize("load_read_only", [True, False, None])
+def test_load_zip(tmp_path: pathlib.Path, path: str | None, load_read_only: bool | None) -> None:
+    file = tmp_path / "test.zip"
+    data = np.arange(100).reshape(10, 10)
+
+    with ZipStore(file, mode="w", read_only=False) as zs:
+        save(zs, data, path=path)
+    with ZipStore(file, mode="r", read_only=load_read_only) as zs:
+        result = zarr.load(store=zs, path=path)
+        assert isinstance(result, np.ndarray)
+        assert np.array_equal(result, data)
+    with ZipStore(file, read_only=load_read_only) as zs:
+        result = zarr.load(store=zs, path=path)
+        assert isinstance(result, np.ndarray)
+        assert np.array_equal(result, data)
+
+
+@pytest.mark.parametrize("path", ["data", None])
+@pytest.mark.parametrize("load_read_only", [True, False])
+def test_load_local(tmp_path: pathlib.Path, path: str | None, load_read_only: bool) -> None:
+    file = tmp_path / "test.zip"
+    data = np.arange(100).reshape(10, 10)
+
+    with LocalStore(file, read_only=False) as zs:
+        save(zs, data, path=path)
+    with LocalStore(file, read_only=load_read_only) as zs:
+        result = zarr.load(store=zs, path=path)
+        assert isinstance(result, np.ndarray)
+        assert np.array_equal(result, data)
+
+
 def test_tree() -> None:
     pytest.importorskip("rich")
     g1 = zarr.group()