-
-
Notifications
You must be signed in to change notification settings - Fork 18.1k
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
DOC: Improve sort parameter doc of DataFrameGroupBy.value_counts #59412
DOC: Improve sort parameter doc of DataFrameGroupBy.value_counts #59412
Conversation
Seems like I have changed the wrong |
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.
Thanks for the PR!
@@ -2303,7 +2303,7 @@ def value_counts( | |||
normalize : bool, default False | |||
Return proportions rather than frequencies. | |||
sort : bool, default True | |||
Sort by frequencies. | |||
Sort by frequencies when True. Sort by DataFrame column values when False. |
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.
I think we also want to modify the docstring on SeriesGroupBy.value_counts
. Can you make the same adjustment there.
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.
Sure thing
…oupBy-value_counts-sort-parameter
@rhshadrach - are there any extra changes needed for this PR? Thanks |
@KevsterAmp I left an additional ? on the parent issue: #59307 (comment) It seems like when the groupby's sort flag and value_counts sort flag are both false, no sorting is done. Is this a behavioral bug, or just something else to add to documentation? edit: It seems also that when
In the input frame, the row |
Not sure if I will address this in this PR, since the scope of this one is to just modify the doc. Thanks |
@rhshadrach - are there any extra changes needed for this PR? Thanks |
@KevsterAmp - I am hesitant to make this change since certain cases that are correct now become incorrect. When you do |
@KevsterAmp - I've opened #59307 based on the discussion in the linked issue. Unfortunately that supersedes this PR. Going to close here - but thanks for working on this! |
doc/source/whatsnew/vX.X.X.rst
file if fixing a bug or adding a new feature.I just copied the sort docstring of
Dataframe.value_counts
since the latter is mirrored to behaved like it.