NEP: Update NEP 42 and 43 according to the current implementation

We modified the `resolve_descriptors` function to not use `context`
which is a bit heavy weight, and that also allows removing of
DTypes from the context.

This means it now passes `self, DTypes` (where self is the unbound
ArrayMethod).

Also add a note on the -1 return value of `resolve_descriptors`.

Apply suggestions from code review

Co-authored-by: Ross Barnowski <rossbar@berkeley.edu>

Fixup note on CastingImpl being mainly a NEP 42 name

Fixup sphinx warnings

2 files changed, 53 insertions, 30 deletions

diff --git a/doc/neps/nep-0042-new-dtypes.rst b/doc/neps/nep-0042-new-dtypes.rst
index d1ddb7101..1a77b5718 100644
--- a/doc/neps/nep-0042-new-dtypes.rst
+++ b/doc/neps/nep-0042-new-dtypes.rst
@@ -203,7 +203,7 @@ Other elements of the casting implementation is the ``CastingImpl``:
         # Object describing and performing the cast
         casting : casting
 
-        def resolve_descriptors(self, Tuple[DType] : input) -> (casting, Tuple[DType]):
+        def resolve_descriptors(self, Tuple[DTypeMeta], Tuple[DType|None] : input) -> (casting, Tuple[DType]):
             raise NotImplementedError
 
         # initially private:
@@ -213,6 +213,8 @@ Other elements of the casting implementation is the ``CastingImpl``:
 which describes the casting from one DType to another. In
 :ref:`NEP 43 <NEP43>` this ``CastingImpl`` object is used unchanged to
 support universal functions.
+Note that the name ``CastingImpl`` here will be generically called
+``ArrayMethod`` to accomodate both casting and universal functions.
 
 
 ******************************************************************************
@@ -525,7 +527,8 @@ This means the implementation will work like this::
             # Find what dtype1 is cast to when cast to the common DType
             # by using the CastingImpl as described below:
             castingimpl = get_castingimpl(type(dtype1), common)
-            safety, (_, dtype1) = castingimpl.resolve_descriptors((dtype1, None))
+            safety, (_, dtype1) = castingimpl.resolve_descriptors(
+                    (common, common), (dtype1, None))
             assert safety == "safe"  # promotion should normally be a safe cast
 
         if type(dtype2) is not common:
@@ -652,7 +655,7 @@ and implements the following methods and attributes:
 
 * To report safeness,
 
-  ``resolve_descriptors(self, Tuple[DType] : input) -> casting, Tuple[DType]``.
+  ``resolve_descriptors(self, Tuple[DTypeMeta], Tuple[DType|None] : input) -> casting, Tuple[DType]``.
 
   The ``casting`` output reports safeness (safe, unsafe, or same-kind), and
   the tuple is used for more multistep casting, as in the example below.
@@ -691,7 +694,7 @@ The full process is:
 
 1. Call
 
-   ``CastingImpl[Int24, String].resolve_descriptors((int24, "S20"))``.
+   ``CastingImpl[Int24, String].resolve_descriptors((Int24, String), (int24, "S20"))``.
 
    This provides the information that ``CastingImpl[Int24, String]`` only
    implements the cast of ``int24`` to ``"S8"``.
@@ -716,7 +719,7 @@ The full process is:
 
    to call
 
-   ``CastingImpl[Int24, String].resolve_descriptors((int24, None))``.
+   ``CastingImpl[Int24, String].resolve_descriptors((Int24, String), (int24, None))``.
 
    In this case the result of ``(int24, "S8")`` defines the correct cast:
 
@@ -763,8 +766,8 @@ even if the user provides only an ``int16 -> int24`` cast. This proposal does
 not provide that, but future work might find such casts dynamically, or at least
 allow ``resolve_descriptors`` to return arbitrary ``dtypes``.
 
-If ``CastingImpl[Int8, Int24].resolve_descriptors((int8, int24))`` returns
-``(int16, int24)``, the actual casting process could be extended to include
+If ``CastingImpl[Int8, Int24].resolve_descriptors((Int8, Int24), (int8, int24))``
+returns ``(int16, int24)``, the actual casting process could be extended to include
 the ``int8 -> int16`` cast. This adds a step.
 
 
