diff --git a/changes/3128.bugfix.rst b/changes/3128.bugfix.rst new file mode 100644 index 0000000000..b93416070e --- /dev/null +++ b/changes/3128.bugfix.rst @@ -0,0 +1 @@ +Fix `zarr.open` default for argument `mode` when `store` is `read_only` \ No newline at end of file diff --git a/src/zarr/api/asynchronous.py b/src/zarr/api/asynchronous.py index 3ef9903c57..54bddd80a8 100644 --- a/src/zarr/api/asynchronous.py +++ b/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! diff --git a/src/zarr/api/synchronous.py b/src/zarr/api/synchronous.py index 6aa1cc4de7..a7f7cfda35 100644 --- a/src/zarr/api/synchronous.py +++ b/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 diff --git a/tests/test_api.py b/tests/test_api.py index 5d41a37493..2a95d7b97c 100644 --- a/tests/test_api.py +++ b/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()