diff --git a/docs/src/whatsnew/latest.rst b/docs/src/whatsnew/latest.rst index 825508dca3..d4d5cb499a 100644 --- a/docs/src/whatsnew/latest.rst +++ b/docs/src/whatsnew/latest.rst @@ -36,7 +36,10 @@ This document explains the changes made to Iris for this release 🐛 Bugs Fixed ============= -#. N/A +#. :user:`gaoflow` made :class:`iris.Constraint` raise a ``TypeError`` when used + in a boolean context (e.g. with the ``and``/``or``/``not`` keywords) instead + of silently discarding one of the constraints. Use the ``&`` operator to + combine constraints. (:issue:`4337`) 💣 Incompatible Changes diff --git a/lib/iris/_constraints.py b/lib/iris/_constraints.py index c0ba2f120f..d3d682e89e 100644 --- a/lib/iris/_constraints.py +++ b/lib/iris/_constraints.py @@ -218,6 +218,18 @@ def __and__(self, other): def __rand__(self, other): return ConstraintCombination(other, self, operator.__and__) + def __bool__(self): + # Constraints have no truth value: combining them with the Python + # keywords ``and``/``or``/``not`` (which call bool()) silently returns + # one of the operands instead of a combined Constraint, losing the + # other. Raise an explanatory error so this is not a silent failure; + # use the ``&`` operator to combine constraints. See #4337. + raise TypeError( + "The truth value of a Constraint is ambiguous. Constraints cannot " + "be combined with the 'and', 'or' and 'not' keywords; use the '&' " + "operator instead, e.g. 'constraint1 & constraint2'." + ) + class ConstraintCombination(Constraint): """Represents the binary combination of two Constraint instances.""" diff --git a/lib/iris/tests/unit/constraints/test_Constraint__bool__.py b/lib/iris/tests/unit/constraints/test_Constraint__bool__.py new file mode 100644 index 0000000000..5b48de32eb --- /dev/null +++ b/lib/iris/tests/unit/constraints/test_Constraint__bool__.py @@ -0,0 +1,68 @@ +# Copyright Iris contributors +# +# This file is part of Iris and is released under the BSD license. +# See LICENSE in the root of the repository for full licensing details. +"""Unit tests for :meth:`iris._constraints.Constraint.__bool__`.""" + +import operator + +import pytest + +from iris._constraints import ( + AttributeConstraint, + Constraint, + ConstraintCombination, + NameConstraint, +) + + +class Test_Constraint__bool__: + # Using a Constraint in a boolean context (e.g. with the ``and``/``or``/ + # ``not`` keywords) used to silently return one of the operands, quietly + # discarding the other. It should instead raise an informative TypeError + # so the user is directed towards the ``&`` operator (see #4337). + + _match = "truth value of a Constraint is ambiguous" + + def test_bool(self): + with pytest.raises(TypeError, match=self._match): + bool(Constraint("air_temperature")) + + def test_keyword_or(self): + c1 = Constraint("air_temperature") + c2 = Constraint("time") + with pytest.raises(TypeError, match=self._match): + c1 or c2 + + def test_keyword_and(self): + c1 = Constraint("air_temperature") + c2 = Constraint("time") + with pytest.raises(TypeError, match=self._match): + c1 and c2 + + def test_keyword_not(self): + with pytest.raises(TypeError, match=self._match): + not Constraint("air_temperature") + + def test_constraint_combination(self): + combination = ConstraintCombination( + Constraint("air_temperature"), + Constraint("time"), + operator.__and__, + ) + with pytest.raises(TypeError, match=self._match): + bool(combination) + + def test_attribute_constraint(self): + with pytest.raises(TypeError, match=self._match): + bool(AttributeConstraint(STASH="m01s00i024")) + + def test_name_constraint(self): + with pytest.raises(TypeError, match=self._match): + bool(NameConstraint(standard_name="air_temperature")) + + def test_and_operator_still_works(self): + # The supported way of combining constraints must be unaffected. + c1 = Constraint("air_temperature") + c2 = Constraint("time") + assert isinstance(c1 & c2, ConstraintCombination)