@@ -774,7 +777,7 @@ The implementation for casting integers to datetime would generally
 say that this cast is unsafe (because it is always an unsafe cast).
 Its ``resolve_descriptors`` function may look like::
 
-     def resolve_descriptors(self, given_dtypes):
+     def resolve_descriptors(self, DTypes, given_dtypes):
         from_dtype, to_dtype = given_dtypes
         from_dtype = from_dtype.ensure_canonical()  # ensure not byte-swapped
         if to_dtype is None:
@@ -835,9 +838,10 @@ Its ``resolve_descriptors`` function may look like::
 
 **Notes:**
 
-The proposed ``CastingImpl`` is designed to be identical to the
-``PyArrayMethod`` proposed in NEP43 as part of restructuring ufuncs to handle
-new DTypes.
+``CastingImpl`` is used as a name in this NEP to clarify that it implements
+all functionality related to a cast. It is meant to be identical to the
+``ArrayMethod`` proposed in NEP 43 as part of restructuring ufuncs to handle
+new DTypes. All type definitions are expected to be named ``ArrayMethod``.
 
 The way dispatching works for ``CastingImpl`` is planned to be limited
 initially and fully opaque. In the future, it may or may not be moved into a
@@ -1297,8 +1301,9 @@ The external API for ``CastingImpl`` will be limited initially to defining:
   instance if the second string is shorter. If neither type is parametric the
   ``resolve_descriptors`` must use it.
 
-* ``resolve_descriptors(dtypes_in[2], dtypes_out[2], casting_out) -> int {0,
-  -1}`` The out
+* ``resolve_descriptors(PyArrayMethodObject *self, PyArray_DTypeMeta *DTypes[2],
+  PyArray_Descr *dtypes_in[2], PyArray_Descr *dtypes_out[2], NPY_CASTING *casting_out)
+  -> int {0, -1}`` The out
   dtypes must be set correctly to dtypes which the strided loop
   (transfer function) can handle.  Initially the result must have instances
   of the same DType class as the ``CastingImpl`` is defined for. The
@@ -1307,9 +1312,12 @@ The external API for ``CastingImpl`` will be limited initially to defining:
   A new, additional flag,
   ``_NPY_CAST_IS_VIEW``, can be set to indicate that no cast is necessary and a
   view is sufficient to perform the cast. The cast should return
-  ``-1`` when a custom error is set and ``NPY_NO_CASTING`` to indicate
-  that a generic casting error should be set (this is in most cases
-  preferable).
+  ``-1`` when an error occurred. If a cast is not possible (but no error
+  occurred), a ``-1`` result should be returned *without* an error set.
+  *This point is under consideration, we may use ``-1`` to indicate
+  a general error, and use a different return value for an impossible cast.*
+  This means that it is *not* possible to inform the user about why a cast is
+  impossible.
 
 * ``strided_loop(char **args, npy_intp *dimensions, npy_intp *strides,
   ...) -> int {0, -1}`` (signature will be fully defined in :ref:`NEP 43 <NEP43>`)
diff --git a/doc/neps/nep-0043-extensible-ufuncs.rst b/doc/neps/nep-0043-extensible-ufuncs.rst
index 96d4794f3..7dbad289b 100644
--- a/doc/neps/nep-0043-extensible-ufuncs.rst
+++ b/doc/neps/nep-0043-extensible-ufuncs.rst
@@ -235,20 +235,30 @@ to define string equality, will be added to a ufunc.
     class StringEquality(BoundArrayMethod):
         nin = 1
         nout = 1
+        # DTypes are stored on the BoundArrayMethod and not on the internal
+        # ArrayMethod, to reference cyles.
         DTypes = (String, String, Bool)
 
-        def resolve_descriptors(context, given_descrs):
+        def resolve_descriptors(self: ArrayMethod, DTypes, given_descrs):
             """The strided loop supports all input string dtype instances
             and always returns a boolean. (String is always native byte order.)
 
             Defining this function is not necessary, since NumPy can provide
             it by default.
+
+            The `self` argument here refers to the unbound array method, so
+            that DTypes are passed in explicitly.
             """
