10000 testing/utils: assert_almost_equal: use the same algorithm for rounding by ignatenkobrain · Pull Request #6601 · numpy/numpy · GitHub
[go: up one dir, main page]

Skip to content

testing/utils: assert_almost_equal: use the same algorithm for rounding #6601

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

Closed

Conversation

ignatenkobrain
Copy link

as used for assert_array_almost_equal

Reference: #5200

as used for assert_array_almost_equal

Reference: numpy#5200
Signed-off-by: Igor Gnatenko <i.gnatenko.brain@gmail.com>
@@ -509,7 +509,7 @@ def _build_err_msg():
return
except (NotImplementedError, TypeError):
pass
if round(abs(desired - actual), decimal) != 0:
if around(abs(desired - actual), decimal) >= 10.0**(-decimal):
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand this logic. Either just round, or compare the absolute difference with some value.

@mhvk
Copy link
Contributor
mhvk commented Nov 1, 2015

I had missed the original discussion, but this seems wrong. Personally, I would simply write this as

if abs(diff) >= 10**(-decimal):
    ...

This should be identical to

if round(abs(diff)) != 0:
    ...

But one shouldn't combine them.

p.s. Looking back, @charris also noted that he preferred the "first version" which just has the rounding.

@charris
Copy link
Member
charris commented Dec 10, 2015

@ignatenkobrain Need to get this finished up.

@charris charris added this to the 1.12.0 release milestone Apr 16, 2016
@charris charris added the 56 - Needs Release Note. Needs an entry in doc/release/upcoming_changes label Apr 16, 2016
@charris charris added the 52 - Inactive Pending author response label Jun 18, 2016
@charris charris closed this Jun 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
00 - Bug 52 - Inactive Pending author response 56 - Needs Release Note. Needs an entry in doc/release/upcoming_changes component: numpy.testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0