Repository: numpy/numpy
Pull request: BUG: masked array proper deepcopies
Fixes #22556
Fixes #21022

* add regression test and fix for gh-22556, where we were relying on the array `copy` arg to deepcopy a compound object type; I thought about performance issues here, but if you are already in the land of `object` and you are explicitly opting in to `deepcopy`, it seems like performance might be wishful thinking anyway

* add regression test and fix for gh-21022--this one was weirder but seems possible to sidestep by not trying to assign a shape of `()` to something that already has shape `()` and a non-writeable `shape` attribute

Problem statement:
BUG: Error while trying to create a deepcopy of numpy masked_array
### Describe the issue:

We faced an issue when trying to create a deepcopy of numpy masked_array which was created from masked constant - the result of `numpy.prod` operation. But when we create masked array with same shape (variable `array` from the example below) not from conversion it deepcopied successfully.

### Reproduce the code example:

```python
import numpy as np
from copy import deepcopy

source = np.ma.masked_array(data=[-1, -1], mask=True, dtype=np.float64)

axis = np.array(0)
result = np.prod(source, axis=axis, keepdims=False)
result = np.ma.masked_array(result, mask=np.ones(result.shape, dtype=np.bool))

array = np.ma.masked_array(data=-1, mask=True, dtype=np.float64)
deepcopy(array)
deepcopy(result)
```


### Error message:

```shell
Traceback (most recent call last):
  File "C:\...\JetBrains\PyCharmCE2021.1\scratches\scratch.py", line 12, in <module>
    deepcopy(result)
  File "C:\...\Python\Python39\lib\copy.py", line 153, in deepcopy
    y = copier(memo)
  File "C:\...\lib\site-packages\numpy\ma\core.py", line 6218, in __deepcopy__
    copied = MaskedArray.__new__(type(self), self, copy=True)
  File "C:\...\lib\site-packages\numpy\ma\core.py", line 2880, in __new__
    data._mask.shape = data.shape
AttributeError: attribute 'shape' of 'numpy.generic' objects is not writable

Process finished with exit code 1
```


### NumPy/Python version information:

1.22.2 3.9.0 (tags/v3.9.0:9cf6752, Oct  5 2020, 15:34:40) [MSC v.1927 64 bit (AMD64)]

BUG: Changing element of deepcopy of masked array alters element of original array
### Describe the issue:

Altering an element of a deepcopy of a masked array alters the element of the original array.

### Reproduce the code example:

```python
import copy
import numpy as np

source = np.ma.array( [ 0,[0,1,2] ] ,dtype=object)
deepcopy = copy.deepcopy(source)

print()
print('Before altering deepcopy:')
print(f'source:   {source}')
print(f'deepcopy: {deepcopy}')

deepcopy[1].append('this should not appear in source')

print()
print('After altering deepcopy:')
print(f'source:   {source}')
print(f'deepcopy: {deepcopy}')

# output:

# Before altering deepcopy:
# source:   [0 list([0, 1, 2])]
# deepcopy: [0 list([0, 1, 2])]

# After altering deepcopy:
# source:   [0 list([0, 1, 2, 'this should not appear in source'])]
# deepcopy: [0 list([0, 1, 2, 'this should not appear in source'])]
```


### Error message:

_No response_

### NumPy/Python version information:

import sys, numpy; print(numpy.__version__, sys.version)
1.23.3 3.9.13 | packaged by conda-forge | (main, May 27 2022, 16:50:36) [MSC v.1929 64 bit (AMD64)]

### Context for the issue:

_No response_

Hints from issue discussion:
Thanks for the report @ogliver. At first sight that does look like a real bug. `deepcopy` is supposed to also copy the objects being referenced by the pointers inside the object array.

This looks related to gh-21022, it'd be good to look into both issues together.
I'd like to take up this issue along with trying to fix gh-21022 as well. Is that okay?
Hey,

Thank you for reaching out. I would like to claim this bug and hope this is
alright with you, since you aren't too far in the process.

I would really appreciate it if you could also share with me any useful
information you might have gathered regarding the bug and also potential
solutions you have considered. This would be of great help to me.

Thanks again.

Best regards,
Ayan

On Fri, Nov 11, 2022 at 11:39 AM LU ***@***.***> wrote:

> I'd like to take up this issue along with trying to fix gh-21022
> <https://github.com/numpy/numpy/issues/21022> as well. Is that okay?
>
> Hi, I have started working on it a few days ago (though haven't progressed
> too much). Do you want to claim it, or let me continue to work on it?
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/numpy/numpy/issues/22556#issuecomment-1311930418>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AOHZQJOWWBBXRGZC45K2VULWHZZEPANCNFSM6AAAAAAR2GQF3A>
> .
> You are receiving this because you commented.Message ID:
> ***@***.***>
>

