REVERT: remove __array_ufunc__ override for np.matmul.

numpy · charris · Apr 27, 2017 · Nov 6, 2016 · Nov 9, 2016 · Nov 12, 2016
commit 36e84948a448c74efda008a9629c68e9fbb0a218
diff --git a/doc/neps/ufunc-overrides.rst b/doc/neps/ufunc-overrides.rst
@@ -548,16 +548,16 @@ classes, we will provide a mixin class that provides overrides for all
 binary operators.
 
 
-Extension to other numpy functions
-----------------------------------
+Future extensions to other functions
+------------------------------------
 
-The ``__array_ufunc__`` method is also used to override
-:func:`~numpy.matmul`, since while this function is not a Ufunc, it is
-very similar.  Indeed, :func:`~numpy.matmul` may well be implemented as
-a (generalized) Ufunc in the future, as may happen with some other
-functions, such as :func:`~numpy.median`, :func:`~numpy.min`,
-:func:`~numpy.argsort`, etc. (in which case it will thus become possible
-to override these as well).
+Some numpy functions could be implemented as (generalized) Ufunc, in
+which case it would be possible for them to be overridden by the
+``__array_ufunc__`` method.  A prime candidate is :func:`~numpy.matmul`,
+which currently is not a Ufunc, but could be relatively easily be
+rewritten as a (set of) generalized Ufuncs. The same may happen with
+functions such as :func:`~numpy.median`, :func:`~numpy.min`, and
+:func:`~numpy.argsort`.
 
 Demo
 ====
@@ -576,16 +576,17 @@ proposed in this NEP.  Here is a demo highlighting the functionality.::
 
     In [4]: b = B()
 
-    In [5]: np.matmul(a, b)
+    In [5]: np.negative(b)
     Out[5]: 'B'
 
     In [6]: np.multiply(a, b)
     Out[6]: 'B'
 
