MAINT: Use XOR for bool arrays in np.diff
#9259
Conversation
numpy/lib/function_base.py
Outdated
| @@ -1912,10 +1911,16 @@ def diff(a, n=1, axis=-1): | |||
| slice2[axis] = slice(None, -1) | |||
| slice1 = tuple(slice1) | |||
| slice2 = tuple(slice2) | |||
|
|
|||
| if a.dtype == bool: | |||
There was a problem hiding this comment.
Also it would be nice if you can make the commit use our typical format such as BUG: ... (or maybe MAINT: here even, sometimes its hard to say ;)).
There was a problem hiding this comment.
@seberg should be good now.
np.diffnp.diff
numpy/lib/function_base.py
Outdated
| @@ -1912,10 +1911,16 @@ def diff(a, n=1, axis=-1): | |||
| slice2[axis] = slice(None, -1) | |||
| slice1 = tuple(slice1) | |||
| slice2 = tuple(slice2) | |||
|
|
|||
| if a.dtype == np.bool_: | |||
| da = a[slice1] ^ a[slice2] | |||
There was a problem hiding this comment.
I might be tempted to use !=?
There was a problem hiding this comment.
Furthermore, if we use !=, then we can add support for dtypes with fields here too. (Possibly an argument for a different name)
There was a problem hiding this comment.
!= is kinda nice. Don't know if it add any funtionality. @eric-wieser Could you explain a bit more.
There was a problem hiding this comment.
we can add support for dtypes with fields here too
@eric-wieser I remember that amazing feature I was always willing to try :). Is better to keep it for a separate PR though.
There was a problem hiding this comment.
I note that at the ufunc level, logical_xor is defined as not_equal for booleans, but the operator != goes through richcompare while ^ does not, hence ^ is a trifle faster.
One problem with the deprecation of - is that if we remove the boolean implementation the bools will be upcast to int8, which will really confuse people, but at least be consistent with python ...
Test would become Of course, it would be weird to not also support this for float arrays, suggesting that the "adjacent entries are different" operation should be given a different name to Possible names for the new function could be:
|
|
I think that's outside the scope of this PR though, and should be considered after merging this |
|
Thanks @soupault . |
|
Did this get backported into NumPy 1.13/land in the 1.13.1 release, @charris? |
|
@jakirkham No, the |
|
Ok, makes sense. Thanks. |
The binary
-operator is no longer supported for booleans. Use xor instead to maintain backwards compatibility.Added some tests for this case, and also for float.
Closes #9251.