DEPR: is_copy arg of take closes #27357 #28751
Conversation
| @@ -3372,6 +3372,8 @@ def take(self, indices, axis=0, is_copy=True, **kwargs): | |||
| selecting rows, ``1`` means that we are selecting columns. | |||
| is_copy : bool, default True | |||
| Whether to return a copy of the original object or not. | |||
|
|
|||
| .. deprecated:: 0.25.2 | |||
| FutureWarning, | ||
| stacklevel=2, | ||
| ) | ||
| is_copy = True |
There was a problem hiding this comment.
Is this assignment even required? Not used later in the function body right?
There was a problem hiding this comment.
There is a if is_copy: a few lines below
There was a problem hiding this comment.
But, we should only put it to True if it was None (to keep the existing behaviour), so put it in an else block ?
There was a problem hiding this comment.
It is used:
# Maybe set copy if we didn't actually change the index.
if is_copy:
if not result._get_axis(axis).equals(self._get_axis(axis)):
result._set_is_copy(self)
return resultBut is_copy is already deprecated. I think it needs to be removed for 1.0. Since this is going into 1.0 I am thinking now that instead of deprecating this parameter it needs to be removed all together, along with is_copy ?
| FutureWarning, | ||
| stacklevel=2, | ||
| ) | ||
| is_copy = True |
There was a problem hiding this comment.
But, we should only put it to True if it was None (to keep the existing behaviour), so put it in an else block ?
| @@ -3430,6 +3432,14 @@ class max_speed | |||
| 1 monkey mammal NaN | |||
| 3 lion mammal 80.5 | |||
| """ | |||
| if is_copy is not None: | |||
| warnings.warn( | |||
| "is_copy is deprecated and will be removed in a future version", | |||
There was a problem hiding this comment.
Can you add that 'take' will always return a copy in the future?
| @@ -800,6 +800,11 @@ def test_take_invalid_kwargs(self): | |||
| with pytest.raises(ValueError, match=msg): | |||
| obj.take(indices, mode="clip") | |||
|
|
|||
| def test_take_deprecated_kwarg_is_copy(self): | |||
| df = DataFrame([1, 2]) | |||
|
@chrisstpierre is this still active? If so can you merge master and repush? |
|
Closing as I think this is stale but @chrisstpierre ping if you'd like to pick back up |
black pandasgit diff upstream/master -u -- "*.py" | flake8 --diff