Skip to content
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

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

Closed
wants to merge 2 commits into from
Closed

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

Show file tree
Hide file tree
Changes from all commits
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

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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

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

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

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

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

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

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
3 changes: 2 additions & 1 deletion pandas/tests/frame/test_alter_axes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -678,8 +678,9 @@ def test_rename_inplace(self, float_frame):

c_id = id(float_frame['C'])
float_frame = float_frame.copy()
float_frame.rename(columns={'C': 'foo'}, inplace=True)
result = float_frame.rename(columns={'C': 'foo'}, inplace=True)

assert result is None
assert 'C' not in float_frame
assert 'foo' in float_frame
assert id(float_frame['foo']) != c_id
Expand Down