From 49e8c53c5ec2d69b12e12b1624553c211313e6ab Mon Sep 17 00:00:00 2001 From: Deepak Cherian Date: Mon, 13 May 2024 13:23:04 -0600 Subject: [PATCH 1/8] Fix a regression with scalar indexing due to #1800 --- docs/release.rst | 2 ++ zarr/core.py | 4 +++- zarr/tests/test_core.py | 10 ++++++++++ 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/docs/release.rst b/docs/release.rst index e2f9f3de85..ce639873ae 100644 --- a/docs/release.rst +++ b/docs/release.rst @@ -31,6 +31,8 @@ Docs Maintenance ~~~~~~~~~~~ +* Fix a regression when indexing out a single value from arrays with size-1 chunks. + By :user:`Deepak Cherian ` Deprecations ~~~~~~~~~~~~ diff --git a/zarr/core.py b/zarr/core.py index 6aa86b6465..b1ccd203db 100644 --- a/zarr/core.py +++ b/zarr/core.py @@ -2030,7 +2030,9 @@ def _process_chunk( and not self._filters and self._dtype != object ): - dest = out[out_selection] + # For 0D arrays out_selection = () and out[out_selection] is a scalar + # Avoid that + dest = out[out_selection] if out_selection else out # Assume that array-like objects that doesn't have a # `writeable` flag is writable. dest_is_writable = getattr(dest, "writeable", True) diff --git a/zarr/tests/test_core.py b/zarr/tests/test_core.py index 730f724314..235aeab1f3 100644 --- a/zarr/tests/test_core.py +++ b/zarr/tests/test_core.py @@ -3157,3 +3157,13 @@ def test_issue_1279(tmpdir): written_data = ds_reopened[:] assert_array_equal(data, written_data) + + +def test_scalar_indexing(): + store = zarr.KVStore({}) + + store["a"] = zarr.create((3,), chunks=(1,), store=store) + store["a"][:] = [1, 2, 3] + + assert store["a"][1] == np.array(2.0) + assert store["a"][(1,)] == np.array(2.0) From 33193598c7e10374661c6d998aca446b98471f1c Mon Sep 17 00:00:00 2001 From: Deepak Cherian Date: Tue, 14 May 2024 07:17:35 -0600 Subject: [PATCH 2/8] Fix #1874 --- zarr/core.py | 7 +++++-- zarr/tests/test_core.py | 3 +++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/zarr/core.py b/zarr/core.py index b1ccd203db..d5d83a7018 100644 --- a/zarr/core.py +++ b/zarr/core.py @@ -2283,8 +2283,11 @@ def _process_for_setitem(self, ckey, chunk_selection, value, fields=None): chunk.fill(value) else: - # ensure array is contiguous - chunk = value.astype(self._dtype, order=self._order, copy=False) + if not isinstance(value, np.ndarray): + chunk = np.asarray(value, dtype=self._dtype, order=self._order) + else: + # ensure array is contiguous + chunk = value.astype(self._dtype, order=self._order, copy=False) else: # partially replace the contents of this chunk diff --git a/zarr/tests/test_core.py b/zarr/tests/test_core.py index 235aeab1f3..9eb111c711 100644 --- a/zarr/tests/test_core.py +++ b/zarr/tests/test_core.py @@ -3167,3 +3167,6 @@ def test_scalar_indexing(): assert store["a"][1] == np.array(2.0) assert store["a"][(1,)] == np.array(2.0) + + store["a"][0] = [-1] + assert store["a"][0] == np.array(-1) From 94fe60ad73a37282ce8e20252b64669cfe94bc7e Mon Sep 17 00:00:00 2001 From: Deepak Cherian Date: Tue, 14 May 2024 07:19:08 -0600 Subject: [PATCH 3/8] Update release notes --- docs/release.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/release.rst b/docs/release.rst index ce639873ae..a81be4638f 100644 --- a/docs/release.rst +++ b/docs/release.rst @@ -31,8 +31,8 @@ Docs Maintenance ~~~~~~~~~~~ -* Fix a regression when indexing out a single value from arrays with size-1 chunks. - By :user:`Deepak Cherian ` +* Fix a regression when getting or setting a single value from arrays with size-1 chunks. + By :user:`Deepak Cherian ` :issue:`1874` Deprecations ~~~~~~~~~~~~ From 2487d9ab5001a121af53255d05a275efb32d3523 Mon Sep 17 00:00:00 2001 From: Deepak Cherian Date: Tue, 14 May 2024 07:20:43 -0600 Subject: [PATCH 4/8] More test --- zarr/tests/test_core.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/zarr/tests/test_core.py b/zarr/tests/test_core.py index 9eb111c711..6b95f1cfc0 100644 --- a/zarr/tests/test_core.py +++ b/zarr/tests/test_core.py @@ -3170,3 +3170,9 @@ def test_scalar_indexing(): store["a"][0] = [-1] assert store["a"][0] == np.array(-1) + + store["a"][0] = -2 + assert store["a"][0] == np.array(-2) + + store["a"][0] = (-3,) + assert store["a"][0] == np.array(-3) From 41a52894d37bf26344e5c1bb978231657094f7d7 Mon Sep 17 00:00:00 2001 From: Deepak Cherian Date: Thu, 16 May 2024 07:45:46 -0600 Subject: [PATCH 5/8] More test --- zarr/tests/test_core.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/zarr/tests/test_core.py b/zarr/tests/test_core.py index 6b95f1cfc0..511569c9b9 100644 --- a/zarr/tests/test_core.py +++ b/zarr/tests/test_core.py @@ -3176,3 +3176,27 @@ def test_scalar_indexing(): store["a"][0] = (-3,) assert store["a"][0] == np.array(-3) + + +def test_object_array_indexing(): + # regression test for #1874 + from numcodecs import MsgPack + + root = zarr.group() + arr = root.create_dataset( + name="my_dataset", + shape=0, + dtype=object, + object_codec=MsgPack(), + ) + new_items = [ + ["A", 1], + ["B", 2, "hello"], + ] + arr_add = np.empty(len(new_items), dtype=object) + arr_add[:] = new_items + arr.append(arr_add) + + elem = ["C", 3] + arr[0] = elem + assert arr[0] == elem From 19f0f1d4491a6920bda1fad308dbce57b67b91eb Mon Sep 17 00:00:00 2001 From: Deepak Cherian Date: Thu, 16 May 2024 07:52:49 -0600 Subject: [PATCH 6/8] Try a different fix --- zarr/core.py | 7 ++----- zarr/indexing.py | 2 ++ 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/zarr/core.py b/zarr/core.py index d5d83a7018..b1ccd203db 100644 --- a/zarr/core.py +++ b/zarr/core.py @@ -2283,11 +2283,8 @@ def _process_for_setitem(self, ckey, chunk_selection, value, fields=None): chunk.fill(value) else: - if not isinstance(value, np.ndarray): - chunk = np.asarray(value, dtype=self._dtype, order=self._order) - else: - # ensure array is contiguous - chunk = value.astype(self._dtype, order=self._order, copy=False) + # ensure array is contiguous + chunk = value.astype(self._dtype, order=self._order, copy=False) else: # partially replace the contents of this chunk diff --git a/zarr/indexing.py b/zarr/indexing.py index 2f2402fe27..35c1e813b1 100644 --- a/zarr/indexing.py +++ b/zarr/indexing.py @@ -52,6 +52,8 @@ def is_scalar(value, dtype): return True if isinstance(value, tuple) and dtype.names and len(value) == len(dtype.names): return True + if dtype.kind == "O" and not isinstance(value, np.ndarray): + return True return False From 1dff2c4240b607d8bf30bb68986e678716de0394 Mon Sep 17 00:00:00 2001 From: Deepak Cherian Date: Thu, 16 May 2024 07:54:59 -0600 Subject: [PATCH 7/8] Update test --- zarr/tests/test_core.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zarr/tests/test_core.py b/zarr/tests/test_core.py index 511569c9b9..9f0b7f6cfd 100644 --- a/zarr/tests/test_core.py +++ b/zarr/tests/test_core.py @@ -3168,13 +3168,13 @@ def test_scalar_indexing(): assert store["a"][1] == np.array(2.0) assert store["a"][(1,)] == np.array(2.0) - store["a"][0] = [-1] + store["a"][slice(1)] = [-1] assert store["a"][0] == np.array(-1) store["a"][0] = -2 assert store["a"][0] == np.array(-2) - store["a"][0] = (-3,) + store["a"][slice(1)] = (-3,) assert store["a"][0] == np.array(-3) From da2dd720206e60f33d416edede9fa3c99d159a81 Mon Sep 17 00:00:00 2001 From: Deepak Cherian Date: Thu, 16 May 2024 07:58:05 -0600 Subject: [PATCH 8/8] more test --- zarr/tests/test_core.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/zarr/tests/test_core.py b/zarr/tests/test_core.py index 9f0b7f6cfd..01a78ecd68 100644 --- a/zarr/tests/test_core.py +++ b/zarr/tests/test_core.py @@ -3197,6 +3197,12 @@ def test_object_array_indexing(): arr_add[:] = new_items arr.append(arr_add) + # heterogeneous elements elem = ["C", 3] arr[0] = elem assert arr[0] == elem + + # homogeneous elements + elem = [1, 3] + arr[1] = elem + assert arr[1] == elem