-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
API: DataFrame.take always returns a copy #30784
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
Changes from 6 commits
e532bc0
2c9af9e
8c17091
d9e75f8
e8e5cbb
6c023d2
d15639d
11e597d
e519485
b2afdf2
f10bd49
d3684f7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -756,7 +756,14 @@ def axes(self) -> List[Index]: | |
# Indexing Methods | ||
|
||
@Appender(generic.NDFrame.take.__doc__) | ||
def take(self, indices, axis=0, is_copy=False, **kwargs) -> "Series": | ||
def take(self, indices, axis=0, is_copy=None, **kwargs) -> "Series": | ||
if is_copy is not None: | ||
warnings.warn( | ||
"is_copy is deprecated and will be removed in a future version. " | ||
"'take' always returns a copy, so there is no need to specify this.", | ||
FutureWarning, | ||
stacklevel=2, | ||
) | ||
nv.validate_take(tuple(), kwargs) | ||
|
||
indices = ensure_platform_int(indices) | ||
|
@@ -771,16 +778,20 @@ def take(self, indices, axis=0, is_copy=False, **kwargs) -> "Series": | |
kwargs = {} | ||
new_values = self._values.take(indices, **kwargs) | ||
|
||
result = self._constructor( | ||
return self._constructor( | ||
new_values, index=new_index, fastpath=True | ||
).__finalize__(self) | ||
|
||
# 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) | ||
def _take_with_is_copy(self, indices, axis=0, **kwargs): | ||
""" | ||
Internal version of the `take` method that sets the `_is_copy` | ||
attribute to keep track of the parent dataframe (using in indexing | ||
for the SettingWithCopyWarning). For Series this does the same | ||
as the public take (it never sets `_is_copy`). | ||
|
||
return result | ||
See the docstring of `take` for full explanation of the parameters. | ||
""" | ||
return self.take(indices=indices, axis=axis, **kwargs) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am including a I am not fully sure if the code paths that use |
||
|
||
def _ixs(self, i: int, axis: int = 0): | ||
""" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure if its relevant, but DTI.take sometimes returns a view
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the case the indices can represent a slice, right?
Personally I don't think we should make that optimization, as IMO it is much clearer for take to always be a copy, but I suppose for Index which is supposed to be immutable it might matter less.