cc @mwiebe who (as far as I can tell from git blame) implemented nonzero

You're right that the current behavior is wrong, but I don't think there's a sensible choice to make here.

>>> np.array([[True]]).nonzero()
(array([0], dtype=int64), array([0], dtype=int64))
>>> np.array([[False]]).nonzero()
(array([], dtype=int64), array([], dtype=int64))

>>> np.array([True]).nonzero()
(array([0], dtype=int64),)
>>> np.array([False]).nonzero()
(array([], dtype=int64),)

>>> np.array(True).nonzero()
(array([0], dtype=int64),)
>>> np.array(False).nonzero()
(array([], dtype=int64),)

Perhaps nonzero should just throw an exception if called on a 0d array?

One thing to think about is what should a[a.nonzero()] return in each case? (a=np.array(0) and a=np.array(5))

@umanwizard: The simple answer is "never use x[y.nonzero()], use x[y.astype(bool)] instead"

@eric-wieser Should we change the docs informing the above as part of NOTE ?

That sounds like a good idea, although you could probably use more helpful phrasing than I used above.

Got it. @umanwizard Can you please tell the docs location so that I could proceed

Sorry, I don't work on Numpy, I am not sure where the docs are in the repository.

It lives in the docstring for nonzero in numpy/core/fromnumeric.py. See also how to document

Thanks

Sorry to be coming late to this, but might it be good to think whether this should not be classified as a bug that we can fix?

In particular, as @umanwizard wrote (#13522 (comment)), the docstring specifically states that you can extract non-zero elements by doing x[x.nonzero()], i.e., that the function returns (title of docstring), "the indices of the elements that are non-zero". Might it be an idea to do exactly that even for array scalars? I.e., return Ellipsis for the scalar-nonzero case and False for the scalar-zero case?

Though admittedly the return values are not that great. For nonzero scalar, it could also be an empty tuple, but that would seem to give the wrong impression. Unfortunately, just indexing with True gives an extra dimension:

a = np.array(5)
a[True]
# array([5])

For the scalar-zero case, it would be nicer to index with an empty array, but one cannot create a zero-dimensional empty array...

The first extra dimension is actually correct, since nonzero returns a tuple of 1D arrays. True and False give correct/analogous indexing operation. But they are silly for interpretation. The other semi correct thing for nonzero would be a (0, 1) shaped array (instead of tuple) when the element is True and (0, 0) shaped when it is False.

But, neither of those fit with returning a tuple, so I am not sure we should do that (at least without some kwarg?). I guess the function is arguably wrong for 0-D currently, so a deprecation would make sense in that case.

@seberg - The problem with any index array is that will not actually index a scalar, so I think it is not in any way more useful than the current state!

Hence, my wondering whether we can make it useful, even if slightly ugly. Specifically, the start of the docstring is:

Return the indices of the elements that are non-zero.

Returns a tuple of arrays, one for each dimension of `a`,
containing the indices of the non-zero elements in that
dimension. 

So, reading this strictly for a zero-dimensional array, I should get a tuple with zero arrays, i.e., (). Which of course is not very useful and if used to index the scalar would always return it unchanged independent of its value. My suggestion above was that it was not going too far from the docstring to take it to mean that one doesn't get a tuple at all, but rather something scalar-like that when used as an index gives the right answer,hence Ellipsis and False (I'd have liked True but clearly that has some advanced indexing semantics that I'm not completely sure I follow).

EDIT: might as well add the explicit example:

a = np.array(5)
a[...]
# array(5)
a[False]
# array([], dtype=int64)

False (I'd have liked True but clearly that has some advanced indexing semantics that I'm not completely sure I follow).

True is correct behavior here - advanced indexing always flattens (or promotes) to a 1d array.

but rather something scalar-like that when used as an index gives the right answer

If the semantics of the function are reduced to just "something that can be indexed with", it can be reduced greatly to just:

def nonzero(a):
    return a.astype(bool, copy=False)

Well, astype(bool) obviously completely changes the meaning. I'm trying to have my cake and eat it! Or "make the function work at least for some of its use cases also for scalars without breaking others". Maybe an empty tuple and False is still better? Then only False is the strange exception to what one would naively expect.

Well, that would mean returning True and False, but that seems strange. I frankly am starting to think if we really want to move this, maybe an error is best, with some information of what is going on.

If someone really wants to use it mostly for indexing, they probably should not be doing that to begin with ;) (although it can speed up things for repeated indexing operations).

astype(bool) completly changes the meaning, but this is what you want to change the meaning to for 0D!

I think we should deprecate this, with a message saying:

• Use atleast1d(a).nonzero() for the old behavior
• Some suggestions for ways to actually spell what the user intended.

@eric-wieser - am also in favour of changing the behaviour for scalars - any preferences for what it would become?

I'm leaning towards raise ValueError until we can design something better alongside the oindex / vindex stuff.\

The something better would be returning something like:

def broadcast_shapes(shapes):
    # hack - we ought to expose this more directly somewhere
    return np.lib.stride_tricks._broadcast_shape(*[np.empty(shape, dtype=[]) for shape in shapes])

class VIndex(tuple):
    shape = ...  #type: Tuple[int]
    def __new__(cls, iter, *, shape=()):
        self = super().__new__(cls, iter)
        self.shape = broadcast_shapes([np.shape(i) for i in iter] + [shape])
        return self
    def __repr__(self):
        return "VIndex({}, shape={})".format(tuple.__repr__(self), self.shape)
    def __add__(self, other):
        if isinstance(other, tuple):
            other = VIndex(other)
        if isinstance(other, VIndex):
            ret = tuple.__new__(type(self), tuple.__add__(self, other))
            ret.shape = broadcast_shapes([self.shape, other.shape])
            return ret
        return NotImplemented

Used as:

def fixed_nonzero(x):
    old = nonzero(x)
    shape = old[0].shape
    if x.ndim == 0:
        old = ()
    return VIndex(old, shape=shape)

Of course, now ndarray.__getitem__ needs to understand these new indexing objects.

In my examples above, I've been a bit silly with respect to the indexing: a[a.nonzero()] always returns a flat list with the non-zero elements, independent of the shape. So, for an array scalar, one should expect to get a 1-D array back as well, with either zero or one element. Which means returning True or False for the scalar case would work. (Long and short: ideas about Ellipsis and () were bad.)

To me, that is still slightly better than starting to error, but I feel either is better than sticking with the status quo.

Deprecation PR is at #13708