BUG: Raise error when expr does not evaluate to bool in df.query

[NumberPiOso]

• closes df.query() doesn't follow python truthy-ness? #8560
• Tests added and passed if fixing a bug or adding a new feature
• All code checks passed.
• Added an entry in the latest doc/source/whatsnew/v1.5.0.rst file if fixing a bug or adding a new feature.

[NumberPiOso]

Special attention to line 4139 in

pandas/pandas/core/frame.py

Lines 4137 to 4141 in b8e1aa4

	if not is_bool_dtype(res):
	# Special condition to check when dealing with higher dimensions
	if not (res.ndim > 1 and (res.dtypes == bool).all()):
	msg = f"expr must evaluate to boolean not {res.dtypes}"
	raise ValueError(msg)

This condition was added in order to pass some special test that used higher dimensions. These tests were

pandas/tests/frame/test_query_eval.py::TestDataFrameQueryNumExprPandas::test_nested_scope
pandas/tests/frame/test_query_eval.py::TestDataFrameQueryPythonPandas::test_nested_scope

This is because these tests produce a res that evaluates to false in first condition (line 4137)

res = 0 1 2 0 True False False 1 False False False 2 False False True 3 True True False 4 False True True

So in this case we check for column to be bool type

[NumberPiOso]

I am not completely sure if this generalizes to higher dimensions or I can fix the condition to res.ndim == 2 for improved readibility

[WillAyd]

We only offer up to 2 dimensional objects in pandas so should be fine, though there also might be a more explicit way with typing to branch here. @simonjayhawkins

[jreback]

you don't need any of list, you can call common.is_bool_indexer

[NumberPiOso]

@jreback
It correctly raises when they are not bool. But we go back to problems with tests
pandas/tests/frame/test_query_eval.py::TestDataFrameQueryNumExprPandas::test_nested_scope
pandas/tests/frame/test_query_eval.py::TestDataFrameQueryPythonPandas::test_nested_scope

The first one produces an output like the following res variable.

import pandas as pd from pandas.core.dtypes.common import is_bool_dtype from pandas.core import common as com res = pd.DataFrame([ [False, False, False], [False, True, False], [False, False, False], [False, False, False], [False, False, False], ]) if res.ndim == 1: is_bool_result = is_bool_dtype(res) elif res.ndim > 1: is_bool_result = all(is_bool_dtype(x) for x in res.dtypes) new_is_bool_result = com.is_bool_indexer(res) # is_bool_result: True # new_is_bool: False

[jreback]

ok when how about we use it then handle those cases. I really do not like re-inventing the wheel.

[rhshadrach]

The only cases is_bool_indexer handles are Series, ndarray, Index, and lists. I don't see anything in dtypes or common that handles DataFrames. I'm not sure if adding DataFrame to is_bool_indexer with its current uses is safe. Perhaps we could add is_bool_frame? The implementation above appears to be correct to me.

[NumberPiOso]

With the new implementation that takes into account #46862 (comment) I feel we need this if/else.

In general, it seems logical to me that these cases are separated.

But if you do not agree, I want to understand a little more the proposed modification. Is this the implementation you imagined?:

 if not is_bool_frame(res): msg = f"expr must evaluate to boolean not {res.dtypes}" raise ValueError(msg) try: result = self.loc[res] except ValueError: # when res is multi-dimensional loc raises, but this is sometimes a # valid query result = self[res]

[jreback]

you don't need any of list, you can call common.is_bool_indexer

[simonjayhawkins]

out of curiosity, as not looked at this in detail, but if we are ensuring a bool indexer, is the following try/except still needed?

[NumberPiOso]

Indeed it is a common problem with the multidimensional res, so we can share the solution.

[NumberPiOso]

Sorry for the long delay, I had a couple of hard weeks. But now on I can and will iterate a lot quicker.

[github-actions]

This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this.

[mroeschke]

Thanks for the pull request, but it appears to have gone stale. If interested in continuing, please merge in the main branch, address any review comments and/or failing tests, and we can reopen.