Skip to content

Prevent mode='r+' from creating new directories #3307

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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: 1 addition & 1 deletion src/zarr/storage/_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ async def make_store_path(

elif isinstance(store_like, Path):
# Create a new LocalStore
store = await LocalStore.open(root=store_like, read_only=_read_only)
store = await LocalStore.open(root=store_like, mode=mode)

elif isinstance(store_like, str):
# Either a FSSpec URI or a local filesystem path
Expand Down
48 changes: 44 additions & 4 deletions src/zarr/storage/_local.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
ByteRequest,
Expand All @@ -16,7 +16,7 @@
)
from zarr.core.buffer import Buffer
from zarr.core.buffer.core import default_buffer_prototype
from zarr.core.common import concurrent_map
from zarr.core.common import AccessModeLiteral, concurrent_map

if TYPE_CHECKING:
from collections.abc import AsyncIterator, Iterable
Expand Down Expand Up @@ -102,16 +102,56 @@ def __init__(self, root: Path | str, *, read_only: bool = False) -> None:
)
self.root = root

def with_read_only(self, read_only: bool = False) -> LocalStore:
def with_read_only(self, read_only: bool = False) -> Self:
# docstring inherited
return type(self)(
root=self.root,
read_only=read_only,
)

async def _open(self) -> None:
@classmethod
async def open(
cls, root: Path | str, *, read_only: bool = False, mode: AccessModeLiteral | None = None
) -> Self:
"""
Create and open the store.

Parameters
----------
root : str or Path
Directory to use as root of store.
read_only : bool
Whether the store is read-only
mode :
Mode in which to create the store. This only affects opening the store,
and the final read-only state of the store is controlled through the
read_only parameter.

Returns
-------
Store
The opened store instance.
"""
# If mode = 'r+', want to open in read only mode (fail if exists),
# but return a writeable store
if mode is not None:
read_only_creation = mode in ["r", "r+"]
else:
read_only_creation = read_only
store = cls(root, read_only=read_only_creation)
await store._open()

# Set read_only state
store = store.with_read_only(read_only)
await store._open()
return store

async def _open(self, *, mode: AccessModeLiteral | None = None) -> None:
if not self.read_only:
self.root.mkdir(parents=True, exist_ok=True)

if not self.root.exists():
raise FileNotFoundError(f"{self.root} does not exist")
return await super()._open()

async def clear(self) -> None:
Expand Down
6 changes: 5 additions & 1 deletion tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -285,8 +285,12 @@ def test_open_with_mode_r(tmp_path: Path) -> None:

def test_open_with_mode_r_plus(tmp_path: Path) -> None:
# 'r+' means read/write (must exist)
new_store_path = tmp_path / "new_store.zarr"
assert not new_store_path.exists(), "Test should operate on non-existent directory"
with pytest.raises(FileNotFoundError):
zarr.open(store=tmp_path, mode="r+")
zarr.open(store=new_store_path, mode="r+")
assert not new_store_path.exists(), "mode='r+' should not create directory"

zarr.ones(store=tmp_path, shape=(3, 3))
z2 = zarr.open(store=tmp_path, mode="r+")
assert isinstance(z2, Array)
Expand Down
Loading