Reject sequences nested in a sequence of a different type in approx (closes #11945) - #14860
Open
Kayvan-Zahiri wants to merge 1 commit into
Open
Reject sequences nested in a sequence of a different type in approx (closes #11945)#14860Kayvan-Zahiri wants to merge 1 commit into
Kayvan-Zahiri wants to merge 1 commit into
Conversation
ApproxSequenceLike guarded against nesting with isinstance(x, type(expected)), which only caught an element of the same type as the container. A tuple inside a list, or a list inside a tuple, passed the guard and was handed to ApproxScalar, where it was compared exactly instead of approximately. That returned a wrong bool rather than raising, so [(1.20000000000001,)] == approx([(1.2,)]) was False while the flat (1.20000000000001,) == approx((1.2,)) was True. Use _is_sequence_like, the same predicate approx() uses to dispatch to ApproxSequenceLike, so nesting is rejected consistently. Mappings are excluded so that a dict nested in a sequence keeps its current behaviour. Closes pytest-dev#11945
Author
|
One thing I should have put in the description: I used AI tooling while working How it was used: to help navigate I have left the |
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.
Closes #11945.
The bug
ApproxSequenceLike.__init__rejects nesting with:That only catches an element of the same type as its container. A tuple inside a
list, or a list inside a tuple, passes the guard, reaches
ApproxScalar, and iscompared with
==. So the comparison is exact, not approximate, and no error israised:
The identical-value case is the quieter half of it, because it looks like it works:
Those return
Truefrom an exact comparison, so the tolerance is silently ignored.A test written that way passes for the wrong reason and starts failing the moment a
value drifts by one ulp.
@crazymerlyn localised this in the issue.
The fix
Use
_is_sequence_like, the predicateapprox()itself uses to decide whether todispatch to
ApproxSequenceLike. The guard and the dispatcher then agree on what asequence is.
Mappings are excluded deliberately.
_is_sequence_likeis true fordict, since adict has
__getitem__and__len__, so using it unqualified would newly reject[{"x": 1.0}] == approx([{"x": 1.0}]), which works today.approx()avoids that bychecking
Mappingbefore_is_sequence_like; the guard needs the same exclusion.A dict nested in a sequence still gets the old exact comparison, and I have left
that alone since it is outside this issue. Happy to address it here or separately
if you would like it changed.
Behaviour change worth flagging
[np.array([1.0, 2.0])] == approx([np.array([1.0, 2.0])])raisedValueError("truth value of an array is ambiguous") on
mainand now raisesTypeErrorwiththe nested-structure message. Both are errors, and the new one names the actual
problem, but it is a different exception type.
Everything else I checked is unchanged: flat lists and tuples,
["string"],[{"x": 1.0}], a top-level numpy array, and same-type nesting.Tests
Two cases added to the existing
test_expected_value_type_errorparametrisation(
tuple-in-list,list-in-tuple), plustest_nested_mixed_sequence_not_silently_comparedwhich pins that these raise rather than returning a bool.
All four fail on
main; the two pre-existing parametrisations pass either way.testing/python/approx.py: 140 passed.