-As a simple example, one could add the following ``__array_ufunc__`` to
-SciPy's sparse matrices (just for ``np.matmul`` and ``np.multiply`` as
-these are the two most common cases where users would attempt to use
-sparse matrices with ufuncs)::
+As a simple example, once ``np.matmul`` is covered as well (see above),
+one could add the following ``__array_ufunc__`` to SciPy's sparse
+matrices (just for ``np.matmul`` and ``np.multiply`` as these are the
+two most common cases where users would attempt to use sparse matrices
+with ufuncs)::
 
     def __array_ufunc__(self, func, method, pos, inputs, **kwargs):
         """Method for compatibility with NumPy's ufuncs and matmul

diff --git a/doc/source/reference/arrays.classes.rst b/doc/source/reference/arrays.classes.rst
@@ -73,13 +73,14 @@ NumPy provides several hooks that classes can customize:
    :func:`__array_ufunc__` operations return :obj:`NotImplemented`, a
    :exc:`TypeError` is raised.
 
-   .. note:: In addition to ufuncs, :func:`__array_ufunc__` also
-      overrides the behavior of :func:`numpy.matmul`, even though it
-      is not a ufunc. But it can be thought of as :ref:`generalized
-      universal functions<c-api.generalized-ufuncs>` (which are
-      overridden). Indeed, we intend to rewrite :func:`numpy.matmul`
-      as such, and possibly do the same to other relevant functions,
-      such as :func:`numpy.median`, :func:`numpy.argsort`, etc.
+   .. note:: We intend to re-implement numpy functions as (generalized)
+       Ufunc, in which case it will become possible for them to be
+       overridden by the ``__array_ufunc__`` method.  A prime candidate is
+       :func:`~numpy.matmul`, which currently is not a Ufunc, but could be
+       relatively easily be rewritten as a (set of) generalized Ufuncs. The
+       same may happen with functions such as :func:`~numpy.median`,
+       :func:`~numpy.min`, and :func:`~numpy.argsort`.
+
 
    Like with some other special methods in python, such as ``__hash__`` and
    ``__iter__``, it is possible to indicate that your class does *not*

diff --git a/numpy/core/src/multiarray/multiarraymodule.c b/numpy/core/src/multiarray/multiarraymodule.c
@@ -2187,7 +2187,7 @@ array_matrixproduct(PyObject *NPY_UNUSED(dummy), PyObject *args, PyObject* kwds)
                                      kwlist, &a, &v, &o)) {
         return NULL;
     }
-    if (o) {
+    if (o != NULL) {
         if (o == Py_None) {
             o = NULL;
         }
@@ -2327,14 +2327,10 @@ array_vdot(PyObject *NPY_UNUSED(dummy), PyObject *args)
  * out:        Either NULL, or an array into which the output should be placed.
  *
  * Returns NULL on error.
- * Returns NotImplemented on priority override.
  */
 static PyObject *
 array_matmul(PyObject *NPY_UNUSED(m), PyObject *args, PyObject* kwds)
 {
-    static PyObject *matmul = NULL;
-    int errval;
-    PyObject *override = NULL;
     PyObject *in1, *in2, *out = NULL;
     char* kwlist[] = {"a", "b", "out", NULL };
     PyArrayObject *ap1, *ap2, *ret = NULL;
@@ -2345,47 +2341,33 @@ array_matmul(PyObject *NPY_UNUSED(m), PyObject *args, PyObject* kwds)
     char *subscripts;
     PyArrayObject *ops[2];
 
-    npy_cache_import("numpy.core.multiarray", "matmul", &matmul);
-    if (matmul == NULL) {
-        return NULL;
-    }
-
-    errval = PyUFunc_CheckOverride((PyUFuncObject*)matmul, "__call__",
-                                   args, kwds, &override, 2, 1);
-    if (errval) {
-        return NULL;
-    }
-    else if (override) {
-        return override;
-    }
-
     if (!PyArg_ParseTupleAndKeywords(args, kwds, "OO|O:matmul", kwlist,
                                      &in1, &in2, &out)) {
         return NULL;
     }
 
-    if (out == Py_None) {
-        out = NULL;
-    }
-    if (out != NULL && !PyArray_Check(out)) {
-        PyErr_SetString(PyExc_TypeError,
-                        "'out' must be an array");
-        return NULL;
+    if (out != NULL) {
+        if (out == Py_None) {
+            out = NULL;
+        }
+        else if (!PyArray_Check(out)) {
+            PyErr_SetString(PyExc_TypeError, "'out' must be an array");
+            return NULL;
+        }
     }
 
     dtype = PyArray_DescrFromObject(in1, NULL);
     dtype = PyArray_DescrFromObject(in2, dtype);
     if (dtype == NULL) {
-        PyErr_SetString(PyExc_ValueError,
-                "Cannot find a common data type.");
+        PyErr_SetString(PyExc_ValueError, "Cannot find a common data type.");
         return NULL;
     }
     typenum = dtype->type_num;
 
     if (typenum == NPY_OBJECT) {
         /* matmul is not currently implemented for object arrays */
         PyErr_SetString(PyExc_TypeError,
-                "Object arrays are not currently supported");
+                        "Object arrays are not currently supported");
         Py_DECREF(dtype);
         return NULL;
     }
@@ -2407,7 +2389,7 @@ array_matmul(PyObject *NPY_UNUSED(m), PyObject *args, PyObject* kwds)
     if (PyArray_NDIM(ap1) == 0 || PyArray_NDIM(ap2) == 0) {
         /* Scalars are rejected */
         PyErr_SetString(PyExc_ValueError,
-                "Scalar operands are not allowed, use '*' instead");
+                        "Scalar operands are not allowed, use '*' instead");
         return NULL;
     }
 

diff --git a/numpy/core/tests/test_multiarray.py b/numpy/core/tests/test_multiarray.py
@@ -5254,43 +5254,6 @@ def test_matrix_matrix_values(self):
         res = self.matmul(m12, m21)
         assert_equal(res, tgt12_21)
 
-    def test_array_ufunc_override(self):
-
-        class B(np.ndarray):
-            def __new__(cls, *args, **kwargs):
-                return np.array(*args, **kwargs).view(cls)
-
-            def __array_ufunc__(self, ufunc, method, *inputs, **kwargs):
-                return "B"
-
-        class C(np.ndarray):
-            def __new__(cls, *args, **kwargs):
-                return np.array(*args, **kwargs).view(cls)
-
-            def __array_ufunc__(self, ufunc, method, *inputs, **kwargs):
-                return NotImplemented
-
-        class D(np.ndarray):
-            def __new__(cls, *args, **kwargs):
-                return np.array(*args, **kwargs).view(cls)
-
-            __array_ufunc__ = None
-
-        a = np.ones(2)
-        b = B([1, 2])
-        c = C([1, 2])
-        d = D([1, 
8000
2])
-        assert_equal(self.matmul(b, a), "B")
-        assert_equal(self.matmul(a, b), "B")
-        assert_equal(self.matmul(b, c), "B")
-        assert_equal(self.matmul(c, b), "B")
-        assert_equal(self.matmul(b, d), "B")
-        assert_equal(self.matmul(d, b), "B")
-        assert_raises(TypeError, self.matmul, a, c)
-        assert_raises(TypeError, self.matmul, c, a)
-        assert_raises(TypeError, self.matmul, d, a)
-        assert_raises(TypeError, self.matmul, a, d)
-
 
 class TestMatmul(MatmulCommon, TestCase):
     matmul = np.matmul

diff --git a/numpy/core/tests/test_umath.py b/numpy/core/tests/test_umath.py
@@ -1577,19 +1577,19 @@ def __array_ufunc__(self, func, method, *inputs, **kwargs):
         a = A()
         b = np.matrix([1])
         res0 = np.multiply(a, b)
-        res1 = np.matmul(a, b)
+        res1 = np.multiply(b, b, out=a)
 
         # self
         assert_equal(res0[0], a)
         assert_equal(res1[0], a)
         assert_equal(res0[1], np.multiply)
-        assert_equal(res1[1], np.matmul)
+        assert_equal(res1[1], np.multiply)
         assert_equal(res0[2], '__call__')
         assert_equal(res1[2], '__call__')
         assert_equal(res0[3], (a, b))
-        assert_equal(res1[3], (a, b))
+        assert_equal(res1[3], (b, b))
         assert_equal(res0[4], {})
-        assert_equal(res1[4], {})
+        assert_equal(res1[4], {'out': (a,)})
 
     def test_ufunc_override_mro(self):
 
@@ -1878,8 +1878,9 @@ def __array_ufunc__(self, *a, **kwargs):
                 raise ValueError("oops")
 
         a = A()
-        for func in [np.divide, np.matmul]:
-            assert_raises(ValueError, func, a, a)
+        assert_raises(ValueError, np.negative, 1, out=a)
+        assert_raises(ValueError, np.negative, a)
+        assert_raises(ValueError, np.divide, 1., a)
 
     def test_gufunc_override(self):
         # gufunc are just ufunc instances, but follow a different path,