Skip to content

ENH: Make allclose(x, y) equivalent to all(x == y) when both x and y are... #4108

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
wants to merge 1 commit into from

Conversation

abalkin
Copy link
Contributor

@abalkin abalkin commented Dec 6, 2013

... exact.

This improves performance and makes allclose() work on non-numeric dtypes.

Closes gh-4107.

…are exact.

This improves performance and makes allclose() work on non-numeric dtypes.

Closes numpygh-4107.
@seberg
Copy link
Member

seberg commented Dec 6, 2013

There is a little awkward thing. For fun large fun integer values (or if large values are given to the allowed difference), the behaviour would change. That should be fine, but the change needs to be documented in the function and release notes. Also, you need to channel objects through these, too; maybe it would be better as a negative list rather than a postive one.

xinf = isinf(x)
yinf = isinf(y)
xexact = x.dtype.kind not in 'fc'
yexact = y.dtype.kind not in 'fc'
Copy link
Member

Choose a reason for hiding this comment

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

The behavior for integers should not be changed.
They should also be compared respecting the tolerances given, not requiring exact equality.

@pv
Copy link
Member

pv commented Dec 6, 2013

I think the correct fix is to make allclose raise an error if non-numeric data types are passed in.

@abalkin
Copy link
Contributor Author

abalkin commented Dec 6, 2013

My use-case is that of record arrays.

With the patch, I get

>>> from numpy import *
>>> x = array([('a', 5)], dtype=[('x', 'S1'), ('y', 'l')])
>>> allclose(x, x)
True

without the patch, this produces an error.

I agree that the behavior for ints should not change. Maybe just change the test to .. not in 'fic'?

@seberg
Copy link
Member

seberg commented Dec 6, 2013

I am +/-0 on integer types, and OK with strings being passed through. But your example makes me more cautious. What if the record array has an inexact field? If it is possible to have an inexact number not being matched within tolerance with the function, I prefer to not change it.

@abalkin
Copy link
Contributor Author

abalkin commented Dec 6, 2013

@seberg - here is what I see with the patch:

>>> x = array([('a', 5)], dtype=[('x', 'S1'), ('y', 'f')])
>>> y = array([('a', 5+1e-10)], dtype=[('x', 'S1'), ('y', 'f')])
>>> allclose(x,y)
True

This is what I want: numeric fields compared with tolerance and non-numeric ones compared exactly.

@seberg
Copy link
Member

seberg commented Dec 6, 2013

@abalkin, that is because 5+1e-10 is 5 for floats, use doubles :) ('f' is float).

@abalkin
Copy link
Contributor Author

abalkin commented Dec 6, 2013

My bad. It looks like record arrays need to be special cased.

Let's take this in steps:

  1. Support non-numeric and non-bool dtypes by doing exact match.
  2. Support record arrays.
  3. Support bool without relying on boolean subtraction.

On #3, I think we need to preserve the following behavior:

>>> allclose(True, False, atol=2)
True

@charris
Copy link
Member

charris commented Feb 15, 2014

Allclose now converts its arguments to inexact types before comparison, so I think this PR goes by the way. I think @pv suggestion to not handle non-numeric types is a good one. @abalkin For record arrays you may want to put a different function together to do what you want.

@charris charris closed this Feb 15, 2014
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.

allclose does not work for some dtypes
4 participants