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

BUG:change formatting of assert_array_almost_equal #4541

Merged
merged 7 commits into from
Mar 26, 2014

Conversation

ElDeveloper
Copy link
Contributor

assert_array_almost_equal now prints the number of digits that were
being compared in the test instead of always printing 8 (the default
for array_repr). This would lead to uninformattive error messages.

Fixes #2367

@charris
Copy link
Member

charris commented Mar 24, 2014

LGTM. Needs some tests, probably in numpy/testing/tests/test_utils.py.

@ElDeveloper
Copy link
Contributor Author

Sounds good to me, when I checked I couldn't find test cases for any of the three functions (build_err_msg, assert_array_equal and assert_array_compare) that are being modified, should I add tests for each of them? I think it only makes sense to add them for build_err_msg but I would be interested in hearing what others think.

@charris
Copy link
Member

charris commented Mar 25, 2014

Some areas of Numpy are under tested. I'd say add a test build_err_msg at a minimum, and ideally the functions that call it. Looks like assert_array_almost_equal is another one. Writing tests is often more work than fixing the bug, but in this case cut and paste will probably help. The tests serve to define what the functions should do.

assert_array_almost_equal now prints the number of digits that were
being compared in the test instead of always printing 8 (the default
for array_repr). This would lead to uninformattive error messages.

Fixes numpy#2367
@ElDeveloper
Copy link
Contributor Author

@charris that sounds good to me, I'll add those for build_err_msg and assert_array_almost_equal.

@ElDeveloper
Copy link
Contributor Author

On a second look, I think only build_err_msg needed the tests (which I've added and I think the travis build passed). I cannot see what test cases I should add for the other functions. Do you have any pointers?

@charris
Copy link
Member

charris commented Mar 25, 2014

build_err_msg is probably enough, they look pretty thorough. What I had in mind for the other functions was just making sure they called that function with the proper precision argument.

For the tests themselves, it might be easier to parse out the numbers and just compare those rather than the complete strings. Could maybe have a small function to do that.

@ElDeveloper
Copy link
Contributor Author

Ah, now I get it, thanks! I'll try to add those.

This check is needed now that build_err_msg takes a precision argument,
which is only relevant if the things being compared are ndarrays.
assert_approx_equal & assert_almost_equal make internal use of
build_err_msg so a few tests have been added to check that the errors
are formatted correctly.
It's silly to check scalars when their string representations will vary
so much from system to system.
The tests were looking at the formatting of other elements in the string
which is not relevant for the things testsed in this case. By looking
only at the array formatting, we guarantee consistency across systems.
@ElDeveloper
Copy link
Contributor Author

@charris I've added some other tests and added a check in build_err_msg to check if it's a scalar or an ndarray. Let me know what else is needed.

charris added a commit that referenced this pull request Mar 26, 2014
BUG:change formatting of assert_array_almost_equal
@charris charris merged commit d35d5c1 into numpy:master Mar 26, 2014
@charris
Copy link
Member

charris commented Mar 26, 2014

Merged, thanks @ElDeveloper.

@ElDeveloper
Copy link
Contributor Author

Thanks for the pointers @charris.

@juliantaylor
Copy link
Contributor

thanks, the now fixed behavior has always bothered me when looking at test failures logs due to rounding issues

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

assert_array_almost_equal prints low precision (Trac #1774)
3 participants