-
-
Notifications
You must be signed in to change notification settings - Fork 19.3k
Fix #12564 for Categorical: consistent result if comparing as DataFrame #12698
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
pandas/core/internals.py
Outdated
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 don't think you need all of these extra checks which are block specific. does it break when you take them out?
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.
x.ndim < self.ndim
ifxis a normal object,_block_shapewill increasex.ndimso thatValueError: Wrong number of dimensionsraises. Thus I checkx.ndim < self.ndim.getattr(self, 'is_categorical', None)
Because we expect thatx.ndim < self.ndimshould only occur inNonConsolidatableMixInclass, so if it is not the case, I believe that it might be a bug.
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.
these are too specific here for this type of general routine. you might want to do this in _try_coerce_results for only Categoricals.
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.
Ok, I will take a look at _try_coerce_results. Thanks for your advice.
|
pls add a whatsnew note in Bug Fixes as well (for 0.18.1) |
|
ok, I'll add a whatsnew note by tomorrow. |
pandas/core/internals.py
Outdated
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.
you don't need this
|
Thanks for your advice, @jreback. |
|
@ningchi if you could that'd be great. It's probably just a conflict in the whatsnew file. And if your comfortable you can squash your commits down to a nice single message too. |
|
yes, pls rebase |
|
many thanks! I have rebased these commits. |
|
thanks @ningchi |
git diff upstream/master | flake8 --diff