DEP: Deprecate boolean array indices with non-matching shape #4353

seberg · 2014-02-23T14:55:49Z

Boolean array indices will (unless the special case is taken)
always be converted using a nonzero logic. However, for example

arr = np.arange(10)
index = np.array([True])
arr[index]

would thus work as if index is filled up with False. This is
a source of confusion and hardly useful in practice. Especially
if the array has more then one dimension this behaviour can
be very unexpected as it conflicts with broadcasting.

juliantaylor · 2014-02-24T17:33:02Z

while it makes sense to me to deprecate it (you can always use slicing if you have a sparse bool mask) what do we gain by deprecating it?
it seems it just adds a bunch more code instead of saving any

seberg · 2014-02-24T17:55:36Z

True, it adds quite a bit of code (could maybe restructure it a bit more, but won't change much). The gain is, that I really dislike this:

a = np.ones((5, 5))
index = np.ones((5, 1), dtype=bool)
a[index]  # Surely this uses broadcasting logic ;)

If nobody else minds that much, we can just leave it as is.

charris · 2014-02-25T15:40:01Z

@juliantaylor Just to clarify the example, currently

In [1]: a = np.ones((5, 5))

In [2]: index = np.ones((5, 1), dtype=bool)

In [3]: a[index]
Out[3]: array([ 1.,  1.,  1.,  1.,  1.])

In [4]: a[index[:,0]]
Out[4]: 
array([[ 1.,  1.,  1.,  1.,  1.],
       [ 1.,  1.,  1.,  1.,  1.],
       [ 1.,  1.,  1.,  1.,  1.],
       [ 1.,  1.,  1.,  1.,  1.],
       [ 1.,  1.,  1.,  1.,  1.]])

charris · 2014-08-25T17:56:47Z

@seberg Needs a rebase.

seberg · 2014-08-27T10:56:56Z

Rebased, removed release notes comment for now. Will have to go over this once more though.

seberg · 2014-08-27T12:19:41Z

One useful thing I could now add (and maybe will), is to tell people that an array was original boolean when there is shape mismatch error. It won't be perfect, but better then nothing.

seberg · 2014-10-30T12:42:52Z

Seems like I would need to rebase this... So what is the take would a VisibleDeprecationWarning make sense? And would it be easy to give that from C?

charris · 2015-05-12T21:02:43Z

I like this, but am inclined to put it off to 1.11.0 unless there is agreement and @seberg has time.

A VisibleDeprecationWarning would make sense, not sure of the easiest route to use it would be. Maybe define it in the Multiarray module?

seberg · 2015-05-13T10:42:03Z

Will have to wait two weeks probably, but after that should be able to find some time.

seberg · 2015-05-27T12:33:55Z

I have updated this to use VisibleDeprecationWarning which is imported on the fly when needed (we seem to do similar things for the Complex warnings), etc. However, I am not sure if this is quite right, and it might make sense to do the importing in the init instead of when the warning is given?
Have to review the code myself again some time, but it may be more or less done.

charris · 2015-06-05T01:31:03Z

@seberg I've passed this on to the mailing list if you want to weigh in.

njsmith · 2015-06-05T02:43:05Z

IMO the bizarre behavior of arr[mask1, mask2] is a strong specific argument for deprecating, above and beyond the general confusingness of allowing mismatched shapes. So 👍 here.

charris · 2015-06-05T15:02:49Z

The consensus seems to be "yes", but I'll wait a bit longer.

charris · 2015-06-05T17:04:05Z

@seberg The doc/sphinxext file should not be changed. Not sure what the easiest fix is here, maybe git submodule update -f will do, or maybe you need to clear the directory and rebase on master, or maybe something else.

seberg · 2015-06-05T18:05:01Z

Fixed that part (in the end, manually checked out the correct submodule branch during a rebase...).

charris · 2015-06-05T21:19:22Z

numpy/core/src/multiarray/mapping.c

@@ -741,7 +744,8 @@ prepare_index(PyArrayObject *self, PyObject *index,
    /*
     * At this point indices are all set correctly, no bounds checking
     * has been made and the new array may still have more dimensions
-     * then is possible.
+     * then is possible. And boolean indexing arrays may have had a


then -> than

"than is possible and boolean indexing arrays may have an incorrect shape."

charris · 2015-06-05T21:23:29Z

numpy/core/src/multiarray/mapping.c

+            if ((indices[i].type == HAS_FANCY) && indices[i].value > 0) {
+                if (indices[i].value != PyArray_DIM(self, used_ndim)) {
+                    PyObject *m, *warning;
+


might as well remove this blank line and insert before sprintf.

charris · 2015-06-05T21:30:30Z

Mostly style stuff. There is not as much new code as I was led to believe.

seberg · 2015-06-06T11:30:24Z

Updated, justed the _pyfunc for now, but happy if you change that (other ideas, might be _from_import or just import, but don't want to do bikeshedding about this small thing :))

Boolean array indices will (unless the special case is taken) always be converted using a nonzero logic. However, for example ``` arr = np.arange(10) index = np.array([True]) arr[index] ``` would thus work as if index is filled up with `False`. This is a source of confusion and hardly useful in practice. Especially if the array has more then one dimension this behaviour can be very unexpected as it conflicts with broadcasting. Uses VisibleDeprecationWarning, since this is probably usually a user bug in the first place.

DEP: Deprecate boolean array indices with non-matching shape

charris · 2015-06-07T13:08:26Z

Thanks Sebastian.

seberg mentioned this pull request Jun 16, 2014

WIP: Fix matplotlib, etc. errors #4804

Merged

seberg mentioned this pull request Aug 22, 2014

Boolean array indexing fails silently #4980

Closed

seberg force-pushed the boolean-bad-shape-dep branch from 2e35648 to af5f0bb Compare August 27, 2014 10:56

seberg force-pushed the boolean-bad-shape-dep branch 3 times, most recently from b9dee35 to 59fde25 Compare August 28, 2014 13:33

rgommers added component: numpy.lib 54 - Needs decision labels Mar 8, 2015

charris added this to the 1.10.0 release milestone Apr 7, 2015

seberg force-pushed the boolean-bad-shape-dep branch from 59fde25 to 35104c2 Compare May 18, 2015 15:21

charris added the 07 - Deprecation label Jun 5, 2015

seberg force-pushed the boolean-bad-shape-dep branch from d9e244f to 8803dbd Compare June 5, 2015 18:04

charris reviewed Jun 5, 2015
View reviewed changes

seberg force-pushed the boolean-bad-shape-dep branch 3 times, most recently from 7b09f16 to 0247bda Compare June 6, 2015 08:48

seberg force-pushed the boolean-bad-shape-dep branch from 0247bda to c5ad058 Compare June 6, 2015 11:56

charris added a commit that referenced this pull request Jun 7, 2015

Merge pull request #4353 from seberg/boolean-bad-shape-dep

01c59d6

DEP: Deprecate boolean array indices with non-matching shape

charris merged commit 01c59d6 into numpy:master Jun 7, 2015

seberg deleted the boolean-bad-shape-dep branch June 8, 2015 07:50

sinhrks mentioned this pull request Jul 22, 2015

Indexing with single boolean behaves inconsistently #6104

Closed

Uh oh!

DEP: Deprecate boolean array indices with non-matching shape #4353

DEP: Deprecate boolean array indices with non-matching shape #4353

Uh oh!

Conversation

seberg commented Feb 23, 2014

Uh oh!

juliantaylor commented Feb 24, 2014

Uh oh!

seberg commented Feb 24, 2014

Uh oh!

charris commented Feb 25, 2014

Uh oh!

charris commented Aug 25, 2014

Uh oh!

seberg commented Aug 27, 2014

Uh oh!

seberg commented Aug 27, 2014

Uh oh!

seberg commented Oct 30, 2014

Uh oh!

charris commented May 12, 2015

Uh oh!

seberg commented May 13, 2015

Uh oh!

seberg commented May 27, 2015

Uh oh!

charris commented Jun 5, 2015

Uh oh!

njsmith commented Jun 5, 2015

Uh oh!

charris commented Jun 5, 2015

Uh oh!

charris commented Jun 5, 2015

Uh oh!

seberg commented Jun 5, 2015

Uh oh!

charris Jun 5, 2015

Choose a reason for hiding this comment

Uh oh!

charris Jun 5, 2015

Choose a reason for hiding this comment

Uh oh!

charris Jun 5, 2015

Choose a reason for hiding this comment

Uh oh!

charris commented Jun 5, 2015

Uh oh!

seberg commented Jun 6, 2015

Uh oh!

charris commented Jun 7, 2015

Uh oh!

Uh oh!