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.

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

Replace usage of np.allclose in testing with assert_allclose #380

Open
davidorme opened this issue Dec 18, 2024 · 1 comment · May be fixed by #382
Open

Replace usage of np.allclose in testing with assert_allclose #380

davidorme opened this issue Dec 18, 2024 · 1 comment · May be fixed by #382
Assignees
Labels
bug Something isn't working

Comments

@davidorme
Copy link
Collaborator

I was expecting these two commands to be equivalent:

In [3]: import numpy as np
   ...: from numpy.testing import assert_allclose
   ...: a = np.array([[2,2]])
   ...: b = np.array([[2,2], [2,2]])
   ...: a.shape == b.shape
   ...: False
   ...: assert np.allclose(a, b)  # DOES NOT RAISE!
   ...: assert_allclose(a, b)
   ...: 
---------------------------------------------------------------------------
AssertionError                            Traceback (most recent call last)
...

(shapes (1, 2), (2, 2) mismatch)
 ACTUAL: array([[2, 2]])
 DESIRED: array([[2, 2],
       [2, 2]])

Turns out np.allclose broadcasts the inputs before testing, so this does not guarantee the inputs are the correct shape. This is a good reason to prefer np.testing.assert_allclose(a, b) over assert np.allclose(a, b). I can sort of see why it does it - the broadcasting is such a fundamental part of numpy that it happens first? - but it is counterintuitive.

I've used np.allclose a lot in the tests. I doubt we've got any issues, but it's tempting fate.

@davidorme davidorme added the bug Something isn't working label Dec 18, 2024
@davidorme davidorme changed the title Replace usage of np.allclose in testing with `test_ Replace usage of np.allclose in testing with assert_allclose Dec 18, 2024
@davidorme davidorme linked a pull request Jan 6, 2025 that will close this issue
10 tasks
@davidorme davidorme linked a pull request Jan 6, 2025 that will close this issue
10 tasks
@davidorme davidorme moved this from Todo to In Progress in ICCS development board Jan 6, 2025
@davidorme
Copy link
Collaborator Author

I doubt we've got any issues, but it's tempting fate.

It was tempting fate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: In Progress
Development

Successfully merging a pull request may close this issue.

1 participant