add _as_immutable and _as_mutable methods to stores; use those methods in storepath creation; get tests to pass

Author: d-v-b

src/zarr/abc/store.py

@@ -412,6 +412,18 @@ async def getsize_prefix(self, prefix: str) -> int:
         sizes = await concurrent_map(keys, self.getsize, limit=limit)
         return sum(sizes)
 
+    def _as_immutable(self: Self) -> Self:
+        """
+        Return a mutable copy of the store.
+        """
+        raise NotImplementedError
+
+    def _as_mutable(self: Self) -> Self:
+        """
+        Return an immutable (read-only) copy of the store.
+        """
+        raise NotImplementedError
+
 
 @runtime_checkable
 class ByteGetter(Protocol):

src/zarr/storage/common.py

@@ -12,8 +12,6 @@
 from zarr.storage.local import LocalStore
 from zarr.storage.memory import MemoryStore
 
-# from zarr.store.remote import RemoteStore
-
 if TYPE_CHECKING:
     from zarr.core.buffer import BufferPrototype
 
@@ -72,6 +70,8 @@ async def open(
         """
 
         await store._ensure_open()
+        if mode == "r" and not store.read_only:
+            store = store._as_immutable()
         self = cls(store, path)
 
         # fastpath if mode is None

src/zarr/storage/local.py

@@ -5,7 +5,7 @@
 import os
 import shutil
 from pathlib import Path
-from typing import TYPE_CHECKING
+from typing import TYPE_CHECKING, Self
 
 from zarr.abc.store import ByteRangeRequest, Store
 from zarr.core.buffer import Buffer
@@ -229,3 +229,9 @@ async def list_dir(self, prefix: str) -> AsyncIterator[str]:
 
     async def getsize(self, key: str) -> int:
         return os.path.getsize(self.root / key)
+
+    def _as_immutable(self: Self) -> Self:
+        return type(self)(self.root, read_only=True)
+
+    def _as_mutable(self: Self) -> Self:
+        return type(self)(self.root, read_only=False)

src/zarr/storage/logging.py

@@ -5,7 +5,7 @@
 import time
 from collections import defaultdict
 from contextlib import contextmanager
-from typing import TYPE_CHECKING, Any
+from typing import TYPE_CHECKING, Any, Self
 
 from zarr.abc.store import ByteRangeRequest, Store
 
@@ -233,3 +233,15 @@ async def getsize(self, key: str) -> int:
     async def getsize_prefix(self, prefix: str) -> int:
         with self.log(prefix):
             return await self._store.getsize_prefix(prefix)
+
+    def _as_immutable(self: Self) -> Self:
+        return type(self)(
+            store=self._store._as_immutable(),
+            log_level=self.log_level,
+            log_handler=self.log_handler,
+        )
+
+    def _as_mutable(self: Self) -> Self:
+        return type(self)(
+            store=self._store._as_mutable(), log_level=self.log_level, log_handler=self.log_handler
+        )

src/zarr/storage/memory.py

@@ -169,6 +169,12 @@ async def list_dir(self, prefix: str) -> AsyncIterator[str]:
         for key in keys_unique:
             yield key
 
+    def _as_immutable(self: Self) -> Self:
+        return type(self)(self._store_dict, read_only=True)
+
+    def _as_mutable(self: Self) -> Self:
+        return type(self)(self._store_dict, read_only=False)
+
 
 class GpuMemoryStore(MemoryStore):
     """A GPU only memory store that stores every chunk in GPU memory irrespective
@@ -236,3 +242,9 @@ async def set(self, key: str, value: Buffer, byte_range: tuple[int, int] | None
         # Convert to gpu.Buffer
         gpu_value = value if isinstance(value, gpu.Buffer) else gpu.Buffer.from_buffer(value)
         await super().set(key, gpu_value, byte_range=byte_range)
+
+    def _as_immutable(self: Self) -> Self:
+        return type(self)(self._store_dict, read_only=True)
+
+    def _as_mutable(self: Self) -> Self:
+        return type(self)(self._store_dict, read_only=False)

src/zarr/storage/remote.py

@@ -1,7 +1,7 @@
 from __future__ import annotations
 
 import warnings
-from typing import TYPE_CHECKING, Any
+from typing import TYPE_CHECKING, Any, Self
 
 from zarr.abc.store import ByteRangeRequest, Store
 from zarr.storage.common import _dereference_path
@@ -338,3 +338,13 @@ async def getsize(self, key: str) -> int:
         else:
             # fsspec doesn't have typing. We'll need to assume or verify this is true
             return int(size)
+
+    def _as_immutable(self: Self) -> Self:
+        return type(self)(
+            self.fs, read_only=True, path=self.path, allowed_exceptions=self.allowed_exceptions
+        )
+
+    def _as_mutable(self: Self) -> Self:
+        return type(self)(
+            self.fs, read_only=False, path=self.path, allowed_exceptions=self.allowed_exceptions
+        )

src/zarr/storage/zip.py

@@ -5,7 +5,7 @@
 import time
 import zipfile
 from pathlib import Path
-from typing import TYPE_CHECKING, Any, Literal
+from typing import TYPE_CHECKING, Any, Literal, Self
 
 from zarr.abc.store import ByteRangeRequest, Store
 from zarr.core.buffer import Buffer, BufferPrototype
@@ -269,3 +269,27 @@ async def list_dir(self, prefix: str) -> AsyncIterator[str]:
                     if k not in seen:
                         seen.add(k)
                         yield k
+
+    def _as_immutable(self: Self) -> Self:
+        self.close()
+        new_store = type(self)(
+            self.path,
+            read_only=True,
+            mode="r",
+            compression=self.compression,
+            allowZip64=self.allowZip64,
+        )
+        new_store._sync_open()
+        return new_store
+
+    def _as_mutable(self: Self) -> Self:
+        self.close()
+        new_store = type(self)(
+            self.path,
+            read_only=False,
+            mode="a",
+            compression=self.compression,
+            allowZip64=self.allowZip64,
+        )
+        new_store._sync_open()
+        return new_store

tests/conftest.py

@@ -36,38 +36,6 @@
 secure_bucket_name = "test-secure"
 
 
-def as_mutable(store: Store) -> Store:
-    """
-    Return a mutable version of the store
-    """
-    if isinstance(store, LocalStore):
-        return LocalStore(store.root, read_only=False)
-    if isinstance(store, MemoryStore):
-        return MemoryStore(store._store_dict, read_only=False)
-    if isinstance(store, RemoteStore):
-        return RemoteStore(fs=store.fs, path=store.path, read_only=False)
-    if isinstance(store, ZipStore):
-        store.close()
-        return sync(ZipStore.open(path=store.path, read_only=False))
-    raise ValueError(f"Unknown store type: {type(store)}")
-
-
-def as_immutable(store: Store) -> Store:
-    """
-    Return an immutable version of the store
-    """
-    if isinstance(store, LocalStore):
-        return LocalStore(store.root, read_only=True)
-    if isinstance(store, MemoryStore):
-        return MemoryStore(store._store_dict, read_only=True)
-    if isinstance(store, RemoteStore):
-        return RemoteStore(fs=store.fs, path=store.path, read_only=True)
-    if isinstance(store, ZipStore):
-        store.close()
-        return sync(ZipStore.open(path=store.path, read_only=True))
-    raise ValueError(f"Unknown store type: {type(store)}")
-
-
 async def parse_store(
     store: str,
     path: str,

tests/test_api.py

@@ -9,7 +9,6 @@
 import zarr
 import zarr.api.asynchronous
 import zarr.core.group
-from tests.conftest import as_immutable
 from zarr import Array, Group
 from zarr.abc.store import Store
 from zarr.api.synchronous import (
@@ -82,7 +81,7 @@ async def test_open_array(store: Store, zarr_format: ZarrFormat) -> None:
     assert isinstance(z, Array)
     assert z.shape == (200,)
 
-    store_r = as_immutable(store)
+    store_r = store._as_immutable()
     z = open(store=store_r, zarr_format=zarr_format, mode="r")
     assert isinstance(z, Array)
     assert z.shape == (200,)
@@ -115,7 +114,7 @@ async def test_open_group(store: Store) -> None:
     # g = open_group(store=store)
     # assert isinstance(g, Group)
     # assert "foo" not in g
-    store_r = as_immutable(store)
+    store_r = store._as_immutable()
 
     g = open_group(store=store_r, mode="r")
     assert isinstance(g, Group)
@@ -226,7 +225,7 @@ def test_save_errors() -> None:
 
 @pytest.mark.parametrize(
     "store",
-    ["local", "memory", "remote", pytest.param("zip", marks=pytest.mark.xfail)],
+    ["local", "memory", "remote", "zip"],
     indirect=True,
 )
 def test_open_store_with_mode_r(store: Store) -> None: