Skip to content

API: remove deep keyword from EA.copy #27083

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jun 27, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions doc/source/whatsnew/v0.25.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -804,6 +804,7 @@ ExtensionArray

- Bug in :func:`factorize` when passing an ``ExtensionArray`` with a custom ``na_sentinel`` (:issue:`25696`).
- :meth:`Series.count` miscounts NA values in ExtensionArrays (:issue:`26835`)
- Keyword argument ``deep`` has been removed from :method:`ExtensionArray.copy` (:issue:`27083`)

Other
^^^^^
Expand Down
7 changes: 1 addition & 6 deletions pandas/core/arrays/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -820,15 +820,10 @@ def take(self, indices, allow_fill=False, fill_value=None):
# pandas.api.extensions.take
raise AbstractMethodError(self)

def copy(self, deep: bool = False) -> ABCExtensionArray:
def copy(self) -> ABCExtensionArray:
"""
Return a copy of the array.

Parameters
----------
deep : bool, default False
Also copy the underlying data backing this array.

Returns
-------
ExtensionArray
Expand Down
2 changes: 1 addition & 1 deletion pandas/core/arrays/datetimelike.py
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,7 @@ def _concat_same_type(cls, to_concat):
values = np.concatenate([x.asi8 for x in to_concat])
return cls(values, dtype=dtype)

def copy(self, deep=False):
def copy(self):
values = self.asi8.copy()
return type(self)._simple_new(values, dtype=self.dtype, freq=self.freq)

Expand Down
11 changes: 3 additions & 8 deletions pandas/core/arrays/integer.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import copy
import sys
from typing import Type
import warnings
Expand Down Expand Up @@ -375,14 +374,10 @@ def take(self, indexer, allow_fill=False, fill_value=None):

return type(self)(result, mask, copy=False)

def copy(self, deep=False):
def copy(self):
data, mask = self._data, self._mask
if deep:
data = copy.deepcopy(data)
mask = copy.deepcopy(mask)
else:
data = data.copy()
mask = mask.copy()
data = data.copy()
mask = mask.copy()
return type(self)(data, mask, copy=False)

def __setitem__(self, key, value):
Expand Down
11 changes: 3 additions & 8 deletions pandas/core/arrays/interval.py
Original file line number Diff line number Diff line change
Expand Up @@ -680,21 +680,16 @@ def _shallow_copy(self, left=None, right=None, closed=None):
return self._simple_new(
left, right, closed=closed, verify_integrity=False)

def copy(self, deep=False):
def copy(self):
"""
Return a copy of the array.

Parameters
----------
deep : bool, default False
Also copy the underlying data backing this array.

Returns
-------
IntervalArray
"""
left = self.left.copy(deep=True) if deep else self.left
right = self.right.copy(deep=True) if deep else self.right
left = self.left.copy(deep=True)
right = self.right.copy(deep=True)
closed = self.closed
# TODO: Could skip verify_integrity here.
return type(self).from_arrays(left, right, closed=closed)
Expand Down
2 changes: 1 addition & 1 deletion pandas/core/arrays/numpy_.py
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ def take(self, indices, allow_fill=False, fill_value=None):
fill_value=fill_value)
return type(self)(result)

def copy(self, deep=False):
def copy(self):
return type(self)(self._ndarray.copy())

def _values_for_argsort(self):
Expand Down
8 changes: 2 additions & 6 deletions pandas/core/arrays/sparse.py
Original file line number Diff line number Diff line change
Expand Up @@ -1262,12 +1262,8 @@ def searchsorted(self, v, side="left", sorter=None):
v, side, sorter
)

def copy(self, deep=False):
if deep:
values = self.sp_values.copy()
else:
values = self.sp_values

def copy(self):
values = self.sp_values.copy()
return self._simple_new(values, self.sp_index, self.dtype)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TomAugspurger do we need to copy the index here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really sure. I don't know if they're designed for sharing across instances, or whether we mutate them inplace.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In any case in sparse.py, we don't mutate them inplace. I would also assume it is fine not to copy them.


@classmethod
Expand Down
4 changes: 3 additions & 1 deletion pandas/core/indexes/interval.py
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,9 @@ def __reduce__(self):

@Appender(_index_shared_docs['copy'])
def copy(self, deep=False, name=None):
array = self._data.copy(deep=deep)
array = self._data
if deep:
array = array.copy()
attributes = self._get_attributes_dict()
if name is not None:
attributes.update(name=name)
Expand Down
2 changes: 1 addition & 1 deletion pandas/core/internals/blocks.py
Original file line number Diff line number Diff line change
Expand Up @@ -2212,7 +2212,7 @@ def copy(self, deep=True):
""" copy constructor """
values = self.values
if deep:
values = values.copy(deep=True)
values = values.copy()
return self.make_block_same_class(values)

def get_values(self, dtype=None):
Expand Down
4 changes: 3 additions & 1 deletion pandas/core/internals/construction.py
Original file line number Diff line number Diff line change
Expand Up @@ -199,8 +199,10 @@ def init_dict(data, index, columns, dtype=None):
arrays = (com.maybe_iterable_to_list(data[k]) for k in keys)
# GH#24096 need copy to be deep for datetime64tz case
# TODO: See if we can avoid these copies
arrays = [arr if not isinstance(arr, ABCIndexClass) else arr._data
for arr in arrays]
arrays = [arr if not is_datetime64tz_dtype(arr) else
arr.copy(deep=True) for arr in arrays]
arr.copy() for arr in arrays]
return arrays_to_mgr(arrays, data_names, index, columns, dtype=dtype)


Expand Down
4 changes: 3 additions & 1 deletion pandas/core/sparse/series.py
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,9 @@ def copy(self, deep=True):
"""
# TODO: https://github.com/pandas-dev/pandas/issues/22314
# We skip the block manager till that is resolved.
new_data = self.values.copy(deep=deep)
new_data = self.values
if deep:
new_data = new_data.copy()
return self._constructor(new_data, sparse_index=self.sp_index,
fill_value=self.fill_value,
index=self.index.copy(),
Expand Down
6 changes: 3 additions & 3 deletions pandas/tests/arrays/sparse/test_array.py
Original file line number Diff line number Diff line change
Expand Up @@ -591,9 +591,9 @@ def test_set_fill_invalid_non_scalar(self, val):
with pytest.raises(ValueError, match=msg):
arr.fill_value = val

def test_copy_shallow(self):
arr2 = self.arr.copy(deep=False)
assert arr2.sp_values is self.arr.sp_values
def test_copy(self):
arr2 = self.arr.copy()
assert arr2.sp_values is not self.arr.sp_values
assert arr2.sp_index is self.arr.sp_index

def test_values_asarray(self):
Expand Down
7 changes: 2 additions & 5 deletions pandas/tests/extension/arrow/bool.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,11 +108,8 @@ def take(self, indices, allow_fill=False, fill_value=None):
allow_fill=allow_fill)
return self._from_sequence(result, dtype=self.dtype)

def copy(self, deep=False):
if deep:
return type(self)(copy.deepcopy(self._data))
else:
return type(self)(copy.copy(self._data))
def copy(self):
return type(self)(copy.copy(self._data))

@classmethod
def _concat_same_type(cls, to_concat):
Expand Down
9 changes: 6 additions & 3 deletions pandas/tests/extension/arrow/test_bool.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ def dtype():

@pytest.fixture
def data():
return ArrowBoolArray.from_scalars(np.random.randint(0, 2, size=100,
dtype=bool))
values = np.random.randint(0, 2, size=100, dtype=bool)
values[1] = ~values[0]
return ArrowBoolArray.from_scalars(values)


@pytest.fixture
Expand All @@ -36,7 +37,9 @@ def test_array_type_with_arg(self, data, dtype):


class TestInterface(BaseArrowTests, base.BaseInterfaceTests):
pass
def test_copy(self, data):
# __setitem__ does not work, so we only have a smoke-test
data.copy()


class TestConstructors(BaseArrowTests, base.BaseConstructorsTests):
Expand Down
8 changes: 8 additions & 0 deletions pandas/tests/extension/base/interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,11 @@ def test_isna_extension_array(self, data_missing):
assert not na.all()

assert na.dtype._is_boolean

def test_copy(self, data):
# GH#27083 removing deep keyword from EA.copy
assert data[0] != data[1]
result = data.copy()

data[1] = data[0]
assert result[1] != result[0]
2 changes: 1 addition & 1 deletion pandas/tests/extension/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def data():
"""Length-100 array for this type.

* data[0] and data[1] should both be non missing
* data[0] and data[1] should not gbe equal
* data[0] and data[1] should not be equal
"""
raise NotImplementedError

Expand Down
6 changes: 2 additions & 4 deletions pandas/tests/extension/decimal/array.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,8 @@ def take(self, indexer, allow_fill=False, fill_value=None):
allow_fill=allow_fill)
return self._from_sequence(result)

def copy(self, deep=False):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add a test in the pandas/tests/extension/constructors.py so it hits all EA's (for view & copy)

if deep:
return type(self)(self._data.copy())
return type(self)(self)
def copy(self):
return type(self)(self._data.copy())

def astype(self, dtype, copy=True):
if isinstance(dtype, type(self.dtype)):
Expand Down
2 changes: 1 addition & 1 deletion pandas/tests/extension/json/array.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ def take(self, indexer, allow_fill=False, fill_value=None):

return self._from_sequence(output)

def copy(self, deep=False):
def copy(self):
return type(self)(self.data[:])

def astype(self, dtype, copy=True):
Expand Down
4 changes: 4 additions & 0 deletions pandas/tests/extension/test_sparse.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,10 @@ class TestInterface(BaseSparseTests, base.BaseInterfaceTests):
def test_no_values_attribute(self, data):
pytest.skip("We have values")

def test_copy(self, data):
# __setitem__ does not work, so we only have a smoke-test
data.copy()


class TestConstructors(BaseSparseTests, base.BaseConstructorsTests):
pass
Expand Down