Fix/from array overwrite source

changes/4405.bugfix.md

@@ -0,0 +1 @@
+`from_array` now raises a `ValueError` when `overwrite=True` and the destination overlaps the source array (the same path, a parent of it, or a path inside it). Previously the destination was deleted before the data was copied, which also deleted the source and left an array of fill values.

src/zarr/core/array.py

@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+import contextlib
 import copy
 import math
 import warnings
@@ -147,6 +148,7 @@
     get_pipeline_class,
 )
 from zarr.storage._common import StorePath, ensure_no_existing_node, make_store_path
+from zarr.storage._memory import MemoryStore
 from zarr.storage._utils import _relativize_path
 
 if TYPE_CHECKING:
@@ -4132,6 +4134,21 @@ class ShardsConfigParam(TypedDict):
 )
 
 
+def _stores_share_data(a: Store, b: Store) -> bool:
+    """Whether two stores are views onto the same underlying data.
+
+    ``Store.__eq__`` can't answer this directly. Several stores include ``read_only`` in
+    their equality check, so a read-only and a writable view of the same data are
+    unequal. ``MemoryStore`` compares its dicts by value, so two distinct stores with
+    equal contents are equal.
+    """
+    if type(a) is MemoryStore and type(b) is MemoryStore:
+        return a._store_dict is b._store_dict
+    with contextlib.suppress(NotImplementedError):
+        a = a.with_read_only(b.read_only)
+    return a == b
+
+
 async def from_array(
     store: StoreLike,
     *,
@@ -4367,6 +4384,23 @@ async def from_array(
     if not hasattr(data, "dtype") or not hasattr(data, "shape"):
         data = np.array(data)
 
+    # init_array deletes everything under the destination before the copy streams from the
+    # source, so a destination that overlaps the source would copy only fill values.
+    if (
+        write_data
+        and overwrite
+        and isinstance(data, Array)
+        and _stores_share_data(data.store_path.store, store_path.store)
+    ):
+        src_prefix = f"{data.store_path.path}/" if data.store_path.path else ""
+        dest_prefix = f"{store_path.path}/" if store_path.path else ""
+        if src_prefix.startswith(dest_prefix) or dest_prefix.startswith(src_prefix):
+            raise ValueError(
+                f"Cannot overwrite {store_path.path!r} with the array at "
+                f"{data.store_path.path!r} because the paths overlap. Write to a different "
+                "path, or pass data=array[...]."
+            )
+
     result = await init_array(
         store_path=store_path,
         shape=data.shape,

tests/test_array.py

@@ -1963,6 +1963,61 @@ def test_from_array_arraylike_gains_no_attributes() -> None:
     assert result.fill_value == 0
 
 
+@pytest.mark.parametrize("store", ["local", "memory"], indirect=True)
+@pytest.mark.parametrize(
+    ("src_name", "dest_name"),
+    [("a", "a"), ("g/a", "g"), ("g/a", ""), ("a", "a/c")],
+)
+def test_from_array_overwrite_overlapping_source_raises(
+    store: Store, src_name: str, dest_name: str
+) -> None:
+    """Overwriting a destination that overlaps the source array raises instead of
+    deleting the source before its data is copied."""
+    src = zarr.create_array(store, name=src_name, data=np.arange(4.0), fill_value=-1.0)
+    with pytest.raises(ValueError, match="paths overlap"):
+        zarr.from_array(store, name=dest_name, data=src, overwrite=True)
+    np.testing.assert_array_equal(src[...], np.arange(4.0))
+
+
+@pytest.mark.parametrize("store", ["local", "memory"], indirect=True)
+def test_from_array_overwrite_read_only_source_raises(store: Store) -> None:
+    """A read-only view of the destination store still counts as overlapping."""
+    zarr.create_array(store, name="a", data=np.arange(4.0), fill_value=-1.0)
+    src = zarr.open_array(store.with_read_only(True), path="a")
+    with pytest.raises(ValueError, match="paths overlap"):
+        zarr.from_array(store, name="a", data=src, overwrite=True)
+    np.testing.assert_array_equal(src[...], np.arange(4.0))
+
+
+def test_from_array_overwrite_equal_memory_stores() -> None:
+    """Distinct MemoryStores with equal contents don't overlap."""
+    src_store, dest_store = MemoryStore(), MemoryStore()
+    src = zarr.create_array(src_store, name="a", data=np.arange(4.0), fill_value=-1.0)
+    zarr.create_array(dest_store, name="a", data=np.arange(4.0), fill_value=-1.0)
+    result = zarr.from_array(dest_store, name="a", data=src, overwrite=True)
+    np.testing.assert_array_equal(result[...], np.arange(4.0))
+
+
+@pytest.mark.parametrize("store", ["memory"], indirect=True)
+def test_from_array_overwrite_non_overlapping_source(store: Store) -> None:
+    """Overlap checks respect path boundaries, and metadata-only copies are allowed."""
+    src = zarr.create_array(
+        store,
+        name="ab",
+        data=np.arange(4.0),
+        chunks=(2,),
+        fill_value=-1.0,
+        attributes={"units": "K"},
+    )
+    zarr.create_array(store, name="a", shape=(2,), dtype="int8")
+    result = zarr.from_array(store, name="a", data=src, overwrite=True)
+    np.testing.assert_array_equal(result[...], np.arange(4.0))
+
+    meta_only = zarr.from_array(store, name="ab", data=src, overwrite=True, write_data=False)
+    np.testing.assert_array_equal(meta_only[...], np.full(4, -1.0))
+    assert zarr.open_array(store, path="ab").metadata == src.metadata
+
+
 def test_from_array_F_order() -> None:
     arr = zarr.create_array(store={}, data=np.array([1]), order="F", zarr_format=2)
     with pytest.warns(