-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Description
The current implementation of pytest.approx()
yields incorrect results when used to compare sets.
Consider the following self-explanatory code comparing two (equal as by __eq__()
) sets:
import numpy as np
import pytest
a = 2**np.arange(12)
b = 2**np.arange(12)
np.random.shuffle(a)
print(a)
print(b)
print(*set(a))
print(*set(b))
print(set(a) == set(b))
print(set(a) == pytest.approx(set(b)))
Although the two sets are obviously the same, the last equality check using approx is failing.
A quick view into the implementation of approx()
makes it obvious why this is the case:
class ApproxSequencelike(ApproxBase):
"""Perform approximate comparisons where the expected value is a sequence of numbers."""
def __repr__(self) -> str:
seq_type = type(self.expected)
if seq_type not in (tuple, list, set):
seq_type = list
return "approx({!r})".format(
seq_type(self._approx_scalar(x) for x in self.expected)
)
def _yield_comparisons(self, actual):
return zip(actual, self.expected)
In _yield_comparisons()
, only __iter__()
is used (in zip()
), but since sets are unordered, so is the resulting iterator. This means, for sets such an implementation cannot work.
What makes things worse is the confusion that seems to exist here between the different abstract base classes:
In the __repr__()
method, clearly set
is mentioned, explicitly. However, a set is not a sequence type, but only a collection type (because of the missing order). It is, however, iterable and since this is the only thing that is actually checked in the implementation, the code seems to work for sets, where, in fact, it does not. As a first step, I would suggest one could keep the current implementation, but explicitly check for sequence types (i.e. classes having a __getitem__()
method) and delete all mentions of set
in the code as well as on the documentation page and make it crystal clear that there is only an implementation for sequence types.
But what would be way better would, of course, be an implementation for arbitrary container comparisons.
Tested with pytest version 7.0.1.