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

DEP: Deprecate boolean array indices with non-matching shape

7 files changed, 101 insertions, 21 deletions

diff --git a/numpy/core/src/multiarray/mapping.c b/numpy/core/src/multiarray/mapping.c
index a510c7b0c..604f5390f 100644
--- a/numpy/core/src/multiarray/mapping.c
+++ b/numpy/core/src/multiarray/mapping.c
@@ -11,6 +11,7 @@
 #include "npy_config.h"
 
 #include "npy_pycompat.h"
+#include "npy_import.h"
 
 #include "common.h"
 #include "iterators.h"
@@ -566,6 +567,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)) {
@@ -612,6 +614,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;
@@ -654,6 +657,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;
@@ -741,7 +745,7 @@ 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.
+     * than is possible and boolean indexing arrays may have an incorrect shape.
      *
      * Check this now so we do not have to worry about it later.
      * It can happen for fancy indexing or with newaxis.
@@ -756,6 +760,50 @@ 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)) {
+                    static PyObject *warning;
+
+                    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);
+
+                    npy_cache_pyfunc(
+                        "numpy", "VisibleDeprecationWarning", &warning);
+                    if (warning == NULL) {
+                        goto failed_building_indices;
+                    }
+
+                    if (PyErr_WarnEx(warning, err_msg, 1) < 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
index f59f6bb22..40ccabd62 100644
--- 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/src/multiarray/multiarraymodule.c b/numpy/core/src/multiarray/multiarraymodule.c
index 7980a0de7..018345e6c 100644
--- a/numpy/core/src/multiarray/multiarraymodule.c
+++ b/numpy/core/src/multiarray/multiarraymodule.c
@@ -4448,6 +4448,7 @@ intern_strings(void)
            npy_ma_str_ndmin;
 }
 
+
 #if defined(NPY_PY3K)
 static struct PyModuleDef moduledef = {
         PyModuleDef_HEAD_INIT,
diff --git a/numpy/core/tests/test_deprecations.py b/numpy/core/tests/test_deprecations.py
index 9e2248205..159753797 100644
--- a/numpy/core/tests/test_deprecations.py
+++ b/numpy/core/tests/test_deprecations.py
@@ -508,5 +508,32 @@ class TestAlterdotRestoredotDeprecations(_DeprecationTestCase):
         self.assert_deprecated(np.restoredot)
 
 
+class TestBooleanIndexShapeMismatchDeprecation():
+    """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,))
+        assert_warns(np.VisibleDeprecationWarning,
+                     arr.__getitem__, index)
+
+        index = np.array([False] * 6)
+        #self.assert_deprecated(arr.__getitem__, args=(index,))
+        assert_warns(np.VisibleDeprecationWarning,
+             arr.__getitem__, index)
+
+        index = np.zeros((4, 4), dtype=bool)
+        #self.assert_deprecated(arr.__getitem__, args=(index,))
+        assert_warns(np.VisibleDeprecationWarning,
+             arr.__getitem__, index)
+        #self.assert_deprecated(arr.__getitem__, args=((slice(None), index),))
+        assert_warns(np.VisibleDeprecationWarning,
+             arr.__getitem__, (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
index 3beef71fb..6346d2d40 100644
--- a/numpy/core/tests/test_indexing.py
+++ b/numpy/core/tests/test_indexing.py
@@ -726,14 +726,9 @@ class TestMultiIndexingAutomated(TestCase):
                 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')
@@ -959,6 +954,7 @@ class TestMultiIndexingAutomated(TestCase):
             # This is so that np.array(True) is not accepted in a full integer
             # index, when running the file separately.
             warnings.filterwarnings('error', '', DeprecationWarning)
+            warnings.filterwarnings('error', '', np.VisibleDeprecationWarning)
             for simple_pos in [0, 2, 3]:
                 tocheck = [self.fill_indices, self.complex_indices,
                            self.fill_indices, self.fill_indices]
@@ -981,7 +977,7 @@ class TestMultiIndexingAutomated(TestCase):
     def test_1d(self):
         a = np.arange(10)
         with warnings.catch_warnings():
-            warnings.filterwarnings('error', '', DeprecationWarning)
+            warnings.filterwarnings('error', '', np.VisibleDeprecationWarning)
             for index in self.complex_indices:
                 self._check_single_index(a, index)
 
diff --git a/numpy/core/tests/test_multiarray.py b/numpy/core/tests/test_multiarray.py
index 74c57e18a..1a7e3cce6 100644
--- a/numpy/core/tests/test_multiarray.py
+++ b/numpy/core/tests/test_multiarray.py
@@ -2548,29 +2548,29 @@ class TestFancyIndexing(TestCase):
 
     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
index 399470214..34f96298f 100644
--- a/numpy/core/tests/test_regression.py
+++ b/numpy/core/tests/test_regression.py
@@ -788,18 +788,22 @@ class TestRegression(TestCase):
             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
+        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)
+            warnings.simplefilter('ignore', np.VisibleDeprecationWarning)
+            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"""