fix: approx() tolerance not applied to sequences/mappings nested inside dicts#14498
Open
Ashutosh-177 wants to merge 4 commits into
Open
fix: approx() tolerance not applied to sequences/mappings nested inside dicts#14498Ashutosh-177 wants to merge 4 commits into
Ashutosh-177 wants to merge 4 commits into
Conversation
When approx() is called on a dict whose values are themselves lists or dicts, _approx_scalar() was returning ApproxScalar for those values, which does exact equality. The top-level approx() dispatcher correctly routes sequences to ApproxSequenceLike and mappings to ApproxMapping, but _approx_scalar() -- called per-element by ApproxMapping.__eq__ -- missed those cases. Adding the same isinstance checks in _approx_scalar means nested structures get recursive approximate comparison instead of falling back to ==. Fixes pytest-dev#8703 Signed-off-by: Ashutosh Kumar Singh <ahutoshhjp1067@gmail.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Extends _approx_scalar in python_api.py to dispatch Mapping and sequence-like values to ApproxMapping and ApproxSequenceLike respectively, enabling nested approximate comparisons within scalar dispatch.
Changes:
- Add
Mappingbranch to returnApproxMapping. - Add sequence-like branch to return
ApproxSequenceLike.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Member
|
Hello, thank you for working on pytest @Ashutosh-177, it seems your second commit canceled the first, so there's nothing to review. |
Ashutosh-177
force-pushed
the
fix/approx-nested-sequences
branch
from
3ada952 to
b613c79
Compare
Signed-off-by: Ashutosh Kumar Singh <ahutoshhjp1067@gmail.com>
psf-chronographer
Bot
added
the
bot:chronographer:provided
Covers the Mapping branch in _approx_scalar() to reach 100% diff coverage. Signed-off-by: Ashutosh Kumar Singh <ahutoshhjp1067@gmail.com>
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.
Running into an issue where
pytest.approxworks fine at the top level but silently falls back to exact equality when the same sequence or mapping appears as a dict value.For example:
This fails even though the tolerance should cover the difference. The top-level
approx()call dispatches toApproxMapping, which iterates over the dict values and calls_approx_scalar()on each one. But_approx_scalar()only checks forDecimalanddatetime/timedelta— everything else, including lists and nested dicts, falls through toApproxScalar, which does exact comparison.The top-level
approx()dispatcher already has the correctisinstancechecks forMappingand sequence-like types. Adding the same two checks to_approx_scalar()makes nested structures recurse properly.Fixes #8703