Fix overwrite modes (#3062)

* Fix overwrite modes

* Add many more tests that data doesn't disappear

* Add bugfix entry.

* Fix function name

Author: dstansby

changes/3062.bugfix.rst

@@ -0,0 +1 @@
+Using various functions to open data with ``mode='a'`` no longer deletes existing data in the store.

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 = {}
 

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

tests/test_api.py

@@ -4,13 +4,15 @@
 from typing import TYPE_CHECKING
 
 import zarr.codecs
+import zarr.storage
 
 if TYPE_CHECKING:
     import pathlib
 
     from zarr.abc.store import Store
     from zarr.core.common import JSON, MemoryOrder, ZarrFormat
 
+import contextlib
 import warnings
 from typing import Literal
 
@@ -27,9 +29,9 @@
     create,
     create_array,
     create_group,
+    from_array,
     group,
     load,
-    open,
     open_group,
     save,
     save_array,
@@ -41,6 +43,10 @@
 from zarr.storage._utils import normalize_path
 from zarr.testing.utils import gpu_test
 
+if TYPE_CHECKING:
+    from collections.abc import Callable
+    from pathlib import Path
+
 
 def test_create(memory_store: Store) -> None:
     store = memory_store
@@ -135,28 +141,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 +239,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 +1083,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 +1242,62 @@ def test_v2_with_v3_compressor() -> None:
         zarr.create(
             store={}, shape=(1), dtype="uint8", zarr_format=2, compressor=zarr.codecs.BloscCodec()
         )
+
+
+def add_empty_file(path: Path) -> Path:
+    fpath = path / "a.txt"
+    fpath.touch()
+    return fpath
+
+
+@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", [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)
+    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()