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

Swapped all numpy assert_allclose & assert_array_equal to pytest approx in tests #780

Open
wants to merge 17 commits into
base: develop
Choose a base branch
from

Conversation

gohil-jay
Copy link
Collaborator

Addresses 2c81964 commit further.

I realized that there are other asserts that are imported directly which I missed in previous commit, so this covers all of them.

@github-actions github-actions bot added the needs changelog Might need a changelog entry label Aug 15, 2022
@@ -793,7 +792,7 @@ def test_getitem(self, ref, growth):
def test_iter(self, ref, growth):
Cat = bh.axis.StrCategory if isinstance(ref[0], str) else bh.axis.IntCategory
a = Cat(ref, growth=growth)
assert_array_equal(a, ref)
assert a == approx(ref)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assert a == approx(ref)
assert np.asarray(a) == approx(ref)

Quick fix. But is it possible to control how pytest treats approx?

@HDembinski
Copy link
Member

Why do you replace the numpy assserts with pytest.approx? The messages produced by numpy.assert_allclose etc are better than those from pytest.approx, and numpy.assert_allclose works on array-like types, too.

@gohil-jay
Copy link
Collaborator Author

I believe this came up while working on the comparison issue but I don't remember the exact details on why! @henryiii do you want to chime in here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs changelog Might need a changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants