Raise TypeError when a Constraint is used in a boolean context (#4337)#7121
Open
gaoflow wants to merge 1 commit into
Open
Raise TypeError when a Constraint is used in a boolean context (#4337)#7121gaoflow wants to merge 1 commit into
gaoflow wants to merge 1 commit into
Conversation
Combining constraints with the Python 'and'/'or'/'not' keywords silently returned one of the operands and discarded the other, because Constraint had no __bool__ and so was always truthy. Add a __bool__ that raises an informative TypeError directing users to the '&' operator, mirroring the way numpy rejects the truth-testing of multi-element arrays. As agreed on the issue, this is not a breaking change: any code it stops was already relying on an anti-pattern that produced wrong results. Fixes SciTools#4337.
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.
🚀 Pull Request
Description
Closes #4337.
A :class:
iris.Constrainthas no truth value, but it also had no__bool__, so Python treated it as always-truthy. That meant combining constraints with theand/or/notkeywords failed silently — the expression evaluated to one operand and quietly discarded the other:So
cube.extract(c1 or c2)would silently ignorec2, which is exactly the support query that prompted this issue.Approach
As agreed by @SciTools/peloton on the issue ("the
Constraintclass should include the appropriate override to prevent it being used in boolean statements ... to raiseTypeError. A unit test for this error should also be added"), this addsConstraint.__bool__raising an informativeTypeErrorthat points users at the supported&operator — mirroring how numpy rejects truth-testing of multi-element arrays:Because
__bool__lives on the baseConstraint, it also coversConstraintCombination, :class:~iris.AttributeConstraintand :class:~iris.NameConstraint.Per the issue discussion this is not considered a breaking change: any code it stops was already relying on an anti-pattern that produced wrong results, and the docs already direct users to
&.Verification
tests/unit/constraints/test_Constraint__bool__.pycoveringbool(), theand/or/notkeywords,ConstraintCombination/AttributeConstraint/NameConstraint(all raise), and a control asserting&still returns aConstraintCombination. The raising cases fail onmainand pass here.tests/unit/constraints/+tests/test_constraints.py(67 passed) and the PP/netCDF constraint-conversion + cubeextracttests all pass, confirming no internal code relied on constraint truthiness.ruff check/ruff formatclean.