Expanding support in testing for external collections.Sequence objects

See issue https://github.com/numpy/numpy/issues/22569.
The debate is whether np.testing.asset_equal should support collections.Sequence objects.
As you can see, a bug has been reported and an easy fix has been suggested.
What is the general view on adding support in testing for Sequence objects?

On Tue, Nov 29, 2022, at 07:21, info@markopacak.com wrote:
The debate is whether np.testing.asset_equal should support collections.Sequence objects.
assert list(sequence1) == list(sequence2)
should do the trick, and also handles int vs float and nan comparisons.
What is the general view on adding support in testing for Sequence objects?
If it's a straightforward change that doesn't impact the NumPy test suite, it's probably fine. Let's see what the others say.
Stéfan

On 30/11/22 05:47, Stefan van der Walt wrote:
On Tue, Nov 29, 2022, at 07:21, info@markopacak.com wrote:
The debate is whether np.testing.asset_equal should support collections.Sequence objects.
assert list(sequence1) == list(sequence2)
should do the trick, and also handles int vs float and nan comparisons.
What is the general view on adding support in testing for Sequence objects?
If it's a straightforward change that doesn't impact the NumPy test suite, it's probably fine. Let's see what the others say.
Stéfan
Apparently nothing is really straightforward. On the issue there is discussion of problems this causes when comparing strings (and how the naive implementation breaks lots of tests), and this comment [0] from a Pandas maintainer
If you do go down this path, take a look at
pandas._libs.lib.is_list_like. I've found it to be a minor PITA to accommodate every corner case that users ask to consider listlike
Matti
[0] https://github.com/numpy/numpy/issues/22569#issuecomment-1331586018

On Wed, Nov 30, 2022 at 10:24 AM Matti Picus matti.picus@gmail.com wrote:
On 30/11/22 05:47, Stefan van der Walt wrote:
On Tue, Nov 29, 2022, at 07:21, info@markopacak.com wrote:
The debate is whether np.testing.asset_equal should support collections.Sequence objects.
assert list(sequence1) == list(sequence2)
should do the trick, and also handles int vs float and nan comparisons.
What is the general view on adding support in testing for Sequence
objects?
If it's a straightforward change that doesn't impact the NumPy test
suite, it's probably fine. Let's see what the others say.
Stéfan
Apparently nothing is really straightforward. On the issue there is discussion of problems this causes when comparing strings (and how the naive implementation breaks lots of tests), and this comment [0] from a Pandas maintainer
If you do go down this path, take a look at
pandas._libs.lib.is_list_like. I've found it to be a minor PITA to accommodate every corner case that users ask to consider listlike
I agree with Matti, this tends to lead to quite a bit of work and dealing with corner cases, for little gain. We should move away from supporting non-array inputs, not allow more.
I'd prefer to tell users to use `np.asarray()` on their inputs instead.
Cheers, Ralf
Matti
[0] https://github.com/numpy/numpy/issues/22569#issuecomment-1331586018
NumPy-Discussion mailing list -- numpy-discussion@python.org To unsubscribe send an email to numpy-discussion-leave@python.org https://mail.python.org/mailman3/lists/numpy-discussion.python.org/ Member address: ralf.gommers@gmail.com

On Wed, Nov 30, 2022 at 7:10 PM Marko Pacak info@markopacak.com wrote:
Hi Ralf, thx for replying to this.
I'd prefer to tell users to use `np.asarray()` on their inputs instead.
How would you do that? Through a warning in the test suite? Or document it somewhere?
The docstring for array_equal should mention, for `actual` and `desired`, what input types are accepted. And the code itself should yield an informative error if an unsupported type is passed. That error message can be tested in the test suite if desired (not always done consistently, but it can't hurt).
Cheers, Ralf
participants (5)
-
info@markopacak.com
-
Marko Pacak
-
Matti Picus
-
Ralf Gommers
-
Stefan van der Walt