8000 BUG: fix percentage reporting when testing.assert_allclose fails. by pp-mo · Pull Request #5025 · numpy/numpy · GitHub
[go: up one dir, main page]
Skip to content

BUG: fix percentage reporting when testing.assert_allclose fails. #5025

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Sep 2, 2014
Merged

BUG: fix percentage reporting when testing.assert_allclose fails. #5025

merged 2 commits into from
Sep 2, 2014

Conversation

pp-mo
Copy link
Contributor
@pp-mo pp-mo commented Aug 31, 2014

Aims to address : #2912

I separated the inner calculation of 'allclose' into a separate routine for assert_allclose to use it.
This now conforms to the expected behaviour of 'comparison' in assert_array_compare.
Governing concepts:

  • subsidiary routine is private, to avoid extending the api with more routines to support
  • assuming ultimate efficiency is not a big deal for this, so extra function call is ok
  • in an ideal world, this function would be simply 'isclose', but I think that equivalence can't now be assured, and we must leave legacy routines as they are.

@njsmith
Copy link
Member
njsmith commented Aug 31, 2014

I've never found those percentage things terribly useful myself, but hey, why not. Extra function call is not even an issue, if we cared about optimizing this function there much more important targets (e.g. all the big array copies it makes). LGTM except that you have a bunch of test failures...

@pp-mo pp-mo force-pushed the assert_allclose_percent branch from 5df1969 to 3910377 Compare September 1, 2014 16:37
@pp-mo
Copy link
Contributor Author
pp-mo commented Sep 1, 2014

Sorry about the mess, I don't have ability to quickly test against different versions (just yet).
Please hang on while I try to sort something suitable for both python 2.6 and 3.x ...

@pp-mo pp-mo force-pushed the assert_allclose_percent branch from 3910377 to ea32c90 Compare September 1, 2014 23:42
@pp-mo
Copy link
Contributor Author
pp-mo commented Sep 1, 2014

I don't have ability to quickly test against different versions

And clearly, I didn't understand the compatibility issues...
Several respins later, it finally passes.

Thanks @njsmith
Please re-review ?

@njsmith
Copy link
Member
njsmith commented Sep 2, 2014

Can someone else pick this up? It'd be appreciated - I'm travelling for the
next few days.
On 2 Sep 2014 00:57, "Patrick Peglar" notifications@github.com wrote:

I don't have ability to quickly test against different versions

And clearly, I didn't understand the compatibility issues...
Several respins later, it finally passes.

Thanks @njsmith https://github.com/njsmith
Please re-review ?

—
Reply to this email directly or view it on GitHub
#5025 (comment).

@juliantaylor
Copy link
Contributor

looks good, thanks

juliantaylor added a commit that referenced this pull request Sep 2, 2014
@juliantaylor
Merge pull request #5025 from pp-mo/assert_allclose_percent
4a501a0 
BUG: fix percentage reporting when testing.assert_allclose fails.
@juliantaylor juliantaylor merged commit 4a501a0 into numpy:master Sep 2, 2014
@pp-mo
Copy link
Contributor Author
pp-mo commented Sep 4, 2014

Thanks @juliantaylor @njsmith
Useful learning experience.

@jorisvandenbossche jorisvandenbossche mentioned this pull request Mar 24, 2015
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
component: numpy.testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@pp-mo @njsmith @juliantaylor @rgommers
0