Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

approx representation of details failed when using a ApproxSequenceLike which is not list or tuple #11797

Open
thorink opened this issue Jan 10, 2024 · 7 comments · May be fixed by #11818
Open

approx representation of details failed when using a ApproxSequenceLike which is not list or tuple #11797

thorink opened this issue Jan 10, 2024 · 7 comments · May be fixed by #11818
Labels
topic: approx Related to pytest.approx function

Comments

@thorink
Copy link

thorink commented Jan 10, 2024

Description

When using a custom sequence like type then the comparison of pytest.approx works as expected if the assert is True. But in case the assertion fails, the detailed output of what went wrong is broken.

Given the following example:

import pytest

class MyVec3:
    def __init__(self, x, y, z):
        self._x, self._y, self._z = x, y, z

    def __len__(self):
        return 3

    def __getitem__(self, key):
        if key == 0:
            return self._x
        if key == 1:
            return self._y
        if key == 2:
            return self._z
        raise IndexError


def test_vec_approx():
    assert MyVec3(0, 1, 2) == pytest.approx(MyVec3(1, 2, 3), abs=2)
    assert MyVec3(0, 1, 2) == pytest.approx(MyVec3(1, 2, 3), abs=0.5)

The first assert statement is fine, as expected.

In the second assert fails, as expected.

But I get this output:

============================================ test session starts ============================================
platform linux -- Python 3.10.12, pytest-7.4.4, pluggy-1.3.0
rootdir: /home/user
collected 1 item                                                                                            

test_approx.py F                                                                                      [100%]

================================================= FAILURES ==================================================
______________________________________________ test_vec_approx ______________________________________________

    def test_vec_approx():
        assert MyVec3(0, 1, 2) == pytest.approx(MyVec3(1, 2, 3), abs=2)
>       assert MyVec3(0, 1, 2) == pytest.approx(MyVec3(1, 2, 3), abs=0.5)
E       AssertionError: assert <test_approx....x7f657f208880> == approx([1 ± 5... 3 ± 5.0e-01])
E         (pytest_assertion plugin: representation of details failed: /home/user/venv/lib/python3.10/site-packages/_pytest/python_api.py:342: TypeError: object of type 'ApproxScalar' has no len().
E          Probably an object has a faulty __repr__.)

test_approx.py:22: AssertionError
========================================== short test summary info ==========================================
FAILED test_approx.py::test_vec_approx - AssertionError: assert <test_approx....x7f657f208880> == approx([1 ± 5... 3 ± 5.0e-01])
============================================= 1 failed in 0.01s =============================================

Setup

Package        Version
-------------- -------
exceptiongroup 1.2.0
iniconfig      2.0.0
packaging      23.2
pip            22.0.2
pluggy         1.3.0
pytest         7.4.4
setuptools     59.6.0
tomli          2.0.1

Also happens with pytest version 7.1.2 and 7.1.3

OS is Ubuntu 22.04
@thorink
Copy link
Author

thorink commented Jan 10, 2024

I digged into it a bit. pytest.approx(MyVec3(1, 2, 3), abs=0.5) results in an object of type ApproxSequenceLike which sound right to me. This is the check that is responsible for that:

    elif (
        hasattr(expected, "__getitem__")
        and isinstance(expected, Sized)
        # Type ignored because the error is wrong -- not unreachable.
        and not isinstance(expected, STRING_TYPES)  # type: ignore[unreachable]
    ):
        cls = ApproxSequenceLike

When generating the detailed output the MyVec3 is wrapped in a ApproxScalar. When doing the same with a list [1,2,3], then it is transformed into [ApproxScalar(1), ApproxScalar(2), ApproxScalar(3)]. This happens in python_api.py::_recursive_sequence_map:

    if isinstance(x, (list, tuple)):
        seq_type = type(x)
        return seq_type(_recursive_sequence_map(f, xi) for xi in x)
    else:
        return f(x)

