DEP: Deprecate boolean array indices with non-matching shape

seberg · seberg · commit 59fde25e7bb4 · 2014-08-28T14:33:52.000+01:00
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.
diff --git a/doc/sphinxext b/doc/sphinxext
@@ -1 +1 @@
-Subproject commit 84cc897d266e0afc28fc5296edf01afb08005472
+Subproject commit 447dd0b59c2fe91ca9643701036d3d04919ddc7e
diff --git a/numpy/core/src/multiarray/mapping.c b/numpy/core/src/multiarray/mapping.c
@@ -563,6 +563,7 @@ prepare_index(PyArrayObject *self, PyObject *index,
 
                 index_type |= HAS_FANCY;
                 indices[curr_idx].type = HAS_0D_BOOL;
+                indices[curr_idx].value = 1;
 
                 /* TODO: This can't fail, right? Is there a faster way? */
                 if (PyObject_IsTrue((PyObject *)arr)) {
@@ -609,6 +610,7 @@ prepare_index(PyArrayObject *self, PyObject *index,
             index_type |= HAS_FANCY;
             for (i=0; i < n; i++) {
                 indices[curr_idx].type = HAS_FANCY;
+                indices[curr_idx].value = PyArray_DIM(arr, i);
                 indices[curr_idx].object = (PyObject *)nonzero_result[i];
 
                 used_ndim += 1;
@@ -651,6 +653,7 @@ prepare_index(PyArrayObject *self, PyObject *index,
 
             index_type |= HAS_FANCY;
             indices[curr_idx].type = HAS_FANCY;
+            indices[curr_idx].value = -1;
             indices[curr_idx].object = (PyObject *)arr;
 
             used_ndim += 1;
@@ -738,7 +741,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
+     * non-matching shape.
      *
      * Check this now so we do not have to worry about it later.
      * It can happen for fancy indexing or with newaxis.
@@ -753,6 +757,42 @@ prepare_index(PyArrayObject *self, PyObject *index,
                          NPY_MAXDIMS, (new_ndim + fancy_ndim));
             goto failed_building_indices;
         }
+
+        /*
+         * If we had a fancy index, we may have had a boolean array index.
+         * So check if this had the correct shape, now that we can find out
+         * which axes it acts on.
+         */
+        used_ndim = 0;
+        for (i=0; i < curr_idx; i++) {
+            if ((indices[i].type == HAS_FANCY) && indices[i].value > 0) {
+                if (indices[i].value != PyArray_DIM(self, used_ndim)) {
+                    char *err_msg[174];
+                    sprintf(err_msg,
+                        "boolean index did not match indexed array along "
+                        "dimension %d; dimension is %" NPY_INTP_FMT
+                        " but corresponding boolean dimension is %" NPY_INTP_FMT,
+                        used_ndim, PyArray_DIM(self, used_ndim),
+                        indices[i].value);
+
+                    if (DEPRECATE(err_msg) < 0) {
+                        goto failed_building_indices;
+                    }
+                    break;
+                }
+            }
+
+            if (indices[i].type == HAS_ELLIPSIS) {
+                used_ndim += indices[i].value;
+            }
+            else if ((indices[i].type  == HAS_NEWAXIS) ||
+                     (indices[i].type == HAS_0D_BOOL)) {
+                used_ndim += 0;
+            }
+            else {
+                used_ndim += 1;
+            }
+        }
     }
 
     *num = curr_idx;
diff --git a/numpy/core/src/multiarray/mapping.h b/numpy/core/src/multiarray/mapping.h
@@ -19,7 +19,11 @@ typedef struct {
      * Object of index: slice, array, or NULL. Owns a reference.
      */
     PyObject *object;
-    /* Value of an integer index or number of slices an Ellipsis is worth */
+    /*
+     * Value of an integer index, number of slices an Ellipsis is worth,
+     * -1 if input was an integer array and the original size of the
+     * boolean array if it is a converted boolean array.
+     */
     npy_intp value;
     /* kind of index, see constants in mapping.c */
     int type;
diff --git a/numpy/core/tests/test_deprecations.py b/numpy/core/tests/test_deprecations.py
@@ -508,5 +508,24 @@ def test_bool_error(self):
         self.assert_deprecated(np.not_equal, args=(a, a))
 
 
+class TestBooleanIndexShapeMismatchDeprecation(_DeprecationTestCase):
+    """Tests deprecation for boolean indexing where the boolean array
+    does not match the input array along the given diemsions.
+    """
+    message = r"boolean index did not match indexed array"
+
+    def test_simple(self):
+        arr = np.ones((5, 4, 3))
+        index = np.array([True])
+        self.assert_deprecated(arr.__getitem__, args=(index,))
+
+        index = np.array([False] * 6)
+        self.assert_deprecated(arr.__getitem__, args=(index,))
+
+        index = np.zeros((4, 4), dtype=bool)
+        self.assert_deprecated(arr.__getitem__, args=(index,))
+        self.assert_deprecated(arr.__getitem__, args=((slice(None), index),))
+
+
 if __name__ == "__main__":
     run_module_suite()
diff --git a/numpy/core/tests/test_indexing.py b/numpy/core/tests/test_indexing.py
@@ -678,14 +678,9 @@ def _get_multi_index(self, arr, indices):
                 arr = arr.reshape((arr.shape[:ax] + (1,) + arr.shape[ax:]))
                 continue
             if isinstance(indx, np.ndarray) and indx.dtype == bool:
-                # This may be open for improvement in numpy.
-                # numpy should probably cast boolean lists to boolean indices
-                # instead of intp!
-
-                # Numpy supports for a boolean index with
-                # non-matching shape as long as the True values are not
-                # out of bounds. Numpy maybe should maybe not allow this,
-                # (at least not array that are larger then the original one).
+                if indx.shape != arr.shape[ax:ax+indx.ndim]:
+                    raise IndexError
+
                 try:
                     flat_indx = np.ravel_multi_index(np.nonzero(indx),
                                     arr.shape[ax:ax+indx.ndim], mode='raise')
diff --git a/numpy/core/tests/test_multiarray.py b/numpy/core/tests/test_multiarray.py
@@ -2139,29 +2139,29 @@ def test_tuple(self):
 
     def test_mask(self):
         x = array([1, 2, 3, 4])
-        m = array([0, 1], bool)
+        m = array([0, 1, 0, 0], bool)
         assert_array_equal(x[m], array([2]))
 
     def test_mask2(self):
         x = array([[1, 2, 3, 4], [5, 6, 7, 8]])
         m = array([0, 1], bool)
-        m2 = array([[0, 1], [1, 0]], bool)
-        m3 = array([[0, 1]], bool)
+        m2 = array([[0, 1, 0, 0], [1, 0, 0, 0]], bool)
+        m3 = array([[0, 1, 0, 0], [0, 0, 0, 0]], bool)
         assert_array_equal(x[m], array([[5, 6, 7, 8]]))
         assert_array_equal(x[m2], array([2, 5]))
         assert_array_equal(x[m3], array([2]))
 
     def test_assign_mask(self):
         x = array([1, 2, 3, 4])
-        m = array([0, 1], bool)
+        m = array([0, 1, 0, 0], bool)
         x[m] = 5
         assert_array_equal(x, array([1, 5, 3, 4]))
 
     def test_assign_mask2(self):
         xorig = array([[1, 2, 3, 4], [5, 6, 7, 8]])
         m = array([0, 1], bool)
-        m2 = array([[0, 1], [1, 0]], bool)
-        m3 = array([[0, 1]], bool)
+        m2 = array([[0, 1, 0, 0], [1, 0, 0, 0]], bool)
+        m3 = array([[0, 1, 0, 0], [0, 0, 0, 0]], bool)
         x = xorig.copy()
         x[m] = 10
         assert_array_equal(x, array([[1, 2, 3, 4], [10, 10, 10, 10]]))
diff --git a/numpy/core/tests/test_regression.py b/numpy/core/tests/test_regression.py
@@ -787,18 +787,21 @@ def test_arange_non_native_dtype(self, level=rlevel):
             assert_equal(np.arange(0.5, dtype=dt).dtype, dt)
             assert_equal(np.arange(5, dtype=dt).dtype, dt)
 
-    def test_bool_indexing_invalid_nr_elements(self, level=rlevel):
+    def test_bool_flat_indexing_invalid_nr_elements(self, level=rlevel):
         s = np.ones(10, dtype=float)
         x = np.array((15,), dtype=float)
         def ia(x, s, v): x[(s>0)]=v
         # After removing deprecation, the following are ValueErrors.
         # This might seem odd as compared to the value error below. This
         # is due to the fact that the new code always uses "nonzero" logic
         # and the boolean special case is not taken.
-        self.assertRaises(IndexError, ia, x, s, np.zeros(9, dtype=float))
-        self.assertRaises(IndexError, ia, x, s, np.zeros(11, dtype=float))
+        with warnings.catch_warnings():
+            warnings.simplefilter('ignore', DeprecationWarning)
+            self.assertRaises(IndexError, ia, x, s, np.zeros(9, dtype=float))
+            self.assertRaises(IndexError, ia, x, s, np.zeros(11, dtype=float))
         # Old special case (different code path):
         self.assertRaises(ValueError, ia, x.flat, s, np.zeros(9, dtype=float))
+        self.assertRaises(ValueError, ia, x.flat, s, np.zeros(11, dtype=float))
 
     def test_mem_scalar_indexing(self, level=rlevel):
         """Ticket #603"""
