Skip to content

Create read only copy if needed when opening a store path #3156

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 17 commits into from
Jun 24, 2025
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/zarr/core/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from typing import (
TYPE_CHECKING,
Any,
Final,
Generic,
Literal,
TypedDict,
Expand Down Expand Up @@ -39,6 +40,7 @@
JSON = str | int | float | Mapping[str, "JSON"] | Sequence["JSON"] | None
MemoryOrder = Literal["C", "F"]
AccessModeLiteral = Literal["r", "r+", "a", "w", "w-"]
ANY_ACCESS_MODE: Final = "r", "r+", "a", "w", "w-"
DimensionNames = Iterable[str | None] | None

TName = TypeVar("TName", bound=str)
Expand Down
83 changes: 61 additions & 22 deletions src/zarr/storage/_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,20 @@

import importlib.util
import json
import warnings
from pathlib import Path
from typing import TYPE_CHECKING, Any, Literal, Self, TypeAlias

from zarr.abc.store import ByteRequest, Store
from zarr.core.buffer import Buffer, default_buffer_prototype
from zarr.core.common import ZARR_JSON, ZARRAY_JSON, ZGROUP_JSON, AccessModeLiteral, ZarrFormat
from zarr.core.common import (
ANY_ACCESS_MODE,
ZARR_JSON,
ZARRAY_JSON,
ZGROUP_JSON,
AccessModeLiteral,
ZarrFormat,
)
from zarr.errors import ContainsArrayAndGroupError, ContainsArrayError, ContainsGroupError
from zarr.storage._local import LocalStore
from zarr.storage._memory import MemoryStore
Expand Down Expand Up @@ -54,59 +62,90 @@
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 ANY_ACCESS_MODE:
raise ValueError(f"Invalid mode: {mode}, expected one of {ANY_ACCESS_MODE}")

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!r}. 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(

Check warning on line 128 in src/zarr/storage/_common.py

View check run for this annotation

Codecov / codecov/patch

src/zarr/storage/_common.py#L128

Added line #L128 was not covered by tests
"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(
Expand Down
2 changes: 1 addition & 1 deletion tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
7 changes: 5 additions & 2 deletions tests/test_store/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'. Creating 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")