Skip to content

ENH: Add equal_nan keyword argument to array_equal#16128

Merged
seberg merged 9 commits intonumpy:masterfrom
rossbar:enh/array_equal_nan
May 11, 2020
Merged

ENH: Add equal_nan keyword argument to array_equal#16128
seberg merged 9 commits intonumpy:masterfrom
rossbar:enh/array_equal_nan

Conversation

@rossbar
Copy link
Contributor

@rossbar rossbar commented May 1, 2020

Following up on a suggestion by @charris in #9229 . Adds an equal_nan kwarg that toggles whether NaN's are considered equivalent when compared (default is False, which is the current behavior). This kwarg is consistent with that of related functions like isclose and allclose.

This would close at least part of #9229, though there is other discussion in that issue about np.testing.assert_array_equal.

Copy link
Member

@seberg seberg left a comment

Choose a reason for hiding this comment

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

Looks good to me, could add a test for 0-D arrays containing NaN, as well as a test with a non-float type just to make a point.

Copy link
Contributor Author

@rossbar rossbar left a comment

Choose a reason for hiding this comment

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

Thanks for taking a look @seberg - excellent suggestions as always!

I have a question about one of your suggestions:

as well as a test with a non-float type just to make a point.

I'm not sure exactly what you have in mind here. Perhaps a test with integer arrays (without nans) to show that the result is insensitive to the equal_nan kwarg? Something like:

>>> a = np.array([1, 2, 3], dtype=int)
>>> assert_(np.array_equal(a, a, equal_nan=False))
>>> assert_(np.array_equal(a, a, equal_nan=True))

LMK if this is what you had in mind or if I'm missing the point :)

@seberg
Copy link
Member

seberg commented May 3, 2020

Yeah, was just thinking about such a test. A test with 2D arrays (maybe both False and True) may be good. That probably would have found the np.all problem (if its a problem).

 * 0D array of nan
 * 2D array with nan
 * integer array
Input arrays.
equal_nan : bool
Whether to compare NaN's as equal.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, bad case of not-careful enough first review. This should have a .. versionadded:: 1.19.0 tag.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, forgot that myself. Does it need a release note as well?

@rossbar
Copy link
Contributor Author

rossbar commented May 3, 2020

Good idea - I've added tests for

  • a 0-D array of with nan
  • an array with non-float dtype
  • 2-D arrays with nan

@seberg seberg added the 62 - Python API Changes or additions to the Python API. Mailing list should usually be notified. label May 3, 2020
rossbar added 3 commits May 3, 2020 15:16
 * Add explanation and code example to docstring detailing how complex
   values are handled when equal_nan=True.
 * Add a test as well.
@rossbar
Copy link
Contributor Author

rossbar commented May 4, 2020

The discussion in #15959 is also relevant here.

@seberg seberg merged commit ba9f93d into numpy:master May 11, 2020
@seberg
Copy link
Member

seberg commented May 11, 2020

Thanks Ross!

@aloosley
Copy link

Nice, thanks @rossbar !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

01 - Enhancement 62 - Python API Changes or additions to the Python API. Mailing list should usually be notified. component: numpy._core

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants