BUG: allow BoolOp, In, and NotIn under DataFrame.query/eval parser='python' - #66560
Draft
bzjpst wants to merge 1 commit into
Draft
BUG: allow BoolOp, In, and NotIn under DataFrame.query/eval parser='python'#66560bzjpst wants to merge 1 commit into
bzjpst wants to merge 1 commit into
Conversation
BaseExprVisitor implements visit_BoolOp and routes In/NotIn through visit_Compare / _rewrite_membership_op / _maybe_evaluate_binop, but the @disallow set on PythonExprVisitor was blocking those AST nodes from ever reaching the base. Carve BoolOp/In/NotIn out of the disallow set; Dict and Not remain disallowed. Adjust tests that previously asserted those nodes raise NotImplementedError to assert correct results instead. MultiIndex queries and boolean/membership expressions now work under parser='python' as they already did under parser='pandas'.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Background
DataFrame.query()andDataFrame.eval()underparser="python"unconditionally raiseNotImplementedErrorwhen an expression contains aBoolOp(and/or),In, orNotInAST node. In practice this means:and/orcombined expression:a > 0 and b > 0→NotImplementedError: 'BoolOp' nodes are not implementeda in b→'In' nodes are not implementeda < b < c(Python lowers this toBoolOp(And, [a<b, b<c])) → samecol == "x"— the parser rewritesEqtoInwhen either side is a string, so it also failsdf.query(...)with aMultiIndexfails on almost anything non-numeric, even when the level name resolves fineThe same expressions work under
parser="pandas".The block came from an overly conservative
@disallow(...)set onPythonExprVisitor.BaseExprVisitoralready implementsvisit_BoolOp(pandas/core/computation/expr.py:763) and routesIn/NotInthroughvisit_Compare→_rewrite_membership_op→_maybe_evaluate_binop(which pushes"in"/"not in"through_maybe_evalin Python space).PandasExprVisitoropts back into those nodes;PythonExprVisitornever did. There is no visitor-level obstacle — only the decorator.Change
Carve
BoolOp,In,NotInout of the disallow set onPythonExprVisitor.DictandNotremain disallowed — they are still legitimately unimplemented for that visitor path.No changes to
PandasExprVisitor, the numexpr/python engines, or any resolver code.parser="pandas"is entirely unaffected.Test-suite fallout (fixed here)
The old restriction was codified in many tests as
pytest.raises(NotImplementedError, match="'BoolOp'...")blocks,pytest.raises(NotImplementedError, match="'(Not)?In'...")blocks, andskip_if_no_pandas_parser(parser)guards onTestDataFrameQueryWithMultiIndex. Those assertions are now wrong. This PR:skip_if_no_pandas_parserfrom tests that pass identically under both parsers: everyTestDataFrameQueryWithMultiIndex.*,test_chained_cmp_and_in,test_local_variable_with_in,test_at_inside_string,test_query_with_nested_strings,test_query_with_nested_special_character. (Kept the skip on tests that still legitimately needparser="pandas"for@-prefix locals / backtick quoting /df.attraccess.)if parser == "pandas": <assert> else: pytest.raises(...)splits intest_str_query_method,test_str_list_query_method,test_query_with_string_columns,test_simple_bool_ops,test_bool_ops_with_constants,test_simple_cmp_ops,test_complex_cmp_ops,test_chained_cmp_op,test_simple_in_ops,test_compound_invert_op,test_date_index_query_with_NaT_duplicates— both branches now assert the same correct result.'Not' → NotImplementedErrorassertion intest_fails_and_or_not(Not remains disallowed).Net: -167/+58 lines across the two test files.
Verification
Against
pandas/tests/frame/test_query_eval.py+pandas/tests/computation/test_eval.py:11,442 passed · 0 failed · 17 skipped · 43 xfailed · 2 xpassed
The 17 skips are the
@-prefix locals / backtick /df.attributetests that legitimately still requireparser='pandas'. Prior-existing xfailed/xpassed counts unchanged.Notes
v3.1.0.rstunder "Other".Signed: Model B