Skip to content

BUG: Fix groupby sorting on ordered Categoricals (GH25871) #25907

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

Closed

Conversation

kpflugshaupt
Copy link
Contributor

As documented in #25871, groupby() on an ordered Categorical messes up category order when 'observed=True' is specified.
Specifically, group labels will be ordered by first occurrence (as for an unordered Categorical), but grouped aggregation results will retain the Categorical's order.
The fix is a modified subset of #25173, which fixes a related case, but has not been merged yet.

kpflugshaupt and others added 4 commits March 26, 2019 17:55
As documented in pandas-dev#25871, groupby() on an ordered Categorical messes up category order when 'observed=True' is specified. 
Specifically, group labels will be ordered by first occurrence (as for an unordered Categorical), but grouped aggregation results will retain the Categorical's order.
The fix is a modified subset of pandas-dev#25173, which fixes a related case, but has not been merged yet.
Testing all combinations of:
- ordered vs. unordered grouping column
- 'observed' True vs. False
- 'sort' True vs. False
In all cases, result group ordering must be correct. 
The test is built such that the result index labels are equal to aggregation results if all goes well (except for the one unobserved category)
@pep8speaks
Copy link

Hello @kpflugshaupt! Thanks for opening this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 456:36: E241 multiple spaces after ','
Line 458:33: E241 multiple spaces after ','
Line 460:60: W291 trailing whitespace
Line 461:80: E501 line too long (118 > 79 characters)
Line 462:80: E501 line too long (109 > 79 characters)
Line 463:20: E211 whitespace before '('
Line 464:7: E221 multiple spaces before operator
Line 467:80: E501 line too long (80 > 79 characters)
Line 468:1: W293 blank line contains whitespace
Line 469:80: E501 line too long (82 > 79 characters)
Line 470:80: E501 line too long (82 > 79 characters)
Line 472:9: E221 multiple spaces before operator
Line 475:33: W291 trailing whitespace
Line 477:1: W293 blank line contains whitespace

@kpflugshaupt
Copy link
Contributor Author

Will fix the PEP 8 issues ASAP (sorry -- did not check the test source...)

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

Successfully merging this pull request may close these issues.

groupby aggregation on ordered Categorial with 'observed=True' breaks order
2 participants