OK, it is left to you. I would say, the cause seems to be from `np.array()` whose parameter `copy` is not handled properly. By my understanding, correction should de done either in `core/multiarray.py` as an indirection, or better modify low level C codes.

Commit message: BUG: masked array proper deepcopies

Fixes #22556
Fixes #21022

* add regression test and fix for gh-22556, where
we were relying on the array `copy` arg to deepcopy
a compound object type; I thought about performance issues
here, but if you are already in the land of `object` and
you are explicitly opting in to `deepcopy`, it seems like
performance might be wishful thinking anyway

* add regression test and fix for gh-21022--this one was
weirder but seems possible to sidestep by not trying
to assign a shape of `()` to something that already has
shape `()` and a non-writeable `shape` attribute

Patch to review:
diff --git a/numpy/ma/core.py b/numpy/ma/core.py
index b76ff8ea9eff..795e79e000b0 100644
--- a/numpy/ma/core.py
+++ b/numpy/ma/core.py
@@ -2870,7 +2870,13 @@ def __new__(cls, data=None, mask=nomask, dtype=None, copy=False,
                     _data._mask = _data._mask.copy()
                     # Reset the shape of the original mask
                     if getmask(data) is not nomask:
-                        data._mask.shape = data.shape
+                        # gh-21022 encounters an issue here
+                        # because data._mask.shape is not writeable, but
+                        # the op was also pointless in that case, because
+                        # the shapes were the same, so we can at least
+                        # avoid that path
+                        if data._mask.shape != data.shape:
+                            data._mask.shape = data.shape
         else:
             # Case 2. : With a mask in input.
             # If mask is boolean, create an array of True or False
@@ -6300,6 +6306,13 @@ def __deepcopy__(self, memo=None):
         memo[id(self)] = copied
         for (k, v) in self.__dict__.items():
             copied.__dict__[k] = deepcopy(v, memo)
+        # as clearly documented for np.copy(), you need to use
+        # deepcopy() directly for arrays of object type that may
+        # contain compound types--you cannot depend on normal
+        # copy semantics to do the right thing here
+        if self.dtype.type is np.object_:
+            for idx, _ in enumerate(copied):
+                copied[idx] = deepcopy(copied[idx])
         return copied
 
 
diff --git a/numpy/ma/tests/test_core.py b/numpy/ma/tests/test_core.py
index 7d23e32ec3b7..506675ca0c64 100644
--- a/numpy/ma/tests/test_core.py
+++ b/numpy/ma/tests/test_core.py
@@ -8,6 +8,7 @@
 
 import sys
 import warnings
+import copy
 import operator
 import itertools
 import textwrap
@@ -5570,3 +5571,22 @@ def method(self):
 original note"""
 
     assert_equal(np.ma.core.doc_note(method.__doc__, "note"), expected_doc)
+
+
+def test_gh_22556():
+    source = np.ma.array([0, [0, 1, 2]], dtype=object)
+    deepcopy = copy.deepcopy(source)
+    deepcopy[1].append('this should not appear in source')
+    assert len(source[1]) == 3
+
+
+def test_gh_21022():
+    # testing for absence of reported error
+    source = np.ma.masked_array(data=[-1, -1], mask=True, dtype=np.float64)
+    axis = np.array(0)
+    result = np.prod(source, axis=axis, keepdims=False)
+    result = np.ma.masked_array(result,
+                                mask=np.ones(result.shape, dtype=np.bool_))
+    array = np.ma.masked_array(data=-1, mask=True, dtype=np.float64)
+    copy.deepcopy(array)
+    copy.deepcopy(result)


Code review comments:
diff_hunk: @@ -6300,6 +6306,13 @@ def __deepcopy__(self, memo=None):
         memo[id(self)] = copied
         for (k, v) in self.__dict__.items():
             copied.__dict__[k] = deepcopy(v, memo)
+        # as clearly documented for np.copy(), you need to use
+        # deepcopy() directly for arrays of object type that may
+        # contain compound types--you cannot depend on normal
+        # copy semantics to do the right thing here
+        if self.dtype.type is np.object_:
+            for idx, _ in enumerate(copied):
+                copied[idx] = deepcopy(copied[idx])
path: numpy/ma/core.py
text: @user1:
Maybe just call deepcopy on `_data`?  You can guard it by `dtype.hasobject` instead.  If we put it here, that may copy it twice, but maybe that is better than the bug.

I don't trust that the code works right now for 2-D masked arrays.