-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
BUG: eval and query not working with ea dtypes #50764
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
Merged
Merged
Changes from all commits
Commits
Show all changes
27 commits
Select commit
Hold shift + click to select a range
244a2fe
BUG: eval and query not working with ea dtypes
phofl 5a66970
Fix windows build
phofl 2fee569
Fix
phofl 3142077
Merge branch 'main' into eval
phofl f01d45f
Merge branch 'main' into eval
phofl 7c56d2e
Merge remote-tracking branch 'upstream/main' into eval
phofl c010ca9
Fix another bug
phofl 778d043
Merge branch 'main' into eval
phofl 8a3d8cb
Merge branch 'main' into eval
phofl 5e2327e
Fix eval
phofl cef04a3
Merge branch 'main' into eval
phofl fddee23
Fix
phofl a23523b
Fix
phofl 5b73792
Merge branch 'main' into eval
phofl 467127f
Add arrow tests
phofl 9022e2e
Fix pyarrow-less ci
phofl 89fe7fd
Merge branch 'main' into eval
phofl 04d9a94
Merge branch 'main' into eval
phofl 5268569
Add try except
phofl 2fdb1d0
Merge branch 'main' into eval
phofl 9f75603
Add warning
phofl b756073
Adjust warning
phofl 931c723
Fix warning
phofl 4fb7a39
Fix
phofl a0d8120
Merge branch 'main' into eval
phofl 1a2c36a
Update test_query_eval.py
phofl 73d72ed
Update pandas/core/computation/eval.py
mroeschke File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Does this mean if a user is specifying
engine="numexpr"
, this switches the engine topython
?If so, I think it would be better to explicitly raise a
NotImplementedError
and tell the user to switch the engine manually.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.
Yep, would you be ok with a RuntimeWarning? Since numexpr is the default, raising seems a bit noisy maybe?
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.
Well the issues linked already raise an exception currently, so a
NotImplementedError
would be similar but more explicit. Unless there's a case I'm not thinking of.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 am more concerned with people using our nullable option who are switching over from NumPy. Would be annoying if query/eval stop working then.
But you are correct, an appropriate NotImplementedError would be an improvement
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.
That's fair. Yeah a
RuntimeWarning
warning & encouraging users to switch to the python engine would be okayThere 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.
Added the warning