Repository: numpy/numpy
Pull request: ENH: Add `subok` parameter to np.copy function (cf. #6509)
This is largely a re-submission of the original change proposed in #6509. Discussion was hosted in multiple forums including #3474, [the numpy mailing list circa 10-2015](https://mail.python.org/pipermail/numpy-discussion/2015-October/074021.html), and the [02-26-2020 NumPy Triage meeting](https://github.com/rossbar/archive/blob/triage_02-26-2020/triage_meetings/triage-2020-02-26.md).

This PR closes #3474 and #15570

Problem statement:
np.copy(masked array) has unexpected behavior
....where "unexpected" means _I_ didn't expect it.  This may just be a documentation fix, or maybe I will be scolded for not realizing how `np.ma` works, but I don't think this behavior is appropriate:

```
In [100]: x = np.ma.ones(5)

In [101]: x.mask = [False,False,True,False,False]

In [102]: np.copy(x)
Out[102]: array([ 1.,  1.,  1.,  1.,  1.])

In [103]: np.ma.copy(x)
Out[103]:
masked_array(data = [1.0 1.0 -- 1.0 1.0],
             mask = [False False  True False False],
       fill_value = 1e+20)

In [104]: np.__version__
Out[104]: '1.8.0.dev-074a40c'
```

I would at least expect a warning, but would prefer to see, in order of preference:
- `np.copy(x)` returns a masked array (since x is a masked array)
- `np.copy(x)` warns that it is unmasking the array, then returns the same array as above
- `np.copy(x)` raises an exception
- `np.copy(x)` returns `[1,1,nan,1,1]`  (this is probably not ideal)

Hints from issue discussion:
Still open in 1.9-devel

Not sure how to fix this without special casing masked arrays, though.

Would changing the logic of np.copy to include `subok=True` in the call to array fix this issue?  
From:

```
return array(a, order=order, copy=True)
```

To:

```
return array(a, order=order, subok=True, copy=True)
```

If so, should subok should be a parameter to `np.copy` and what should the default be, True or False?  True will return the first preferred result, False will keep the current behavior, but allow the expected behavior but adding an argument. 

I'm not in favor of adding a `subok` parameter to `np.copy` when we already have two completely viable solutions:
- use the `.copy()` method
- use `np.array(a, subok=True)` (it copies by default)

I would actually argue the opposite, since I think to most "copy" gives the impression that one would get something back that is, actually, a copy, and thus of the same type. I also see it as a small but useful contribution to make the whole of numpy more consistently able to deal with subclasses (just as I did earlier with `np.broadcast_arrays`, where no good substitute was available). However, given the need to remain backwards compatible, I do think we're stuck with a default of `subok=False`.

Anyway, @jjhelmus, why not make a quick PR and see what others think?

@shoyer: np.copy should probably go on your list of functions that need
some kind of duck array support, though -- for a duck array I think np.copy
and np.array should probably do different things. Maybe it should just call
arr.copy?

On Thu, Oct 15, 2015 at 7:00 AM, Marten van Kerkwijk <
notifications@github.com> wrote:

> I would actually argue the opposite, since I think to most "copy" gives
> the impression that one would get something back that is, actually, a copy,
> and thus of the same type. I also see it as a small but useful contribution
> to make the whole of numpy more consistently able to deal with subclasses
> (just as I did earlier with np.broadcast_arrays, where no good substitute
> was available). However, given the need to remain backwards compatible, I
> do think we're stuck with a default of subok=False.
> 
> Anyway, @jjhelmus https://github.com/jjhelmus, why not make a quick PR
> and see what others think?
> 
> —
> Reply to this email directly or view it on GitHub
> https://github.com/numpy/numpy/issues/3474#issuecomment-148395201.

## 

Nathaniel J. Smith -- http://vorpus.org

Yes, I would be +1 on making np.copy try calling the copy method on duck arrays. 

Still no reason not to support `subok` as well.  And of course one can just call `__copy__` if it exists.

Created PR #6509 which adds a **subok** parameter to the function with a default value of **False**.  I'm guessing this will open up some discussion on the topic. Not sure if this typically takes place on the PR, the issue, or the mailing list.

Api discussion should be on the mailing list.
On Oct 18, 2015 7:37 PM, "Jonathan J. Helmus" notifications@github.com
wrote:

> Created PR #6509 https://github.com/numpy/numpy/pull/6509 which adds a
> _subok_ parameter to the function with a default value of _False_. I'm
> guessing this will open up some discussion on the topic. Not sure if this
> typically takes place on the PR, the issue, or the mailing list.
> 
> —
> Reply to this email directly or view it on GitHub
> https://github.com/numpy/numpy/issues/3474#issuecomment-149080511.

Commit message: Added release note.

Patch to review:
diff --git a/doc/release/upcoming_changes/15685.new_feature.rst b/doc/release/upcoming_changes/15685.new_feature.rst
new file mode 100644
index 000000000000..54c341d5c3d8
--- /dev/null
+++ b/doc/release/upcoming_changes/15685.new_feature.rst
@@ -0,0 +1,7 @@
+``subok`` option for `np.copy`
+------------------------------
+A new kwarg, ``subok``, was added to `np.copy` to allow users to toggle the 
+behavior of `np.copy` with respect to array subclasses. The default value
+is ``False`` which is consistent with the behavior of `np.copy` for 
+previous numpy versions. To create a copy that preserves an array subclass with
+`np.copy`, call ``np.copy(arr, subok=True)``.
diff --git a/numpy/lib/function_base.py b/numpy/lib/function_base.py
index 12320ae4795a..cfee406b9056 100644
--- a/numpy/lib/function_base.py
+++ b/numpy/lib/function_base.py
@@ -708,12 +708,12 @@ def select(condlist, choicelist, default=0):
     return result
 
 
-def _copy_dispatcher(a, order=None):
+def _copy_dispatcher(a, order=None, subok=None):
     return (a,)
 
 
 @array_function_dispatch(_copy_dispatcher)
-def copy(a, order='K'):
+def copy(a, order='K', subok=False):
     """
     Return an array copy of the given object.
 
@@ -728,12 +728,21 @@ def copy(a, order='K'):
         as possible. (Note that this function and :meth:`ndarray.copy` are very
         similar, but have different default values for their order=
         arguments.)
+    subok : bool, optional
+        If True, then sub-classes will be passed-through, otherwise the
+        returned array will be forced to be a base-class array (default).
+
+        .. versionadded:: 1.19.0
 
     Returns
     -------
     arr : ndarray
         Array interpretation of `a`.
 
+    See Also
+    --------
+    ndarray.copy : Preferred method for creating an array copy
+
     Notes
     -----
     This is equivalent to:
@@ -757,7 +766,7 @@ def copy(a, order='K'):
     False
 
     """
-    return array(a, order=order, copy=True)
+    return array(a, order=order, subok=subok, copy=True)
 
 # Basic operations
 
diff --git a/numpy/lib/tests/test_function_base.py b/numpy/lib/tests/test_function_base.py
index 7953de15d837..d727b881714b 100644
--- a/numpy/lib/tests/test_function_base.py
+++ b/numpy/lib/tests/test_function_base.py
@@ -273,6 +273,13 @@ def test_order(self):
         assert_(not a_fort_copy.flags.c_contiguous)
         assert_(a_fort_copy.flags.f_contiguous)
 
+    def test_subok(self):
+        mx = ma.ones(5)
+        assert_(not ma.isMaskedArray(np.copy(mx, subok=False)))
+        assert_(ma.isMaskedArray(np.copy(mx, subok=True)))
+        # Default behavior
+        assert_(not ma.isMaskedArray(np.copy(mx)))
+
 
 class TestAverage:
 


Code review comments:
diff_hunk: @@ -0,0 +1,7 @@
+``subok`` option for `np.copy`
+------------------------------
+A new kwarg, ``subok``, was added to `np.copy` to allow users to toggle the 
path: doc/release/upcoming_changes/15685.new_feature.rst
text: @user1:
```suggestion
A new kwarg, ``subok``, was added to `numpy.copy` to allow users to toggle the 
```

Links don't work if they start with `np` instead of `numpy`.