-            assert isinstance(given_descrs[0], context.DTypes[0])
-            assert isinstance(given_descrs[1], context.DTypes[1])
+            assert isinstance(given_descrs[0], DTypes[0])
+            assert isinstance(given_descrs[1], DTypes[1])
+            assert given_descrs[2] is None or isinstance(given_descrs[2], DTypes[2])
             
+            out_descr = given_descrs[2]  # preserve input (e.g. metadata)
+            if given_descrs[2] is None:
+                out_descr = DTypes[2]()
+
             # The operation is always "safe" casting (most ufuncs are)
-            return (given_descrs[0], given_descrs[1], context.DTypes[2]()), "safe"
+            return (given_descrs[0], given_descrs[1], out_descr), "safe"
 
         def strided_loop(context, dimensions, data, strides, innerloop_data):
             """The 1-D strided loop, similar to those used in current ufuncs"""
@@ -422,9 +432,8 @@ a new ``ArrayMethod`` object:
         # More general flags:
         flags: int
 
-        @staticmethod
-        def resolve_descriptors(
-                Context: context, Tuple[DType]: given_descrs)-> Casting, Tuple[DType]:
+        def resolve_descriptors(self,
+                Tuple[DTypeMeta], Tuple[DType|None]: given_descrs) -> Casting, Tuple[DType]:
             """Returns the safety of the operation (casting safety) and the
             """
             # A default implementation can be provided for non-parametric
@@ -468,8 +477,6 @@ With ``Context`` providing mostly static information about the function call:
         int : nin = 1
         # The number of output arguments:
         int : nout = 1
-        # The DTypes this Method operates on/is defined for:
-        Tuple[DTypeMeta] : dtypes
         # The actual dtypes instances the inner-loop operates on:
         Tuple[DType] : descriptors
 
@@ -616,7 +623,8 @@ definitions (see also :ref:`NEP 42 <NEP42>` ``CastingImpl``):
 
       NPY_CASTING
       resolve_descriptors(
-              PyArrayMethod_Context *context,
+              PyArrayMethodObject *self,
+              PyArray_DTypeMeta *dtypes,
               PyArray_Descr *given_dtypes[nin+nout],
               PyArray_Descr *loop_dtypes[nin+nout]);
 
@@ -652,20 +660,20 @@ definitions (see also :ref:`NEP 42 <NEP42>` ``CastingImpl``):
 * The optional ``get_loop`` function will not be public initially, to avoid
   finalizing the API which requires design choices also with casting:
 
-  .. code-block::
+  .. code-block:: C
 
         innerloop *
         get_loop(
             PyArrayMethod_Context *context,
-            /* (move_references is currently used internally for casting) */
             int aligned, int move_references,
             npy_intp *strides,
             PyArray_StridedUnaryOp **out_loop,
             NpyAuxData **innerloop_data,
             NPY_ARRAYMETHOD_FLAGS *flags);
   
-  The ``NPY_ARRAYMETHOD_FLAGS`` can indicate whether the Python API is required
-  and floating point errors must be checked.
+  ``NPY_ARRAYMETHOD_FLAGS`` can indicate whether the Python API is required
+  and floating point errors must be checked. ``move_references`` is used
+  internally for NumPy casting at this time.
 
 * The inner-loop function::
 
@@ -739,6 +747,13 @@ casting can be prepared.
 While the returned casting-safety (``NPY_CASTING``) will almost always be
 "safe" for universal functions, including it has two big advantages:
 
+* ``-1`` indicates that an error occurred. If a Python error is set, it will
+  be raised.  If no Python error is set this will be considered an "impossible"
+  cast and a custom error will be set. (This distinction is important for the
+  ``np.can_cast()`` function, which should raise the first one and return
+  ``False`` in the second case, it is not noteworthy for typical ufuncs).
+  *This point is under consideration, we may use ``-1`` to indicate
+  a general error, and use a different return value for an impossible cast.*
 * Returning the casting safety is central to NEP 42 for casting and
   allows the unmodified use of ``ArrayMethod`` there.
 * There may be a future desire to implement fast but unsafe implementations.