From fe981bff9f435f63e3aa44729e17e9598a3f1bed Mon Sep 17 00:00:00 2001 From: David Stansby Date: Thu, 15 May 2025 15:33:16 +0100 Subject: [PATCH 1/4] Fix overwrite modes --- src/zarr/api/asynchronous.py | 3 +-- tests/test_api.py | 28 ++++++++++++++++++++-------- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/src/zarr/api/asynchronous.py b/src/zarr/api/asynchronous.py index cdedd5b033..4f3c9c3f8f 100644 --- a/src/zarr/api/asynchronous.py +++ b/src/zarr/api/asynchronous.py @@ -88,7 +88,7 @@ _READ_MODES: tuple[AccessModeLiteral, ...] = ("r", "r+", "a") _CREATE_MODES: tuple[AccessModeLiteral, ...] = ("a", "w", "w-") -_OVERWRITE_MODES: tuple[AccessModeLiteral, ...] = ("a", "r+", "w") +_OVERWRITE_MODES: tuple[AccessModeLiteral, ...] = ("w",) def _infer_overwrite(mode: AccessModeLiteral) -> bool: @@ -817,7 +817,6 @@ async def open_group( warnings.warn("chunk_store is not yet implemented", RuntimeWarning, stacklevel=2) store_path = await make_store_path(store, mode=mode, storage_options=storage_options, path=path) - if attributes is None: attributes = {} diff --git a/tests/test_api.py b/tests/test_api.py index 6904f91fe7..1a27fac569 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -29,7 +29,6 @@ create_group, group, load, - open, open_group, save, save_array, @@ -41,6 +40,9 @@ from zarr.storage._utils import normalize_path from zarr.testing.utils import gpu_test +if TYPE_CHECKING: + from pathlib import Path + def test_create(memory_store: Store) -> None: store = memory_store @@ -135,28 +137,28 @@ async def test_open_array(memory_store: MemoryStore, zarr_format: ZarrFormat) -> store = memory_store # open array, create if doesn't exist - z = open(store=store, shape=100, zarr_format=zarr_format) + z = zarr.api.synchronous.open(store=store, shape=100, zarr_format=zarr_format) assert isinstance(z, Array) assert z.shape == (100,) # open array, overwrite # store._store_dict = {} store = MemoryStore() - z = open(store=store, shape=200, zarr_format=zarr_format) + z = zarr.api.synchronous.open(store=store, shape=200, zarr_format=zarr_format) assert isinstance(z, Array) assert z.shape == (200,) # open array, read-only store_cls = type(store) ro_store = await store_cls.open(store_dict=store._store_dict, read_only=True) - z = open(store=ro_store, mode="r") + z = zarr.api.synchronous.open(store=ro_store, mode="r") assert isinstance(z, Array) assert z.shape == (200,) assert z.read_only # path not found with pytest.raises(FileNotFoundError): - open(store="doesnotexist", mode="r", zarr_format=zarr_format) + zarr.api.synchronous.open(store="doesnotexist", mode="r", zarr_format=zarr_format) @pytest.mark.parametrize("store", ["memory", "local", "zip"], indirect=True) @@ -233,12 +235,12 @@ def test_save(store: Store, n_args: int, n_kwargs: int) -> None: save(store) elif n_args == 1 and n_kwargs == 0: save(store, *args) - array = open(store) + array = zarr.api.synchronous.open(store) assert isinstance(array, Array) assert_array_equal(array[:], data) else: save(store, *args, **kwargs) # type: ignore [arg-type] - group = open(store) + group = zarr.api.synchronous.open(store) assert isinstance(group, Group) for array in group.array_values(): assert_array_equal(array[:], data) @@ -1077,7 +1079,7 @@ def test_tree() -> None: def test_open_positional_args_deprecated() -> None: store = MemoryStore() with pytest.warns(FutureWarning, match="pass"): - open(store, "w", shape=(1,)) + zarr.api.synchronous.open(store, "w", shape=(1,)) def test_save_array_positional_args_deprecated() -> None: @@ -1236,3 +1238,13 @@ def test_v2_with_v3_compressor() -> None: zarr.create( store={}, shape=(1), dtype="uint8", zarr_format=2, compressor=zarr.codecs.BloscCodec() ) + + +def test_create_no_delete_file(tmp_path: Path) -> None: + with open(tmp_path / "file.txt", "w") as f: + f.write("abc") + + with pytest.raises(NotImplementedError, match="loading groups not yet supported"): + zarr.load("./tmp") + # Even though above fails, it shouldn't delete existing file + assert (tmp_path / "file.txt").exists() From b7d7c1ca52512a258d0acd77ec131fc35dbe9532 Mon Sep 17 00:00:00 2001 From: David Stansby Date: Fri, 16 May 2025 17:47:27 +0100 Subject: [PATCH 2/4] Add many more tests that data doesn't disappear --- src/zarr/api/synchronous.py | 1 + tests/test_api.py | 67 +++++++++++++++++++++++++++++++++---- 2 files changed, 61 insertions(+), 7 deletions(-) diff --git a/src/zarr/api/synchronous.py b/src/zarr/api/synchronous.py index 24ab937db5..d4b652ad6e 100644 --- a/src/zarr/api/synchronous.py +++ b/src/zarr/api/synchronous.py @@ -858,6 +858,7 @@ def create_array( Ignored otherwise. overwrite : bool, default False Whether to overwrite an array with the same name in the store, if one exists. + If `True`, all existing paths in the store will be deleted. config : ArrayConfigLike, optional Runtime configuration for the array. write_data : bool diff --git a/tests/test_api.py b/tests/test_api.py index 1a27fac569..6ee65d5630 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -4,6 +4,7 @@ from typing import TYPE_CHECKING import zarr.codecs +import zarr.storage if TYPE_CHECKING: import pathlib @@ -11,6 +12,7 @@ from zarr.abc.store import Store from zarr.core.common import JSON, MemoryOrder, ZarrFormat +import contextlib import warnings from typing import Literal @@ -27,6 +29,7 @@ create, create_array, create_group, + from_array, group, load, open_group, @@ -41,6 +44,7 @@ from zarr.testing.utils import gpu_test if TYPE_CHECKING: + from collections.abc import Callable from pathlib import Path @@ -1240,11 +1244,60 @@ def test_v2_with_v3_compressor() -> None: ) -def test_create_no_delete_file(tmp_path: Path) -> None: - with open(tmp_path / "file.txt", "w") as f: - f.write("abc") +def add_empty_file(path: Path) -> Path: + fpath = path / "a.txt" + fpath.touch() + return fpath - with pytest.raises(NotImplementedError, match="loading groups not yet supported"): - zarr.load("./tmp") - # Even though above fails, it shouldn't delete existing file - assert (tmp_path / "file.txt").exists() + +@pytest.mark.parametrize("create_function", [create_array, from_array]) +@pytest.mark.parametrize("overwrite", [True, False]) +def test_no_overwrite_array(tmp_path: Path, create_function: Callable, overwrite: bool) -> None: # type:ignore[type-arg] + store = zarr.storage.LocalStore(tmp_path) + existing_fpath = add_empty_file(tmp_path) + + assert existing_fpath.exists() + create_function(store=store, data=np.ones(shape=(1,)), overwrite=overwrite) + if overwrite: + assert not existing_fpath.exists() + else: + assert existing_fpath.exists() + + +@pytest.mark.parametrize("create_function", [create_group, group]) +@pytest.mark.parametrize("overwrite", [True, False]) +def test_no_overwrite_group(tmp_path: Path, create_function: Callable, overwrite: bool) -> None: # type:ignore[type-arg] + store = zarr.storage.LocalStore(tmp_path) + existing_fpath = add_empty_file(tmp_path) + + assert existing_fpath.exists() + create_function(store=store, overwrite=overwrite) + if overwrite: + assert not existing_fpath.exists() + else: + assert existing_fpath.exists() + + +@pytest.mark.parametrize("open_func", [open, open_group]) +@pytest.mark.parametrize("mode", ["r", "r+", "a", "w", "w-"]) +def test_no_overwrite_open(tmp_path: Path, open_func: Callable, mode: str) -> None: # type:ignore[type-arg] + store = zarr.storage.LocalStore(tmp_path) + existing_fpath = add_empty_file(tmp_path) + + assert existing_fpath.exists() + with contextlib.suppress(FileExistsError, FileNotFoundError): + open_func(store=store, mode=mode) + if mode == "w": + assert not existing_fpath.exists() + else: + assert existing_fpath.exists() + + +def test_no_overwrite_load(tmp_path: Path) -> None: + store = zarr.storage.LocalStore(tmp_path) + existing_fpath = add_empty_file(tmp_path) + + assert existing_fpath.exists() + with contextlib.suppress(NotImplementedError): + zarr.load(store) + assert existing_fpath.exists() From ce7b3e745cd950fa05df1d9e6769d7161872c699 Mon Sep 17 00:00:00 2001 From: David Stansby Date: Fri, 16 May 2025 17:48:13 +0100 Subject: [PATCH 3/4] Add bugfix entry. --- changes/3062.bugfix.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 changes/3062.bugfix.rst diff --git a/changes/3062.bugfix.rst b/changes/3062.bugfix.rst new file mode 100644 index 0000000000..9e1e52ddb7 --- /dev/null +++ b/changes/3062.bugfix.rst @@ -0,0 +1 @@ +Using various functions to open data with ``mode='a'`` no longer deletes existing data in the store. From d7270fca4551d7c31a507b9f45e69df82ddd72e9 Mon Sep 17 00:00:00 2001 From: David Stansby Date: Fri, 16 May 2025 17:51:32 +0100 Subject: [PATCH 4/4] Fix function name --- tests/test_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_api.py b/tests/test_api.py index 6ee65d5630..ae112756c5 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -1278,7 +1278,7 @@ def test_no_overwrite_group(tmp_path: Path, create_function: Callable, overwrite assert existing_fpath.exists() -@pytest.mark.parametrize("open_func", [open, open_group]) +@pytest.mark.parametrize("open_func", [zarr.open, open_group]) @pytest.mark.parametrize("mode", ["r", "r+", "a", "w", "w-"]) def test_no_overwrite_open(tmp_path: Path, open_func: Callable, mode: str) -> None: # type:ignore[type-arg] store = zarr.storage.LocalStore(tmp_path)