Skip to content

DOC: Improve "inplace" param in Dataframe.rename() docs #24037

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

DOC: Improve "inplace" param in Dataframe.rename() docs #24037

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
38f2a80
DOC: Improve "inplace" param in Dataframe.rename() docs
nmusolino Dec 1, 2018
f273ef0
TST: Assert result of in-place rename is None
nmusolino Dec 1, 2018
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
DOC: Improve "inplace" param in Dataframe.rename() docs
  • Loading branch information
@nmusolino
nmusolino committed Dec 1, 2018
commit 38f2a8050dfd68eb0cb496c2604cdf57effbba52
4 changes: 2 additions & 2 deletions pandas/core/frame.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3909,8 +3909,8 @@ def rename(self, *args, **kwargs):
copy : boolean, default True
Also copy underlying data
inplace : boolean, default False
Whether to return a new DataFrame. If True then value of copy is
ignored.
If True, perform the operation in-place, modifying this DataFrame,
and return None.
Copy link
Member

@datapythonista datapythonista Dec 2, 2018

Choose a reason for hiding this comment

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

I don't think this is technically correct. The name is misleading, but using inplace doesn't necessarily mean that we're modifying the DataFrame, as we can be returning a copy.

I think the previous description is more accurate, except that it sounds like inplace=True will return a new DataFrame.
Copy link
Contributor Author

@nmusolino nmusolino Dec 2, 2018

Choose a reason for hiding this comment

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

Sorry, I don't understand your first paragraph. Can you give an example of when an operation with inplace=True doesn't modify "self", i.e. the data frame on which the method is called?

When inplace is True, the method should modify the data frame, and no second frame should be created. When inplace is False, the method should return a second data frame, and self should be unmodified.
Copy link
Member

@datapythonista datapythonista Dec 2, 2018

Choose a reason for hiding this comment

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

If you have a DataFrame named df, using the memory in the address 1000-2000, and you do df.operation(inplace=True), df will be modified, but the data can be now in the memory address 3000-4000.

So, technically speaking, the operation is not in place, as the result has been copied to a different location (even if the reference df is reassigned to the new memory).

That's why I don't like your description, as for a user who doesn't care about the memory is perfectly fine and correct. But if you think about the memory, you description is not really correct.
Copy link
Contributor

@jreback jreback Dec 2, 2018

Choose a reason for hiding this comment

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

@nmusolino there are about 2 inplace operations that actually operate inplace. all others, simply do an internal copy, do the op, then assign back to the top level data. inplace has never been a real operation in pandas, nor will it ever be. This is why we are deprecating it. You can certainly give a longer explanation, but discouraging usage would be good here.
Copy link
Member

@datapythonista datapythonista Dec 3, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@jreback are we ready to deprecate all inplace parameters? A quick grep shows around 60 methods with an inplace parameter, so it's not so much work to start raising warnings (and update the descriptions) for all them.

Or do we prefer to wait?
Copy link
Member

@gfyoung gfyoung Dec 3, 2018

Choose a reason for hiding this comment

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

@datapythonista :

See #16529 - I think we were planning to wait until 1.0, though can always push up.
Copy link
Member

@datapythonista datapythonista Dec 3, 2018

Choose a reason for hiding this comment

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

Thanks for pointing out the issue @gfyoung, I'll continue the conversation there.
Copy link
Contributor Author

@nmusolino nmusolino Dec 3, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@datapythonista, @jreback , I understand that these operations may involve copies. I wrote earlier that "internal data structures (e.g. block manager or index) might be copied even when inplace is True."

I think the distinction you are trying to draw is that a rename or fillna operation does not necessarily overwrite individual elements in place; is that right? As a simple example, I seem to recall that Index objects are generally immutable, so any operation that changes the index would necessarily replace the index.

Avoiding that implication is a reasonable point. To be clear, however, the methods are modifying the dataframe.

How about this description?

 inplace : boolean, default False If true, rename index labels in this DataFrame and return None, without creating a new DataFrame.

Or a simpler, more generic version:

 inplace : boolean, default False If true, modify this DataFrame and return None; does not create a new DataFrame.
level : int or level name, default None
In case of a MultiIndex, only rename labels in the specified
level.
Expand Down