Skip to content

Remove NotImplemented handling from the ufunc machinery (almost) #5864

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

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 43 additions & 4 deletions doc/release/1.10.0-notes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,23 @@ Highlights
evaluation order.
* Addition of `nanprod` to the set of nanfunctions.

Dropped Support:

Dropped Support
===============
* The polytemplate.py file has been removed.
* The _dotblas module is no longer available.
* The testcalcs.py file has been removed.

Future Changes:

Future Changes
==============
* In array comparisons like ``arr1 == arr2``, many corner cases
involving strings or structured dtypes that used to return scalars
now issue ``FutureWarning`` or ``DeprecationWarning``, and in the
future will be change to either perform elementwise comparisons or
raise an error.
* The SafeEval class will be removed.
* The alterdot and restoredot functions will be removed.

See below for more details on these changes.

Compatibility notes
===================
Expand Down Expand Up @@ -250,6 +254,41 @@ array is writeable.
Deprecations
============

Array comparisons involving strings or structured dtypes
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Normally, comparison operations on arrays perform elementwise
comparisons and return arrays of booleans. But in some corner cases,
especially involving strings are structured dtypes, NumPy has
historically returned a scalar instead. For example::

### Current behaviour

np.arange(2) == "foo"
# -> False

np.arange(2) < "foo"
# -> True on Python 2, error on Python 3

np.ones(2, dtype="i4,i4") == np.ones(2, dtype="i4,i4,i4")
# -> False

Continuing work started in 1.9, in 1.10 these comparisons will now
raise ``FutureWarning`` or ``DeprecationWarning``, and in the future
they will be modified to behave more consistently with other
comparison operations, e.g.::

### Future behaviour

np.arange(2) == "foo"
# -> array([False, False])

np.arange(2) < "foo"
# -> error, strings and numbers are not orderable

np.ones(2, dtype="i4,i4") == np.ones(2, dtype="i4,i4,i4")
# -> [False, False]

SafeEval
~~~~~~~~
The SafeEval class in numpy/lib/utils.py is deprecated and will be removed
Expand Down
115 changes: 70 additions & 45 deletions numpy/core/src/multiarray/arrayobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -1295,6 +1295,34 @@ array_richcompare(PyArrayObject *self, PyObject *other, int cmp_op)
PyObject *obj_self = (PyObject *)self;
PyObject *result = NULL;

/* Special case for string arrays (which don't and currently can't have
* ufunc loops defined, so there's no point in trying).
*/
if (PyArray_ISSTRING(self)) {
array_other = (PyArrayObject *)PyArray_FromObject(other,
NPY_NOTYPE, 0, 0);
if (array_other == NULL) {
PyErr_Clear();
/* Never mind, carry on, see what happens */
}
else if (!PyArray_ISSTRING(array_other)) {
Py_DECREF(array_other);
/* Never mind, carry on, see what happens */
}
else {
return _strings_richcompare(self, array_other, cmp_op, 0);
}
/* If we reach this point, it means that we are not comparing
* string-to-string. It's possible that this will still work out,
* e.g. if the other array is an object array, then both will be cast
* to object or something? I don't know how that works actually, but
* it does, b/c this works:
* l = ["a", "b"]
* assert np.array(l, dtype="S1") == np.array(l, dtype="O")
* So we fall through and see what happens.
*/
}

switch (cmp_op) {
case Py_LT:
if (needs_right_binop_forward(obj_self, other, "__gt__", 0) &&
Expand Down Expand Up @@ -1324,16 +1352,6 @@ array_richcompare(PyArrayObject *self, PyObject *other, int cmp_op)
Py_INCREF(Py_False);
return Py_False;
}
if (needs_right_binop_forward(obj_self, other, "__eq__", 0) &&
Py_TYPE(obj_self)->tp_richcompare != Py_TYPE(other)->tp_richcompare) {
Py_INCREF(Py_NotImplemented);
return Py_NotImplemented;
}
result = PyArray_GenericBinaryFunction(self,
(PyObject *)other,
n_ops.equal);
if (result && result != Py_NotImplemented)
break;

/*
* The ufunc does not support void/structured types, so these
Expand All @@ -1351,6 +1369,11 @@ array_richcompare(PyArrayObject *self, PyObject *other, int cmp_op)
*/
if (array_other == NULL) {
PyErr_Clear();
if (DEPRECATE(
"elementwise == comparison failed and returning scalar "
Copy link
Member

Choose a reason for hiding this comment

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

Might be clearer if the text lines up as an argument to DEPRECATE(

OK, let's not worry about it. It will go away eventually anyway.

"instead; this will raise an error in the future.") < 0) {
return NULL;
}
Py_INCREF(Py_NotImplemented);
return Py_NotImplemented;
}
Expand All @@ -1359,18 +1382,31 @@ array_richcompare(PyArrayObject *self, PyObject *other, int cmp_op)
PyArray_DESCR(array_other),
NPY_EQUIV_CASTING);
if (_res == 0) {
Py_DECREF(result);
Py_DECREF(array_other);
if (DEPRECATE_FUTUREWARNING(
Copy link
Member

Choose a reason for hiding this comment

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

But that won't work here. Hmm....

"elementwise == comparison failed and returning scalar "
"instead; this will raise an error or perform "
"elementwise comparison in the future.") < 0) {
return NULL;
}
Py_INCREF(Py_False);
return Py_False;
}
else {
Py_DECREF(result);
result = _void_compare(self, array_other, cmp_op);
}
Py_DECREF(array_other);
return result;
}

