Document deep parameter in ExtensionArray.copy

[TomAugspurger]

And pass it through via Series.copy.

diff --git a/pandas/tests/extension/decimal/array.py b/pandas/tests/extension/decimal/array.py
index 108b8874b..7085a231b 100644
--- a/pandas/tests/extension/decimal/array.py
+++ b/pandas/tests/extension/decimal/array.py
@@ -78,6 +78,7 @@ class DecimalArray(ExtensionArray, ExtensionScalarOpsMixin):
         return self._from_sequence(result)
 
     def copy(self, deep=False):
+        print(deep)
         if deep:
             return type(self)(self._data.copy())
         return type(self)(self)

In [1]: import pandas as pd; import numpy as np;

In [2]: from pandas.tests.extension.decimal.array import *

In [3]: s = pd.Series(DecimalArray([decimal.Decimal(1)]))

In [4]: s.copy(deep=False)
Out[4]:
0    1
dtype: decimal

In [5]: s.copy(deep=True)
False
Out[5]:
0    1
dtype: decimal

[TomAugspurger]

We should also clarify what exactly a deep vs. shallow copy should mean...

My use-case is SparseArray.copy. Should the underlying data sp_index and sp_values be copied only if deep=True (that's what we do on master)?

[jorisvandenbossche]

Should the underlying data sp_index and sp_values be copied only if deep=True (that's what we do on master)?

I suppose so.

We should also clarify what exactly a deep vs. shallow copy should mean...

The docstring of pd.Series.copy has a quite detailed explanation:

When deep=True (default), a new object will be created with a
copy of the calling object's data and indices. Modifications to
the data or indices of the copy will not be reflected in the
original object (see notes below).

When deep=False, a new object will be created without copying
the calling object's data or index (only references to the data
and index are copied). Any changes to the data of the original
will be reflected in the shallow copy (and vice versa).

(with of course the additional caveats regarding python objects not being copied recursively and index data not being copied).

For ExtensionArray it is of course not necessarily that straightforward, since there is no fixed "data" for all EAs. But I think typically an EA will have some kind of _data (or multiple objects), i.e. the actual stored data, so we can expect that to be copied with deep=True.
In practice, with deep=True we expect that doing array[0] = other_value does not change a deep copy of array, but does change a shallow copy of array.

[h-vetinari]

Like in #19862, I would argue that:

IMO, copy(deep=True) should completely sever all connections between the original and the copied object - compare the official python docs (https://docs.python.org/3/library/copy.html):

A deep copy constructs a new compound object and then, recursively, inserts copies into it of the objects found in the original.

[jorisvandenbossche]

@h-vetinari I think the other discussion is not fully relevant here, as an ExtensionArray has (in general) no notion of 'index' that is immutable and not copied like a Series/DataFrame has.

[TomAugspurger]

This is also not tested directly in the base extension tests. Testing the semantics will be a tad, since we don't have a no-copy way of getting some kind of ExtensionArray._data.

[jorisvandenbossche]

We could test the expected "user semantics" ?

a = EA(..)
a2 = a.copy(deep=True)
a[0] = a[1]
assert a2[0] != a[1]

a = EA(..)
a2 = a.copy(deep=False)
a[0] = a[1]
assert a2[0] == a[1]

[TomAugspurger]

Ah, yeah that will work for mutable EAs (my head is in sparse-land right now, which is not mutable).

[jorisvandenbossche]

Ah, OK, I now understand why you asked the initial question. In that case (SparseArray being immutable), then it is again more similar to the "copy index of Series or not" discussion.

Is there a reason they are immutable? (it's just more work to implement it? Scipy sparse matrices are mutable, but raise a performance warning if the mutation changes the sparsity structure)

[TomAugspurger]

Just more work to implement I think.

…

On Mon, Aug 13, 2018 at 11:09 AM Joris Van den Bossche < ***@***.***> wrote: Ah, OK, I now understand why you asked the initial question. In that case (SparseArray being immutable), then it is again more similar to the "copy index of Series or not" discussion. Is there a reason they are immutable? (it's just more work to implement it? Scipy sparse matrices are mutable, but raise a performance warning if the mutation changes the sparsity structure) — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#22314 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABQHIvAqtktzLAH_SQ-L0iR9BbNf2TWwks5uQaTUgaJpZM4V6fuW> .

[jorisvandenbossche]

Coming back to my comment above #22314 (comment) about expected user semantics.
I am not sure about that anymore, because I would now think to expect that any copy() call would make sure that modifying an array does not change a copy of it as well.

Another option is to simply remove the deep keyword for EAs (just like np.copy has no deep keyword)

[jbrockmendel]

Closed by #27083.