DEP: Drop deprecated boolean indexing behavior and update to new. #8312

charris · 2016-11-25T05:54:56Z

This affects mostly old boolean indexing behavior that was deprecated in NumPy 1.9. At the time the new behavior was causing errors in a number of downstream projects, hopefully that has been taken care of.

charris · 2016-11-25T06:02:20Z

Copied from post from @seberg.

OK, this seems to do the trick:


@@ -441,16 +441,6 @@ prepare_index(PyArrayObject *self, PyObject *index,
 
             if (PyArray_NDIM(arr) == 0) {
                 /*
-                 * TODO, WARNING: This code block cannot be used due to
-                 *                FutureWarnings at this time. So instead
-                 *                just raise an IndexError.
-                 */
-                PyErr_SetString(PyExc_IndexError,
-                        "in the future, 0-d boolean arrays will be "
-                        "interpreted as a valid boolean index");
-                Py_DECREF((PyObject *)arr);
-                goto failed_building_indices;
-                /*
                  * This can actually be well defined. A new axis is added,
                  * but at the same time no axis is "used". So if we have True,
                  * we add a new axis (a bit like with np.newaxis). If it is
@@ -459,7 +449,6 @@ 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)) {
@@ -468,6 +457,7 @@ prepare_index(PyArrayObject *self, PyObject *index,
                 else {
                     n = 0;
                 }
+                indices[curr_idx].value = n;
                 indices[curr_idx].object = PyArray_Zeros(1, &n,
                                             PyArray_DescrFromType(NPY_INTP), 0);
                 Py_DECREF(arr);
@@ -2450,6 +2440,10 @@ mapiter_fill_info(PyArrayMapIterObject *mit, npy_index_info *indices,
             mit->fancy_dims[j] = 1;
             /* Does not exist */
             mit->iteraxes[j++] = -1;
+            if (mit->dimensions[mit->nd_fancy-1] > 1) {
+                goto broadcast_error;
+            }
+            mit->dimensions[mit->nd_fancy-1] *= indices[i].value;
         }
 
         /* advance curr_dim for non-fancy indices */
@@ -2487,7 +2481,7 @@ mapiter_fill_info(PyArrayMapIterObject *mit, npy_index_info *indices,
     }
 
     for (i = 0; i < index_num; i++) {
-        if (indices[i].type != HAS_FANCY) {
+        if (!(indices[i].type & HAS_FANCY)) {
             continue;
         }
         tmp = convert_shape_to_string(

charris · 2016-11-27T02:21:56Z

numpy/core/tests/test_indexing.py

@@ -714,7 +709,8 @@ def setUp(self):
            0,
            # Boolean indices, up to 3-d for some special cases of eating up
            # dimensions, also need to test all False
-            np.array(False),
+            # Fixme


@seberg, this test fails. Should it?

Sure, the automated test, could probably be fixed, but we could even think about just removing this part since the other tests are more comprehensive nowadays.

I had a glance at the code, I think I can probably fix it with two small modifications.

I thought about removing it, not least because it is complicated.

Good with me, it was very useful when there were not many tests, now its not awfully useful (hopefully). How about just removing this one case and maybe adding a comment that there is likely no point in supporting it indefinitely (could also move to its own file?). So that we can basically remove it whenever it should be too much trouble to tweak it?

Just am not sure if I like removing the test completely without a strong need. Who knows maybe it does test some things otherwise not tested, indexing is quite branched with many special cases.

charris · 2016-11-27T02:22:26Z

numpy/core/tests/test_indexing.py

@@ -1171,8 +1167,9 @@ def test_bool_as_int_argument(self):
        # array is thus also deprecated, but not with the same message:
        assert_raises(TypeError, operator.index, np.array(True))
        assert_raises(TypeError, np.take, args=(a, [0], False))
-        assert_raises(IndexError, lambda: a[False, 0])
-        assert_raises(IndexError, lambda: a[False, 0, 0])
+        #fixme


@seberg these two tests fail, should they?

The whole test should probably be replaced with new tests for the new behaviour. Sounds like it tests old behaviour

Just wondering if the tested behavior was no longer valid. I seems that now booleans and integers can be mixed.

charris · 2016-11-30T20:58:24Z

OK, I've removed the failing tests. This should be ready to go unless more tests are desired.

seberg · 2016-11-30T21:26:18Z

Should probably add tests for some of the wonkier behaviour like :/

arr = np.ones((2, 3, 4))
arr[True, [0, 1], True, True, [1], [[2]]].shape == (1, 2)  # I think, who knows ;)
arr[False, True, ...].shape == (0, 2, 3, 4)
arr[False, [0, 1], ...]  # IndexError, can't broadcast

I can try to write more some time.

seberg · 2016-11-30T21:27:14Z

Obviously all of those examples don't make any real sense at all, but they are valid combinations of the nonsense boolean indexing rules.

charris · 2016-11-30T22:13:27Z

OK, I added a def test_boolean_indexing_weirdness(): function. Your second example throws an IndexError.

seberg · 2016-11-30T22:29:12Z

numpy/core/src/multiarray/mapping.c

@@ -2452,6 +2345,10 @@ mapiter_fill_info(PyArrayMapIterObject *mit, npy_index_info *indices,
            mit->fancy_dims[j] = 1;
            /* Does not exist */
            mit->iteraxes[j++] = -1;
+            if (mit->dimensions[mit->nd_fancy - 1] > 1) {


That test I guessed the result wrong... If its a True, its like a dimension of 1 and that means it can broadcast:

if ((indices[i].value == 0) && (mit->dimensions[mit->nd_fancy - 1] > 1)) {

should fix it. Wonky stuff mixing boolean arrays with other indexes....

I guess the truth is, it is completely insane to do it anyway, but at least in my head there is a set of rules you can apply to get the result ;).

Oh, I gave the wrong one, this is the one that fails:

a[True, [0, 1], True, True, [1], [[2]]]

Still need the fix?

Yeah, I had pulled the PR to see the error message.

charris · 2016-12-03T03:04:49Z

OK, updated.

seberg · 2016-12-03T15:37:51Z

LGTM. Its good to put this in early, who knows what problems might crop up. If you want I can try to write more tests some time, though could do that also after this is put in (could open an issue in that case I guess). Go ahead and put it in (or I can do it).

seberg · 2016-12-03T15:38:25Z

Btw. thanks for taking care of all of this maintenance work!

This affects mostly old boolean indexing behavior that was deprecated in NumPy 1.9. At the time the new behavior was causing errors in a number of downstream projects, hopefully that has been taken care of.

charris · 2016-12-03T18:49:26Z

I've updated the release notes and will put this in. Thanks for the review. And of course more tests, and perhaps clarification in the release notes, are welcome ;)

charris added 07 - Deprecation 03 - Maintenance component: numpy._core labels Nov 25, 2016

charris added this to the 1.13.0 release milestone Nov 25, 2016

charris mentioned this pull request Nov 25, 2016

DEP: Handle expired deprecations. #8297

Merged

charris changed the title ~~WIP: Change expired index deprecations.~~ WIP: Deal with expired index deprecations. Nov 27, 2016

charris force-pushed the index-deprecation branch from c13e8a4 to 1265f86 Compare November 27, 2016 02:21

charris commented Nov 27, 2016

View reviewed changes

charris force-pushed the index-deprecation branch from 1265f86 to a92e6ed Compare November 30, 2016 20:56

charris changed the title ~~WIP: Deal with expired index deprecations.~~ DEP: Drop deprecated boolean indexing behavior and update to new. Nov 30, 2016

charris force-pushed the index-deprecation branch from a92e6ed to b005be2 Compare November 30, 2016 22:12

seberg reviewed Nov 30, 2016

View reviewed changes

charris force-pushed the index-deprecation branch from b005be2 to 2ac4537 Compare December 3, 2016 03:04

DEP: Drop deprecated boolean indexing behavior and update to new.

b80bcee

This affects mostly old boolean indexing behavior that was deprecated in NumPy 1.9. At the time the new behavior was causing errors in a number of downstream projects, hopefully that has been taken care of.

charris force-pushed the index-deprecation branch from 2ac4537 to b80bcee Compare December 3, 2016 18:47

charris merged commit 6429c6f into numpy:master Dec 3, 2016

charris deleted the index-deprecation branch December 3, 2016 19:29

halkk990 mentioned this pull request Feb 25, 2024

AA halkk990/docs#1

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DEP: Drop deprecated boolean indexing behavior and update to new. #8312

DEP: Drop deprecated boolean indexing behavior and update to new. #8312

charris commented Nov 25, 2016 •

edited

Loading

charris commented Nov 25, 2016

charris Nov 27, 2016

seberg Nov 27, 2016

seberg Nov 27, 2016

charris Nov 27, 2016

seberg Nov 27, 2016

charris Nov 27, 2016

seberg Nov 27, 2016

charris Nov 27, 2016

charris commented Nov 30, 2016

seberg commented Nov 30, 2016

seberg commented Nov 30, 2016

charris commented Nov 30, 2016

seberg Nov 30, 2016

seberg Nov 30, 2016

charris Dec 1, 2016

seberg Dec 1, 2016

charris commented Dec 3, 2016

seberg commented Dec 3, 2016

seberg commented Dec 3, 2016

charris commented Dec 3, 2016

DEP: Drop deprecated boolean indexing behavior and update to new. #8312

DEP: Drop deprecated boolean indexing behavior and update to new. #8312

Conversation

charris commented Nov 25, 2016 • edited Loading

charris commented Nov 25, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

charris commented Nov 30, 2016

seberg commented Nov 30, 2016

seberg commented Nov 30, 2016

charris commented Nov 30, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

charris commented Dec 3, 2016

seberg commented Dec 3, 2016

seberg commented Dec 3, 2016

charris commented Dec 3, 2016

charris commented Nov 25, 2016 •

edited

Loading