Skip to content

Commit b05a91f

Browse files
ianhid-v-b
andauthored
Fix/from array overwrite source (#4405)
* fix(array): refuse from_array overwrites that overlap the source With overwrite=True, from_array deletes the destination before streaming data from the source. When the source array lives at, above, or below the destination path in the same store, that delete removes the source's chunks and the result is all fill values. Raise a ValueError before anything is deleted instead. * test(array): check metadata-only overwrite of the source keeps its metadata * special case weird eq behavior * docs: number the changelog entry for #4405 --------- Co-authored-by: Davis Bennett <[email protected]>
1 parent 069fd20 commit b05a91f

3 files changed

Lines changed: 90 additions & 0 deletions

File tree

‎changes/4405.bugfix.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
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‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from __future__ import annotations
22

3+
import contextlib
34
import copy
45
import math
56
import warnings
@@ -155,6 +156,7 @@
155156
get_pipeline_class,
156157
)
157158
from zarr.storage._common import StorePath, make_store_path
159+
from zarr.storage._memory import MemoryStore
158160
from zarr.storage._utils import _relativize_path
159161

160162
if TYPE_CHECKING:
@@ -4177,6 +4179,21 @@ class ShardsConfigParam(TypedDict):
41774179
type ShardsLike = ChunksLike | ShardsConfigParam | Literal["auto"]
41784180

41794181

4182+
def _stores_share_data(a: Store, b: Store) -> bool:
4183+
"""Whether two stores are views onto the same underlying data.
4184+
4185+
``Store.__eq__`` can't answer this directly. Several stores include ``read_only`` in
4186+
their equality check, so a read-only and a writable view of the same data are
4187+
unequal. ``MemoryStore`` compares its dicts by value, so two distinct stores with
4188+
equal contents are equal.
4189+
"""
4190+
if type(a) is MemoryStore and type(b) is MemoryStore:
4191+
return a._store_dict is b._store_dict
4192+
with contextlib.suppress(NotImplementedError):
4193+
a = a.with_read_only(b.read_only)
4194+
return a == b
4195+
4196+
41804197
async def from_array(
41814198
store: StoreLike,
41824199
*,
@@ -4412,6 +4429,23 @@ async def from_array(
44124429
if not hasattr(data, "dtype") or not hasattr(data, "shape"):
44134430
data = np.array(data)
44144431

4432+
# init_array deletes everything under the destination before the copy streams from the
4433+
# source, so a destination that overlaps the source would copy only fill values.
4434+
if (
4435+
write_data
4436+
and overwrite
4437+
and isinstance(data, Array)
4438+
and _stores_share_data(data.store_path.store, store_path.store)
4439+
):
4440+
src_prefix = f"{data.store_path.path}/" if data.store_path.path else ""
4441+
dest_prefix = f"{store_path.path}/" if store_path.path else ""
4442+
if src_prefix.startswith(dest_prefix) or dest_prefix.startswith(src_prefix):
4443+
raise ValueError(
4444+
f"Cannot overwrite {store_path.path!r} with the array at "
4445+
f"{data.store_path.path!r} because the paths overlap. Write to a different "
4446+
"path, or pass data=array[...]."
4447+
)
4448+
44154449
result = await init_array(
44164450
store_path=store_path,
44174451
shape=data.shape,

‎tests/test_array.py‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1975,6 +1975,61 @@ def test_from_array_arraylike_gains_no_attributes() -> None:
19751975
assert result.fill_value == 0
19761976

19771977

1978+
@pytest.mark.parametrize("store", ["local", "memory"], indirect=True)
1979+
@pytest.mark.parametrize(
1980+
("src_name", "dest_name"),
1981+
[("a", "a"), ("g/a", "g"), ("g/a", ""), ("a", "a/c")],
1982+
)
1983+
def test_from_array_overwrite_overlapping_source_raises(
1984+
store: Store, src_name: str, dest_name: str
1985+
) -> None:
1986+
"""Overwriting a destination that overlaps the source array raises instead of
1987+
deleting the source before its data is copied."""
1988+
src = zarr.create_array(store, name=src_name, data=np.arange(4.0), fill_value=-1.0)
1989+
with pytest.raises(ValueError, match="paths overlap"):
1990+
zarr.from_array(store, name=dest_name, data=src, overwrite=True)
1991+
np.testing.assert_array_equal(src[...], np.arange(4.0))
1992+
1993+
1994+
@pytest.mark.parametrize("store", ["local", "memory"], indirect=True)
1995+
def test_from_array_overwrite_read_only_source_raises(store: Store) -> None:
1996+
"""A read-only view of the destination store still counts as overlapping."""
1997+
zarr.create_array(store, name="a", data=np.arange(4.0), fill_value=-1.0)
1998+
src = zarr.open_array(store.with_read_only(True), path="a")
1999+
with pytest.raises(ValueError, match="paths overlap"):
2000+
zarr.from_array(store, name="a", data=src, overwrite=True)
2001+
np.testing.assert_array_equal(src[...], np.arange(4.0))
2002+
2003+
2004+
def test_from_array_overwrite_equal_memory_stores() -> None:
2005+
"""Distinct MemoryStores with equal contents don't overlap."""
2006+
src_store, dest_store = MemoryStore(), MemoryStore()
2007+
src = zarr.create_array(src_store, name="a", data=np.arange(4.0), fill_value=-1.0)
2008+
zarr.create_array(dest_store, name="a", data=np.arange(4.0), fill_value=-1.0)
2009+
result = zarr.from_array(dest_store, name="a", data=src, overwrite=True)
2010+
np.testing.assert_array_equal(result[...], np.arange(4.0))
2011+
2012+
2013+
@pytest.mark.parametrize("store", ["memory"], indirect=True)
2014+
def test_from_array_overwrite_non_overlapping_source(store: Store) -> None:
2015+
"""Overlap checks respect path boundaries, and metadata-only copies are allowed."""
2016+
src = zarr.create_array(
2017+
store,
2018+
name="ab",
2019+
data=np.arange(4.0),
2020+
chunks=(2,),
2021+
fill_value=-1.0,
2022+
attributes={"units": "K"},
2023+
)
2024+
zarr.create_array(store, name="a", shape=(2,), dtype="int8")
2025+
result = zarr.from_array(store, name="a", data=src, overwrite=True)
2026+
np.testing.assert_array_equal(result[...], np.arange(4.0))
2027+
2028+
meta_only = zarr.from_array(store, name="ab", data=src, overwrite=True, write_data=False)
2029+
np.testing.assert_array_equal(meta_only[...], np.full(4, -1.0))
2030+
assert zarr.open_array(store, path="ab").metadata == src.metadata
2031+
2032+
19782033
def test_from_array_F_order() -> None:
19792034
arr = zarr.create_array(store={}, data=np.array([1]), order="F", zarr_format=2)
19802035
with pytest.warns(

0 commit comments

Comments
 (0)