This isinstance(x, (list, tuple) is much more narrow as hasattr(expected, "__getitem__") and isinstance(expected, Sized) and not isinstance(expected, STRING_TYPES) from above.

@thorink
Copy link
Author

thorink commented Jan 10, 2024

Replacing _recursive_sequence_map with

def _recursive_sequence_map(f, x):
    """Recursively map a function over a sequence of arbitrary depth"""
    if (
        hasattr(x, "__getitem__")
        and isinstance(x, Sized)
        # Type ignored because the error is wrong -- not unreachable.
        and not isinstance(x, STRING_TYPES)  # type: ignore[unreachable]
    ):
        if isinstance(x, (list, tuple)):
            seq_type = type(x)
        else:
            seq_type = list
        return seq_type(_recursive_sequence_map(f, xi) for xi in x)
    else:
        return f(x)

would result in

    def test_vec_approx():
        assert MyVec3(0, 1, 2) == pytest.approx(MyVec3(1, 2, 3), abs=2)
>       assert MyVec3(0, 1, 2) == pytest.approx(MyVec3(1, 2, 3), abs=0.5)
E       assert <test_approx....x7f9c09e6ca60> == approx([1 ± 5... 3 ± 5.0e-01])
E         comparison failed. Mismatched elements: 3 / 3:
E         Max absolute difference: 1
E         Max relative difference: inf
E         Index | Obtained | Expected   
E         0     | 0        | 1 ± 5.0e-01
E         1     | 1        | 2 ± 5.0e-01
E         2     | 2        | 3 ± 5.0e-01

@RonnyPfannschmidt
Copy link
Member

Approx is not general,the support for lists as is, is already a stretch

Custom collection objects are not easily supportable

@thorink
Copy link
Author

thorink commented Jan 15, 2024

Hi @RonnyPfannschmidt, I see your point and I'm of course totally fine with not changing code for a stretch use case! We would then just implement our own MyVec approx type.

On the other hand, approx accepts as input (hasattr(x, "__getitem__") and isinstance(x, Sized)) and also works as expected in terms of the comparison it performs. My idea would be to just make sure the __repr__ of ApproxSequenceLike is on the same level.

If applying such a change would be considered, I would be happy to work on a proper MR.

@RonnyPfannschmidt
Copy link
Member

I need to do a little digging today,but it seems like a Good idea, I'll try to get it together soon

@RonnyPfannschmidt
Copy link
Member

i'm slightly hesitant to transform any custom collection into a list
however since its only happen on the internal part of the compare we might be fine

that being said, the rep traceback shouldn't happen as well, i'll try to figure the exact exception

@RonnyPfannschmidt
Copy link
Member 

self = approx([1 ± 5.0e-01, 2 ± 5.0e-01, 3 ± 5.0e-01])
other_side = <approx.TestApprox.test_strange_sequence.<locals>.MyVec3 object at 0x7f8237a32d50>

    def _repr_compare(self, other_side: Sequence[float]) -> List[str]:
        import math
    
        if len(self.expected) != len(other_side):
            return [
                "Impossible to compare lists with different sizes.",
                f"Lengths: {len(self.expected)} and {len(other_side)}",
            ]
    
        approx_side_as_map = _recursive_sequence_map(self._approx_scalar, self.expected)
    
>       number_of_elements = len(approx_side_as_map)
E       TypeError: object of type 'ApproxScalar' has no len()

../../src/_pytest/python_api.py:332: TypeError

i have a error indicating reproducer i'll extract a is_sequencelike and use that

RonnyPfannschmidt added a commit to RonnyPfannschmidt/pytest that referenced this issue Jan 15, 2024
@RonnyPfannschmidt
fix pytest-dev#11797: be more lenient on SequenceLike approx
2a16b61 
this needs a validation as it allows partially implemented sequences
@RonnyPfannschmidt RonnyPfannschmidt linked a pull request Jan 15, 2024 that will close this issue
Open
RonnyPfannschmidt added a commit to RonnyPfannschmidt/pytest that referenced this issue Jan 15, 2024
@RonnyPfannschmidt
fix pytest-dev#11797: be more lenient on SequenceLike approx
79efc62 
this needs a validation as it allows partially implemented sequences
@bluetech bluetech added the topic: approx Related to pytest.approx function label Feb 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
topic: approx Related to pytest.approx function
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix #11797: be more lenient on SequenceLike approx RonnyPfannschmidt/pytest
3 participants
@RonnyPfannschmidt @bluetech @thorink