TST: follow numpy/pytest traceback conventions in simple assert_* test helper functions - #16080

Not sure to understand the benefit. If a test fail I will want to know why and hence dive into this function to find which attribute and examine the value, so why hiding it ?

I tried to explain the expected gain in #16071
I could furthermore add that:

• in a sense, this satisfies the principle of least surprise (as a user of numpy.testing.assert_* functions, I grew to expect the internals of such functions to be hidden in tracebacks)
• you can still opt-in the current behaviour using --full-trace, while the behaviour this is enabling is not achievable currently

I personally would be more surprised by not being able to know why I had a failure. The traceback would still go through the failure within the unit test. Often I end up having to bisect issues by examining these helper functions and making temporary modifications to them.

If we do go through with this change I suggest we carefully consider the error messages being raised so that they are more indicative of what has caused the failure.

Yes but being useful for numpy.testing.assert_ doesn't mean it's always better. Here the code is specific to what is being tested and used by a few tests only, and there is a good chance that I will want to see the full traceback is there a problem.
Also the problem is that I don't know what to expect with __tracebackhide__ activated, what will pytest print ?

As you'll have seen from my comment above, I agree with @saimn here - I can see representations as being a single object where one wants to hide the traceback, since their content is uniform, but not a more complicated object where one also compares attributes.

If we do go through with this change I suggest we carefully consider the error messages being raised so that they are more indicative of what has caused the failure.

Also the problem is that I don't know what to expect with tracebackhide activated, what will pytest print ?

In short: pytest prints the exact same message as it would without __tracebackhide__, the difference is in the line that is associated with that message.

To illustrate let's take my t.py test module from #16071 (copied here in a dropdown for convenience)

# t.py
import numpy as np

def assert_equal_len_v1(x, y):
    assert len(x) == len(y)

def assert_equal_len_v2(x, y):
    __tracebackhide__ = True
    assert len(x) == len(y)

def test_v1():
    a = np.arange(2)
    b = np.arange(3)
    assert_equal_len_v1(a, b)

def test_v2():
    a = np.arange(2)
    b = np.arange(3)
    assert_equal_len_v2(a, b)

Here's the output

$ pytest t.py
====================== test session starts ======================
platform darwin -- Python 3.12.0, pytest-8.0.0, pluggy-1.4.0
...
t.py FF                                                   [100%]

=========================== FAILURES ============================
____________________________ test_v1 ____________________________

    def test_v1():
        a = np.arange(2)
        b = np.arange(3)
>       assert_equal_len_v1(a, b)

t.py:14:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

x = array([0, 1]), y = array([0, 1, 2])

    def assert_equal_len_v1(x, y):
>       assert len(x) == len(y)
E       assert 2 == 3
E        +  where 2 = len(array([0, 1]))
E        +  and   3 = len(array([0, 1, 2]))

t.py:5: AssertionError
____________________________ test_v2 ____________________________

    def test_v2():
        a = np.arange(2)
        b = np.arange(3)
>       assert_equal_len_v2(a, b)
E       assert 2 == 3
E        +  where 2 = len(array([0, 1]))
E        +  and   3 = len(array([0, 1, 2]))

t.py:19: AssertionError
==================== short test summary info ====================
FAILED t.py::test_v1 - assert 2 == 3
FAILED t.py::test_v2 - assert 2 == 3
======================= 2 failed in 0.06s =======================

Hmm, really these should just be replaced! But can be for follow-up...

Right. I can do that as a follow up ! Meanwhile, I'll avoid touching them here

There's an open issue for this already: #15782

These are being removed already in #15839, but in most instances the corresponding numpy.testing functions would be used; so if I understood you correctly, the numpy versions would already take care of setting __tracebackhide__? A couple uses of assert_equal would be changed to simple asserts though.

thanks @eerovaher , I wasn't aware of this issue. Since it's mark with "good first issue" I should leave it to new contributors, right ?

if I understood you correctly, the numpy versions would already take care of setting __tracebackhide__?

Correct ! For instance, see the first line in numpy.testing.assert_equal's definition https://github.com/numpy/numpy/blob/e920ca42d98c4911c2294784f8809b38735923a4/numpy/testing/_private/utils.py#L313

Simple and non-critical tasks such as removing these functions are indeed best left for new contributors.

thanks @eerovaher , I wasn't aware of this issue. Since it's mark with "good first issue" I should leave it to new contributors, right ?

It is all but done in #15839 already, but your comments would be very welcome there. Pretty much all that remains to be done is settling on where to best use numpy.testing.assert_equal() vs. assert a == b. And if this PR is being merged first, might be worth looking if the new asserts in #15839 should get their __tracebackhide__ as well.

Thank you, I'll make sure to review this one tomorrow !

As you'll have seen from my comment above, I agree with @saimn here - I can see representations as being a single object where one wants to hide the traceback, since their content is uniform, but not a more complicated object where one also compares attributes.