if (needs_right_binop_forward(obj_self, other, "__eq__", 0) &&
Py_TYPE(obj_self)->tp_richcompare != Py_TYPE(other)->tp_richcompare) {
Py_INCREF(Py_NotImplemented);
return Py_NotImplemented;
}
result = PyArray_GenericBinaryFunction(self,
(PyObject *)other,
n_ops.equal);
/*
* If the comparison results in NULL, then the
* two array objects can not be compared together;
Expand All @@ -1382,8 +1418,8 @@ array_richcompare(PyArrayObject *self, PyObject *other, int cmp_op)
* is not possible.
*/
PyErr_Clear();
if (DEPRECATE("elementwise comparison failed; "
"this will raise the error in the future.") < 0) {
if (DEPRECATE("elementwise == comparison failed; "
"this will raise an error in the future.") < 0) {
return NULL;
}

Expand All @@ -1400,15 +1436,6 @@ array_richcompare(PyArrayObject *self, PyObject *other, int cmp_op)
Py_INCREF(Py_True);
return Py_True;
}
if (needs_right_binop_forward(obj_self, other, "__ne__", 0) &&
Py_TYPE(obj_self)->tp_richcompare != Py_TYPE(other)->tp_richcompare) {
Py_INCREF(Py_NotImplemented);
return Py_NotImplemented;
}
result = PyArray_GenericBinaryFunction(self, (PyObject *)other,
n_ops.not_equal);
if (result && result != Py_NotImplemented)
break;

/*
* The ufunc does not support void/structured types, so these
Expand All @@ -1426,6 +1453,11 @@ array_richcompare(PyArrayObject *self, PyObject *other, int cmp_op)
*/
if (array_other == NULL) {
PyErr_Clear();
if (DEPRECATE(
"elementwise != comparison failed and returning scalar "
"instead; this will raise an error in the future.") < 0) {
return NULL;
}
Py_INCREF(Py_NotImplemented);
return Py_NotImplemented;
}
Expand All @@ -1434,27 +1466,38 @@ array_richcompare(PyArrayObject *self, PyObject *other, int cmp_op)
PyArray_DESCR(array_other),
NPY_EQUIV_CASTING);
if (_res == 0) {
Py_DECREF(result);
Py_DECREF(array_other);
if (DEPRECATE_FUTUREWARNING(
"elementwise != comparison failed and returning scalar "
"instead; this will raise an error or perform "
"elementwise comparison in the future.") < 0) {
return NULL;
}
Py_INCREF(Py_True);
return Py_True;
}
else {
Py_DECREF(result);
result = _void_compare(self, array_other, cmp_op);
Py_DECREF(array_other);
}
return result;
}

if (needs_right_binop_forward(obj_self, other, "__ne__", 0) &&
Py_TYPE(obj_self)->tp_richcompare != Py_TYPE(other)->tp_richcompare) {
Py_INCREF(Py_NotImplemented);
return Py_NotImplemented;
}
result = PyArray_GenericBinaryFunction(self, (PyObject *)other,
n_ops.not_equal);
if (result == NULL) {
/*
* Comparisons should raise errors when element-wise comparison
* is not possible.
*/
PyErr_Clear();
if (DEPRECATE("elementwise comparison failed; "
"this will raise the error in the future.") < 0) {
if (DEPRECATE("elementwise != comparison failed; "
"this will raise an error in the future.") < 0) {
return NULL;
}

Expand Down Expand Up @@ -1484,24 +1527,6 @@ array_richcompare(PyArrayObject *self, PyObject *other, int cmp_op)
result = Py_NotImplemented;
Py_INCREF(result);
}
if (result == Py_NotImplemented) {
/* Try to handle string comparisons */
if (PyArray_TYPE(self) == NPY_OBJECT) {
return result;
}
array_other = (PyArrayObject *)PyArray_FromObject(other,
NPY_NOTYPE, 0, 0);
if (array_other == NULL) {
PyErr_Clear();
return result;
}
if (PyArray_ISSTRING(self) && PyArray_ISSTRING(array_other)) {
Py_DECREF(result);
result = _strings_richcompare(self, (PyArrayObject *)
array_other, cmp_op, 0);
}
Py_DECREF(array_other);
}
return result;
}

Expand Down
4 changes: 4 additions & 0 deletions numpy/core/src/multiarray/number.c
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,10 @@ PyArray_GenericBinaryFunction(PyArrayObject *m1, PyObject *m2, PyObject *op)
* See also:
* - https://github.com/numpy/numpy/issues/3502
* - https://github.com/numpy/numpy/issues/3503
*
* NB: there's another copy of this code in
* numpy.ma.core.MaskedArray._delegate_binop
* which should possibly be updated when this is.
*/
double m1_prio = PyArray_GetPriority((PyObject *)m1,
NPY_SCALAR_PRIORITY);
Expand Down
Loading