Skip to content

DEP: Handle expired deprecations. #8297

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Nov 26, 2016
Merged

Conversation

charris
Copy link
Member

@charris charris commented Nov 22, 2016

Changes

Deprecations to error

  • partition, TypeError when non-integer partition index is used.
  • NpyIter_AdvancedNew, ValueError when oa_ndim == 0 and op_axes is NULL
  • negative(bool_), TypeError when negative applied to booleans.
  • subtract(bool_, bool_), TypeError when subtracting boolean from boolean.
  • np.equal, np.not_equal, object identity doesn't override failed comparison.
  • np.equal, np.not_equal, object identity doesn't override non-boolean comparison.

FutureWarnings to changed behavior

  • array == None and array != None do element-wise comparison.
  • np.equal, np.not_equal, object identity doesn't override comparison result.

These changes have been warned of since numpy 1.7.0 for the == case and
since numpy 1.8 for the != case. This makes both operators compare
element-wise for this case.
Non-integer index has been deprecated since NumPy 1.8.0.
NpyIter_AdvancedNew raises ValueError when `oa_ndim == 0` and
`op_axes` is NULL.

Deprecated since NumPy 1.8.
@charris charris added this to the 1.13.0 release milestone Nov 22, 2016
The unary minus of booleans was deprecated in NumPy 1.9.
Subtracting a bool_ from a bool_ was deprecated in NumPy 1.9
@charris charris force-pushed the expired-deprecations branch from 4a46daf to c9adc35 Compare November 23, 2016 00:13
This only applies to object arrays. Previously object identity would
override object comparison failures, comparison of objects that did not
return a boolean (arrays), and comparison of objects where the
comparison result did not agree with the object identity result (NaNs)

The first two behaviors have been deprecated since 1.9 and now return
errors. The last has issued a FutureWarning since 1.9 and now returns
the result of the comparison.
@charris
Copy link
Member Author

charris commented Nov 24, 2016

@seberg I'm dealing with all the boolean indexing deprecations and I have found the following odd behavior:

In [1]: a = arange(10)

In [2]: a[array(False)]
Out[2]: array([[0, 1, 2, 3, 4, 5, 6, 7, 8, 9]])

In [3]: a[array(True)]
Out[3]: array([[0, 1, 2, 3, 4, 5, 6, 7, 8, 9]])

So scalar indexing adds a dimension. What is the logic there? I also find it strange that True and False lead to the same results.

@seberg
Copy link
Member

seberg commented Nov 24, 2016

Hmmm, that is a bug then :(. It should be arr[array(False)].shape = (0, 10). Scalar indexing adding a dimension is correct though, if you indexing N-D with N-D you get 1-D. You index 0-D with 0-D plus one extra dimension you don't change, so that is 1-D + 1-D result.
The code branch of this is not well tested yet, it did work at some point but I am not sure if it worked in the end.

@seberg
Copy link
Member

seberg commented Nov 24, 2016

Did not test, but try to chang line 464 indices[curr_idx].value = 1; to set the value to the n below. That would seem a reasonable explain for the problem, and it is possible I did not use that always, so that would make sense as a way to sneak in the mistake.

#assert_equal(a[np.array(False), a[None][0:0]])
assert_equal(a[np.array(True)], a[None])
# @seberg, should this work?
# assert_equal(a[np.array(False), a[None][0:0]])
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@seberg This was a failing test that caught that wrong behavior for False.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ummm, the test has the squared brackets set wrong, but other then that, yes it should work.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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(

I still suggest to split it off, it likely warrents more new tests, and another look.

@charris
Copy link
Member Author

charris commented Nov 24, 2016

Nope, that doesn't solve the problem...

@seberg
Copy link
Member

seberg commented Nov 24, 2016

:( I will have to check myself more closely I guess.

@charris
Copy link
Member Author

charris commented Nov 24, 2016

Seems to work except for that bit. I'm tempted to put in the other deprecation updates and put this one off until it gets working.

@seberg
Copy link
Member

seberg commented Nov 24, 2016

Yeah. Split it off for now. That one might be a bit more hassle even after finding out that bug.

@charris charris force-pushed the expired-deprecations branch from 8259082 to f562e5b Compare November 25, 2016 05:51
@charris
Copy link
Member Author

charris commented Nov 25, 2016

I moved the index stuff to #8312.

@charris
Copy link
Member Author

charris commented Nov 25, 2016

The move seems to have removed your last comment.

@charris
Copy link
Member Author

charris commented Nov 25, 2016

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 charris changed the title WIP, DEP: Handle expired deprecations. DEP: Handle expired deprecations. Nov 26, 2016
@charris charris merged commit dc27edb into numpy:master Nov 26, 2016
@@ -1361,19 +1361,18 @@ def test_eq_with_None(self):
# With partial mask
with suppress_warnings() as sup:
sup.filter(FutureWarning, "Comparison to `None`")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this still needed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it may not be now, could try removing it. Which reminds me that there are a lot of expired deprecations in 1.15 that we should deal with soon.

@charris charris deleted the expired-deprecations branch May 2, 2020 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants