Repository navigation
np.diff is broken for boolean ndarrays in 1.13.0 #9251
Description
Activity
This seems like an easy fix. The
difffunction is pure Python and lives here:https://github.com/numpy/numpy/blob/master/numpy/lib/function_base.py#L1835
You would probably want to branch on
a.dtype == bool, and given the note regarding booleans in the docstring, use^(or!=) instead of-to keep backwards compatibility.It seems to me that if
-is deprecated, thendiffis perfectly reasonable to disallow too.Well - I can see the purity argument for disallowing, but it seems to me that it is reasonable to expect diff to work on booleans, and not surprising that scikit image was doing that. So I would have thought a branch on
boolwas the better route.Reacted by Jaime, Michael Seifert and jakirkhamI'd vote for keeping the functionality. If we all agree, I'll submit a patch later today or tomorrow.
P.S. Could you, please, point me at the discussion where the rationale for deprecating
-=was given?If
diffis deprecated on booleans, then the docstring needs updating to reflect that. And we'd probably still want to branch onboolto raise a more in context error?Reacted by Eric WieserSeems to me that we might want an
np.uint1type that behaves as a integer and not a boolean, on whichnp.diffand-can be defined.My vote would be to avoid the user having to stop and debug an error on
np.diff(some_bools); first - it breaks backward compatibility, and second, the desired behavior is useful and clearly defined.Reacted by Joseph Fox-Rabinovitz and Zaharidfirst - it breaks backward compatibility, and second, the desired behavior is useful and clearly defined.
Both of these arguments apply to
np.subtracttoo, don't they? Perhaps that discussion should be revisitedYes, they do, and if you think of instinctively think of
diffas implementingsubtractthen you'd probably want an error. My belief is that people think ofdiffin a slightly different way fromsubtract, so that they would agree that subtracting bools does not make sense, whereas taking adiffof bools does - meaning 'is this value different from the previous one'.Reacted by Joseph Fox-Rabinovitz"is this value different from the previous one" sounds like an operation worthy of it's own name that generalizes to all dtypes, rather than spelling it
np.diff(x).astype(bool).Perhaps
ufunc.pairwise(x, axis=...), implemented asufunc(x[:-1], x[1:])ignoring the axis argument? That would make the desired functionnp.not_equal.pairwise.Although
pairwisemight imply the behaviour thatouteralready has...Reacted by Joseph Fox-RabinovitzSure, that might be a good idea as well - but I would still argue that it would be kinder (practicality beats purity) to forgive this way of thinking of bools and
np.diff.Reacted by Joseph Fox-Rabinovitz5 remaining items
One other thing we should do is add a test for
diffusing a boolean array, be it toassert_raisesor to validate the result.Since the solution seems to be controversial (I think I'm with Matthew on this one, although I don't care much either way), I have sent an e-mail to the list so that everyone gets a say.
I'm also of the practical beats purity type here, especially since this used to work. Somehow, like @matthew-brett, there is a "diff" is "difference/differs" confusion in my mind, and I certainly have used
np.diffas a quick way to find places where values changed.p.s. @jaimefrio - I didn't see any message on the (numpy discussion) mailing list.
+1 for practicality.
And definitely -1 for a new function. If it's useful, then put it in
diff. The namespace is way too large already.Reacted by Joseph Fox-RabinovitzYes, I noticed I couldn't find my mail in the archives to link it here. But there's an email in my sent folder to [email protected] at 18:00 CET. Perhaps the server is acting up again? Or were those issues fixed for good already?
Wrong address, it has moved ;) [email protected] .
Ah, that would explain it...
Here's a link to the mail, resent to the correct address, for future reference:
https://mail.python.org/pipermail/numpy-discussion/2017-June/076877.html
This doesn't look good at all - scipy/scipy#7493 .
Someone want to make a PR adding support for boolean arrays to diff?
This also applies to
np.gradient. Do we fix that too? (could fold into #9411)Kind ping on @eric-wieser 's question. Should we fix
np.gradientas well?Personally, I don't see much point in supporting gradients of bool arrays.
- added a commit that references this issue
on Jan 14, 2025
Should modify the docstring accordingly, raise a meaningful error, or (preferably) make it work by hidding some logic under the hood.
Traceback:
xref scikit-image/scikit-image#2681