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.

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

DEPR: NDFrame.set_axis inplace defaults to false #27525 #27600

Merged
merged 5 commits into from Jul 26, 2019
Merged

DEPR: NDFrame.set_axis inplace defaults to false #27525 #27600

merged 5 commits into from Jul 26, 2019

Conversation

ghost
Copy link

@ghost ghost commented Jul 25, 2019

.. warning::
``inplace=None`` currently falls back to to True, but in a
future version, will default to False. Use inplace=True
explicitly rather than relying on the default.

has been there since #20164 #16994 (issue was #14636), part of 0.21.0.

With discussion of plans to deprecate similar functionality from set_index in #24046, it's time to make sure set_axis conforms to the rest of pandas in this.

Copy link
Member

@WillAyd WillAyd left a comment

Choose a reason for hiding this comment

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

looks pretty good minor comments

pandas/core/generic.py Outdated Show resolved Hide resolved
doc/source/whatsnew/v1.0.0.rst Outdated Show resolved Hide resolved
@WillAyd
Copy link
Member

WillAyd commented Jul 26, 2019

In case anyone is looking for it I think you meant to reference #16994 (the OP just points to a documentation update)

@WillAyd WillAyd added the Deprecate Functionality to remove in pandas label Jul 26, 2019
@WillAyd WillAyd added this to the 1.0 milestone Jul 26, 2019
Copy link
Member

@WillAyd WillAyd left a comment

Choose a reason for hiding this comment

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

Minor whatsnew quibbles but lgtm otherwise

doc/source/whatsnew/v1.0.0.rst Outdated Show resolved Hide resolved
doc/source/whatsnew/v1.0.0.rst Outdated Show resolved Hide resolved
@ghost
Copy link
Author

ghost commented Jul 26, 2019

Thanks for the review

@jreback jreback merged commit 619f52b into pandas-dev:master Jul 26, 2019
@jreback
Copy link
Contributor

jreback commented Jul 26, 2019

thanks @pilkibun

@ghost ghost deleted the set_axis_inplace_default2 branch July 26, 2019 14:30
@ghost
Copy link
Author

ghost commented Jul 26, 2019

cc @toobaz, you had some sentiments relating this to the set_index vs. set_axis debate in #16994 (comment) in relation to set_index vs. set_axis. I'm not what your position was, but It sounds like two years ago you were against both the little-endians and the big-endians ;)

... Can we haz some docs now?

@toobaz
Copy link
Member

toobaz commented Jul 26, 2019

@pilkibun I think I'm missing something, as I don't understand.

But related to the comment you link, I guess it is probably time to remove this warning I had introduced...

'set_axis now takes "labels" as first argument, and '

(and the subsequent reassignment).

@ghost
Copy link
Author

ghost commented Jul 26, 2019

@pilkibun I think I'm missing something, as I don't understand.

You objected to "publicizing" set_axis in the docs because of its defects and also that changing the default to inplace=False (this PR) is necessary before it can become officially recommended.

Now that that's done, I hope you'll push for making a decision on set_index vs. set_axis, so that we can finally document what the "official" method is.

@toobaz
Copy link
Member

toobaz commented Jul 27, 2019

OK, now I see where you're going.

The defects of set_axis are, as you saw, a thing of the past, and they are/were entirely unrelated to set_index. You can just go on with a nice docs PR "publicizing" set_axis if you like.

The API contains a lot of "official" methods to do a lot of different things.

Please stop provoking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Deprecate Functionality to remove in pandas
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants