-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
ENH: add sort_categories argument to union_categoricals #13846
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
Conversation
new_codes = np.concatenate([c.codes for c in to_union]) | ||
|
||
if sort_categories: | ||
categories = categories.sort_values() |
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 sort can be skipped if categories is monotonic-increasing.
Current coverage is 85.28% (diff: 100%)@@ master #13846 diff @@
==========================================
Files 140 139 -1
Lines 50455 50020 -435
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
- Hits 43014 42659 -355
+ Misses 7441 7361 -80
Partials 0 0
|
# GH 13763 | ||
c1 = Categorical(['x', 'y', 'z']) | ||
c2 = Categorical(['a', 'b', 'c']) | ||
result = union_categoricals([c1, c2], sort_categories=True) |
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.
can you put companion tests with sort_categories=False
can you add this issue to the whatsnew (just add on to same line where |
lgtm. @sinhrks pls have a look. |
lgtm. pls fix the lint issue:) |
@sinhrks - updated, thanks! |
thanks! |
git diff upstream/master | flake8 --diff
cc: @JanSchulz, @sinhrks