-
-
Notifications
You must be signed in to change notification settings - Fork 19k
BUG: String[pyarrow] comparison with mixed object #62424
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
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.
Looks good!
Didn't add a whatsnew note since I'm not sure if the bug is in a released version cc @jorisvandenbossche ?
It's also fixing a bug in the existing nullable string dtype, and we are generally noting bug fixes for the new str
dtype as well in the 2.3.x whatsnew. I am planning to backport this, so you can add a note there.
(if it would only have been for the new dtype and targetting 3.0, it would indeed not need a whatsnew note)
arr.searchsorted(b) | ||
|
||
|
||
def test_mixed_object_comparison(dtype): |
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.
Maybe move this to pandas/tests/arrays/string_/test_string.py
? (in general this file only contains the generic extension tests, not string-specific ones)
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.
(although it is not actually that string specific, generally all of our dtypes should do this comparison to mixed object dtype? So we could also make this a base extension test. But let's do that later, that might involve more fixes in other (test) EAs which doesn't need to be backported)
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 started a branch this morning that adds a tests.arithmetic.test_string file. if this is merged, i'll move this test in that branch too
Are we good here? I'd like to follow up with the branch mentioned about refactoring string arithmetic tests. |
Thanks @jbrockmendel! |
Owee, I'm MrMeeseeks, Look at me. There seem to be a conflict, please backport manually. Here are approximate instructions:
And apply the correct labels and milestones. Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon! Remember to remove the If these instructions are inaccurate, feel free to suggest an improvement. |
Co-authored-by: Joris Van den Bossche <[email protected]>
Manual backport -> #62504 |
…62424) (#62504) Co-authored-by: jbrockmendel <[email protected]>
doc/source/whatsnew/vX.X.X.rst
file if fixing a bug or adding a new feature.Didn't add a whatsnew note since I'm not sure if the bug is in a released version cc @jorisvandenbossche ?