-
-
Notifications
You must be signed in to change notification settings - Fork 11.3k
ENH: Add an "isclose" function for element-wise approximate floating point comparison of two arrays. #224
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
Conversation
Might be worth making this NA aware. |
atol = 1e-8 | ||
|
||
def test_ip_isclose(self): | ||
"""Parametric test factory.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Docstring should be removed, otherwise np.test(verbose=2)
won't show the arguments to yield
(IIRC). Same for other test methods.
Looks good, nice test coverage. It would be useful to test against |
I do confess to not liking the name |
|
…w arguments, as suggested by rgommers.
I've made things MA-aware, and added |
Actually, I was thinking of NA, where NA means missing data:
There is a skipna flag in all that you might want to use in some spots. Maybe it should be an option in isclose also. You can get the mask using isna(a), and for efficiency you can also check it the array is maskna
.Edit: There is also some trailing whitespace. |
Ah! Sorry, I was confused. 408f94c should make things NA-aware. Would it be useful to go ahead and make |
xfin[isna(x)] = False | ||
if y.flags.maskna: | ||
yfin[isna(y)] = False | ||
if all(xfin) and all(yfin): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be simplified also:
In [9]: a = arange(5, maskna=1)
In [10]: a[1] = NA
In [12]: isfinite(a) & isfinite(a)
Out[12]: array([ True, NA, True, True, True], dtype=bool)
In [12]: isfinite(a) & isfinite(a)
Out[12]: array([ True, NA, True, True, True], dtype=bool)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Done in 3d28404 along with your other suggestions.
Hmm, that is strange. There may indeed be a bug somewhere. I just pushed this PR after squashing it a bit and adding a release note. But I'll leave it open for a while. If you can produce a simple failing example it would be helpful. |
Oh, and how current is your numpy fork? |
The "bug" I was running into turned out to just be because |
I think it's safe to close the pull request. There's no bug, I'm just easily confused. :) Thanks! |
OK, that is a bug in ma, please open a ticket for that. Or you could make a PR with a fix ;) I'll go ahead and close this PR. |
feat: Add vst2[q]_[s8|s16|s32|f32|u8|u16|u32|s64|u64]
Following the discussion on the mailing list, it seems like a floating point comparison function that returns a boolean array would be useful.