(fix): add warning for setting X on view with repeat indices (#1501)

* (fix): add warning for setting `X` on view with repeat indices * (chore): release note * (fix): add check on indices for np array-ness * (fix): only warn with matrix value setter * (chore): update message * (fix): message * (fix): `ravel` idx before checking `len` * fmt * fmt --------- Co-authored-by: Phil Schaf <[email protected]>
scverse · Jul 4, 2024 · 9664ce4 · 9664ce4
1 parent 686786d
commit 9664ce4
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 4 deletions.
diff --git a/docs/release-notes/0.10.9.md b/docs/release-notes/0.10.9.md
@@ -3,6 +3,7 @@
 
 #### Bugfix
 
+* Add warning for setting `X` on a view with repeated indices {pr}`1501` {user}`ilan-gold`
 * Coerce {class}`numpy.matrix` classes to arrays when trying to store them in `AnnData` {pr}`1516` {user}`flying-sheep`
 * Fix for setting a dense `X` view with a sparse matrix {pr}`1532` {user}`ilan-gold`
 * Upper bound {mod}`numpy` for `gpu` installation on account of https://github.com/cupy/cupy/issues/8391 {pr}`1540` {user}`ilan-gold`

diff --git a/src/anndata/_core/anndata.py b/src/anndata/_core/anndata.py
@@ -601,10 +601,23 @@ def X(self, value: np.ndarray | sparse.spmatrix | SpArray | None):
             or (self.n_vars == 1 and self.n_obs == len(value))
             or (self.n_obs == 1 and self.n_vars == len(value))
         ):
-            if not np.isscalar(value) and self.shape != value.shape:
-                # For assigning vector of values to 2d array or matrix
-                # Not necessary for row of 2d array
-                value = value.reshape(self.shape)
+            if not np.isscalar(value):
+                if self.is_view and any(
+                    isinstance(idx, np.ndarray)
+                    and len(np.unique(idx)) != len(idx.ravel())
+                    for idx in [oidx, vidx]
+                ):
+                    msg = (
+                        "You are attempting to set `X` to a matrix on a view which has non-unique indices. "
+                        "The resulting `adata.X` will likely not equal the value to which you set it. "
+                        "To avoid this potential issue, please make a copy of the data first. "
+                        "In the future, this operation will throw an error."
+                    )
+                    warnings.warn(msg, FutureWarning, stacklevel=1)
+                if self.shape != value.shape:
+                    # For assigning vector of values to 2d array or matrix
+                    # Not necessary for row of 2d array
+                    value = value.reshape(self.shape)
             if self.isbacked:
                 if self.is_view:
                     X = self.file["X"]

diff --git a/tests/test_x.py b/tests/test_x.py
@@ -41,6 +41,17 @@ def test_setter_singular_dim(shape, orig_array_type, new_array_type):
     assert isinstance(adata.X, type(to_assign))
 
 
+def test_repeat_indices_view():
+    adata = gen_adata((10, 10), X_type=np.asarray)
+    subset = adata[[0, 0, 1, 1], :]
+    mat = np.array([np.ones(adata.shape[1]) * i for i in range(4)])
+    with pytest.warns(
+        FutureWarning,
+        match=r"You are attempting to set `X` to a matrix on a view which has non-unique indices",
+    ):
+        subset.X = mat
+
+
 @pytest.mark.parametrize("orig_array_type", UNLABELLED_ARRAY_TYPES)
 @pytest.mark.parametrize("new_array_type", UNLABELLED_ARRAY_TYPES)
 def test_setter_view(orig_array_type, new_array_type):