Repository: numpy/numpy
Pull request: BUG: fancy indexing copy
* Fixes gh-26542

* For these very simple advanced indexing cases, if the `result` and `self` arrays share the same data pointer, use a copy of `result` to assign values to `self`, otherwise the outcome of the fancy indexing suffers from mutation of `result` before the assignments are complete.

Problem statement:
Fancy indexing fails on a[idx] = a
### Describe the issue:

Fancy indexing seems to produce incorrect results when calling
```
a[idx] = a
```
(see code example below). I guess, the [] operator does not to check if the id(r.h.s.) == id(self). One work-around is to call:
```
a[idx] = np.copy(a)
```
However, this is far from optimal, of course, when the set of indices in idx is small (compared to the full set of indices of a). At any rate, I believe the current result is not what a user expects, and it would be better to raise an exception in this case than have this kind of result creep in.

### Reproduce the code example:

```python
>>> import numpy as np
>>> a = np.array([0,1,2])
>>> idx = np.array([2,1,0])
>>> a[idx] = a
>>> a
array([0, 1, 0])
```


### Error message:

_No response_

### Python and NumPy Versions:

>>> print(np.__version__); print(sys.version)
1.26.4
3.9.13 (main, Jul 29 2022, 10:49:47) 
[Clang 10.0.1 (clang-1001.0.46.4)]

### Runtime Environment:

_No response_

### Context for the issue:

_No response_

Hints from issue discussion:
I agree it is a bit confusing, I suppose one might naively expect this to reverse the array. That said, the footgun would still be easy to achieve even if an id check were in place I think:

```python
import numpy as np
a = np.array([0, 1, 2])
idx = np.array([2, 1, 0])
b = a[::]
print(id(a))
print(id(b))
a[idx] = b
print(a)
```

```
4312938512
4322288400
[0 1 0]
```

You could use `np.may_share_memory(a, b)`, but then I think from a design standpoint you end up with an NP-complete problem of assessing memory overlap of two arrays according to the NumPy docs! I'm guessing examples like this have been discussed previously.

I suppose one could add checks for the more obvious cases, but it may not be clear where to draw the line, and my guess would be there isn't much appetite for those kinds of shims, and performance tradeoffs on preventing corner case footguns, but I could be wrong.
I would actually consider this a small bug and otherwise an enhancement.  In most places NumPy detects this and implicitly makes the copy for the user now.  (e.g. with `out=` in ufuncs)
Indexing should probably do that check as well.
Interesting! Would/does the check go as far as detecting partial memory overlap, or just for having the same i.e., `arr.base`?
Yes, it would not be able to optimize by inspecting the index of cojrse.

Commit message: MAINT, BUG: PR 26558 revisions

* Avoid leaking the old `tmp_arr`, based on reviewer feedback.

* Add a test for a similar 2D case that fails, then hoist
`solve_may_share_memory()` check farther up in the control
flow such that the test passes.

* Add a reviewer-requested test for index overlap.

Patch to review:
diff --git a/numpy/_core/src/multiarray/mapping.c b/numpy/_core/src/multiarray/mapping.c
index 1861241a040e..fa40af24bc48 100644
--- a/numpy/_core/src/multiarray/mapping.c
+++ b/numpy/_core/src/multiarray/mapping.c
@@ -1960,6 +1960,10 @@ array_assign_subscript(PyArrayObject *self, PyObject *ind, PyObject *op)
         tmp_arr = (PyArrayObject *)op;
     }
 
+    if (tmp_arr && solve_may_share_memory(self, tmp_arr, 1) == 1) {
+        Py_SETREF(tmp_arr, (PyArrayObject *)PyArray_NewCopy(tmp_arr, NPY_ANYORDER));
+    }
+
     /*
      * Special case for very simple 1-d fancy indexing, which however
      * is quite common. This saves not only a lot of setup time in the
diff --git a/numpy/_core/tests/test_indexing.py b/numpy/_core/tests/test_indexing.py
index bea1c1017fb2..9611f75221d2 100644
--- a/numpy/_core/tests/test_indexing.py
+++ b/numpy/_core/tests/test_indexing.py
@@ -133,6 +133,28 @@ def test_empty_fancy_index(self):
         b = np.array([])
         assert_raises(IndexError, a.__getitem__, b)
 
+    def test_gh_26542(self):
+        a = np.array([0, 1, 2])
+        idx = np.array([2, 1, 0])
+        a[idx] = a
+        expected = np.array([2, 1, 0])
+        assert_equal(a, expected)
+
+    def test_gh_26542_2d(self):
+        a = np.array([[0, 1, 2]])
+        idx_row = np.zeros(3, dtype=int)
+        idx_col = np.array([2, 1, 0])
+        a[idx_row, idx_col] = a
+        expected = np.array([[2, 1, 0]])
+        assert_equal(a, expected)
+
+    def test_gh_26542_index_overlap(self):
+        arr = np.arange(100)
+        expected_vals = np.copy(arr[:-10])
+        arr[10:] = arr[:-10]
+        actual_vals = arr[10:]
+        assert_equal(actual_vals, expected_vals)
+
     def test_ellipsis_index(self):
         a = np.array([[1, 2, 3],
                       [4, 5, 6],


Code review comments:
diff_hunk: @@ -1960,6 +1960,10 @@ array_assign_subscript(PyArrayObject *self, PyObject *ind, PyObject *op)
         tmp_arr = (PyArrayObject *)op;
     }
 
+    if (tmp_arr && solve_may_share_memory(self, tmp_arr, 1) == 1) {
path: numpy/_core/src/multiarray/mapping.c
text: @user1:
```suggestion
    if (tmp_arr && solve_may_share_memory(self, tmp_arr, 1) != 0) {
```
Since `< 0` means "failed to solve", which would mean we should do a defensive copy.

We will be missing if the index has overlap, such as something awkward like:
```
a = np.arange(10)
a[2:][a[:-2]] = 3
```
but let's just accept that here and make a new issue for that.