-
-
Notifications
You must be signed in to change notification settings - Fork 18.2k
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
test backportability of #38120 #38137
test backportability of #38120 #38137
Conversation
|
||
uniques = Index(uniques) | ||
return codes, uniques | ||
|
||
if is_extension_array_dtype(values.dtype): | ||
values = extract_array(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.
there was a merge conflict here, and should have removed this line
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.
when the current ci has completed, will remove this and check not needed.
pandas/core/algorithms.py
Outdated
@@ -652,8 +662,13 @@ def factorize( | |||
# responsible only for factorization. All data coercion, sorting and boxing | |||
# should happen here. | |||
|
|||
if isinstance(values, ABCRangeIndex): |
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.
this was moved in the PR but not on 1.1.x so may need to remove
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.
RangeIndex.factorize was added in #38034, so this causes a recursion error, removing.
@jbrockmendel the test faiures are for PeriodArray/Index. There must be some differences on master on how this is handled. will continue to investigate. |
so it appears the class hierarchy has changed. I think the docstring for DatelikeOps, 'Common ops for DatetimeIndex/PeriodIndex, but not TimedeltaIndex.' and TimelikeOps is now confusing. I'm not sure why the merge put the new implementation of factorize in so have kept it in |
#38120