DOC,MAINT: Address "simple" review comments by Marten

4 files changed, 32 insertions, 27 deletions

diff --git a/numpy/core/include/numpy/experimental_dtype_api.h b/numpy/core/include/numpy/experimental_dtype_api.h
index 48941baec..05fb1d936 100644
--- a/numpy/core/include/numpy/experimental_dtype_api.h
+++ b/numpy/core/include/numpy/experimental_dtype_api.h
@@ -342,8 +342,9 @@ typedef enum {
  * If an identity exists, should set the `NPY_METH_ITEM_IS_IDENTITY`, normally
  * the `NPY_METH_ITEM_IS_DEFAULT` should also be set, but it is distinct.
  * By default NumPy provides a "default" for `object` dtype, but does not use
- * it as an identity.
- * The `NPY_METH_IS_REORDERABLE` flag should be set if the operatio is
+ * it as an identity (this is e.g. to allows reducing even Python strings
+ * for `np.sum()` while the empty sum returns 0).
+ * The `NPY_METH_IS_REORDERABLE` flag should be set if the operation is
  * reorderable.
  *
  * NOTE: `item` can be `NULL` when a user passed a custom initial value, in
diff --git a/numpy/core/src/multiarray/array_method.c b/numpy/core/src/multiarray/array_method.c
index 0cc2310fd..e9b1c3e83 100644
--- a/numpy/core/src/multiarray/array_method.c
+++ b/numpy/core/src/multiarray/array_method.c
@@ -166,7 +166,7 @@ npy_default_get_strided_loop(
 }
 
 
-/* Declared in `ufunc_object.c` */
+/* TODO: Declared in `ufunc_object.c`, should be included more directly */
 NPY_NO_EXPORT PyObject *
 PyUFunc_GetIdentity(PyUFuncObject *ufunc, npy_bool *reorderable);
 
@@ -182,36 +182,43 @@ default_get_identity_function(PyArrayMethod_Context *context,
 
     PyUFuncObject *ufunc = (PyUFuncObject *)context->caller;
     if (!PyObject_TypeCheck(ufunc, &PyUFunc_Type)) {
-        /* Should not happen, but if it does, just give up */
-        return 0;
+        /*
+         * Could also just report no identity/reorderable, but NumPy would
+         * never get here and it is unclear that anyone else will either.
+         */
+        PyErr_SetString(PyExc_NotImplementedError,
+                "default `get_identity_function` requires a ufunc context; "
+                "Please contact the NumPy developers if you have questions.");
+        return -1;
     }
 
     npy_bool reorderable;
     PyObject *identity_obj = PyUFunc_GetIdentity(ufunc, &reorderable);
+
     if (identity_obj == NULL) {
         return -1;
     }
     if (reorderable) {
         *flags |= NPY_METH_IS_REORDERABLE;
     }
-    if (item == NULL) {
-        /* Only reorderable flag was requested (user provided initial value) */
+
+    if (item == NULL || identity_obj == Py_None) {
+        /*
+         * Only reorderable flag was requested (user provided initial value)
+         * or there is no identity/default value to report.
+         */
         Py_DECREF(identity_obj);
         return 0;
     }
 
-    if (identity_obj != Py_None) {
-        /* We do not consider the value an identity for object dtype */
-        *flags |= NPY_METH_ITEM_IS_DEFAULT;
-        if (context->descriptors[0]->type_num != NPY_OBJECT) {
-            *flags |= NPY_METH_ITEM_IS_IDENTITY;
-        }
-        int res = PyArray_Pack(context->descriptors[0], item, identity_obj);
-        Py_DECREF(identity_obj);
-        return res;
+    /* Report the default value and identity unless object dtype */
+    *flags |= NPY_METH_ITEM_IS_DEFAULT;
+    if (context->descriptors[0]->type_num != NPY_OBJECT) {
+        *flags |= NPY_METH_ITEM_IS_IDENTITY;
     }
+    int res = PyArray_Pack(context->descriptors[0], item, identity_obj);
     Py_DECREF(identity_obj);
-    return 0;
+    return res;
 }
 
 
@@ -300,7 +307,7 @@ fill_arraymethod_from_slots(
     /* Set the defaults */
     meth->get_strided_loop = &npy_default_get_strided_loop;
     meth->resolve_descriptors = &default_resolve_descriptors;
-    meth->get_identity = default_get_identity_function;
+    meth->get_identity = &default_get_identity_function;
 
     /* Fill in the slots passed by the user */
     /*
diff --git a/numpy/core/src/multiarray/array_method.h b/numpy/core/src/multiarray/array_method.h
index ea022a7e6..0715bcd39 100644
--- a/numpy/core/src/multiarray/array_method.h
+++ b/numpy/core/src/multiarray/array_method.h
@@ -221,12 +221,12 @@ typedef struct PyArrayMethodObject_tag {
     NPY_ARRAYMETHOD_FLAGS flags;
     resolve_descriptors_function *resolve_descriptors;
     get_loop_function *get_strided_loop;
+    get_identity_function  *get_identity;
     /* Typical loop functions (contiguous ones are used in current casts) */
     PyArrayMethod_StridedLoop *strided_loop;
     PyArrayMethod_StridedLoop *contiguous_loop;
     PyArrayMethod_StridedLoop *unaligned_strided_loop;
     PyArrayMethod_StridedLoop *unaligned_contiguous_loop;
-    get_identity_function  *get_identity;
     /* Chunk only used for wrapping array method defined in umath */
     struct PyArrayMethodObject_tag *wrapped_meth;
     PyArray_DTypeMeta **wrapped_dtypes;
@@ -263,10 +263,10 @@ extern NPY_NO_EXPORT PyTypeObject PyBoundArrayMethod_Type;
 #define NPY_METH_resolve_descriptors 1
 #define NPY_METH_get_loop 2
 #define NPY_METH_strided_loop 3
-#define NPY_METH_contiguous_loop 4
-#define NPY_METH_unaligned_strided_loop 5
-#define NPY_METH_unaligned_contiguous_loop 6
-#define NPY_METH_get_identity 7
+#define NPY_METH_get_identity 4
+#define NPY_METH_contiguous_loop 5
+#define NPY_METH_unaligned_strided_loop 6
+#define NPY_METH_unaligned_contiguous_loop 7
 
 
 /*
diff --git a/numpy/core/src/umath/reduction.c b/numpy/core/src/umath/reduction.c
index bdb4e6a44..fa37bdef0 100644
--- a/numpy/core/src/umath/reduction.c
+++ b/numpy/core/src/umath/reduction.c
@@ -149,9 +149,7 @@ PyArray_CopyInitialReduceValues(
  * context     : The ArrayMethod context (with ufunc, method, and descriptors).
  * operand     : The array to be reduced.
  * out         : NULL, or the array into which to place the result.
- * wheremask   : NOT YET SUPPORTED, but this parameter is placed here
- *               so that support can be added in the future without breaking
- *               API compatibility. Pass in NULL.
+ * wheremask   : Reduction mask of valid values used for `where=`.
  * axis_flags  : Flags indicating the reduction axes of 'operand'.
  * keepdims    : If true, leaves the reduction dimensions in the result
  *               with size one.
@@ -160,7 +158,6 @@ PyArray_CopyInitialReduceValues(
  * initial     : Initial value, if NULL the default is fetched from the
  *               ArrayMethod (typically as the default from the ufunc).
  * loop        : `reduce_loop` from `ufunc_object.c`.  TODO: Refactor
- * data        : Data which is passed to the inner loop.
  * buffersize  : Buffer size for the iterator. For the default, pass in 0.
  * funcname    : The name of the reduction function, for error messages.
  * errormask   : forwarded from _get_bufsize_errmask