DOC: update binary-op / ufunc interactions and recommendations in ufunc overrides NEP #9027

shoyer · 2017-04-30T04:32:12Z

Split off and extended from #9014

Adds:

More explicit language about how binary operators work, hopefully assuming less knowledge about how Python defines binary operations (I'm guessing this NEP will be read by users not as familiar with them as we are).
A recommendation to use "Option (1)" for implementing special methods on array-like classes.
More explanation of our reasoning for why opting out of numpy style binary operations requires __array_ufunc__ = None.
A table of binary operators and corresponding numpy ufuncs. I think this will be useful for implementors, e.g., for someone writing a unit library or another mixin class like NDArrayOperatorsMixin.

Because we have listed authors on the NEP, I would like to make sure that every listed author approves of the document before we merge this change:

(Feel free to check off your name on this list if you have edit rights, or I will do so, after you've responded affirmatively in a comment.)

charris · 2017-04-30T16:38:17Z

doc/neps/ufunc-overrides.rst

-provides overrides for all binary operators with corresponding ufuncs.
-
+implementation for :class:`ndarray` itself. We decided to require disabling
+ufuncs to opt out to ensure that a class cannot define Ufuncs to return


Bit confusing, "ufuncs to opt out in order to ensure" would be clearer I think.

charris · 2017-04-30T16:39:51Z

doc/neps/ufunc-overrides.rst

+``-``  ``sub``      :func:`subtract`
+``*``  ``mul``      :func:`multiply`
+``/``  ``truediv``  :func:`true_divide`
+       (Python 3)


Would be better to put this on the line above, same below.

Sphinx does a weird thing if I put it on one line, making the column very wide (I think this is basic on the longer length of the column header ======). So this version ends up looking better when rendered to HTML.

charris · 2017-04-30T16:42:59Z

doc/neps/ufunc-overrides.rst

+``&``  ``and_``     :func:`bitwise_and`
+``^``  ``xor_``     :func:`bitwise_xor`
+``|``  ``or_``      :func:`bitwise_or`
+\      ``divmod``   :func:`floor_divide`, :func:`mod` [*]_


Maybe "--" instead of spaces.

I put in "NA" instead. I am concerned that "--" could be confused with an infix operator.

charris · 2017-04-30T16:44:43Z

doc/neps/ufunc-overrides.rst

+``^``  ``xor_``     :func:`bitwise_xor`
+``|``  ``or_``      :func:`bitwise_or`
+\      ``divmod``   :func:`floor_divide`, :func:`mod` [*]_
+``@``  ``matmul``   Not yet implemented


Maybe "Not yet implemented as a ufunc".

mhvk · 2017-04-30T20:25:08Z

@shoyer - thanks for doing this. I'm happy with your changes.

charris · 2017-04-30T22:31:58Z

doc/neps/ufunc-overrides.rst

@@ -620,8 +620,8 @@ make more sense to ask classes like ``MyObject`` to implement a full
 ``__array_ufunc__`` [6]_. In the end, allowing classes to opt out was
 preferred, and the above reasoning led us to agree on a similar
 implementation for :class:`ndarray` itself. We decided to require disabling
-ufuncs to opt out to ensure that a class cannot define Ufuncs to return
-different results than the corresponding binary operations (i.e., if
+ufuncs to opt out in order to ensure that a class cannot define Ufuncs to


Hmm, now that I look at this in combination with "We decided to require disabling" I still can't figure out what it says ;) I think what this is trying to say is that ufuncs will raise TypeError when one of the arguments opts out in order to ensure ...

I took another pass at this.

charris · 2017-05-01T14:18:52Z

doc/neps/ufunc-overrides.rst

+disabling Ufuncs so a class cannot define a Ufuncs to return a different
+result than the corresponding binary operations (i.e., if
+``np.add(x, y)`` is defined, it should match ``x + y``). Our goal was to
+simply the dispatch logic for binary operations with NumPy arrays


simply <- simplify

charris · 2017-05-01T14:22:08Z

Looks good apart from the wrong word. At some point it could use a bit more polish but for now it is good enough.

njsmith · 2017-05-02T06:24:06Z

doc/neps/ufunc-overrides.rst

+- Otherwise, :class:`ndarray` unilaterally calls the corresponding Ufunc.
+  Ufuncs never return ``NotImplemented``, so **reflexive methods such
+  as** ``other.__rmul__`` **cannot be used to override arithmetic with
+  NumPy arrays if** ``__array_ufunc__`` **is set**. Instead, the resulting


maybe make it "set to any value besides None"?

That's better.

njsmith · 2017-05-02T06:38:57Z

doc/neps/ufunc-overrides.rst

@@ -482,12 +506,12 @@ are not compatible, i.e., implementations should be something like::

        # Option 1: call ufunc directly


BTW, feel free to declare this out-of-scope for this PR, but maybe we should drop the discussion of "option 2" from the NEP now that we've pretty much settled on "option 1" as recommended?

njsmith · 2017-05-02T06:54:22Z

doc/neps/ufunc-overrides.rst

+binary arithmetic with :class:`ndarray` (i.e., they must either implement
+``__array_ufunc__`` or set it to :obj:`None`). We believe this is a good
+thing, because it ensures the consistency of ufuncs and arithmetic on all
+objects that support them.


Oh, I see, you added more discussion of option 1 vs 2 instead. That's also a plausible approach!

I probably wouldn't use "viral" for something we want to recommend... it's a pretty loaded word? Plus it kinda doesn't matter whether people want their classes to be "viral" or not, because this is what ndarray does, so to the extent it's viral they have to follow, and if they don't follow it's not so viral then is it...?

It might be simplest to just delete the option 2 stuff from the NEP; the history's always available in git. Or this is probably fine; honestly it's 11:55 pm and I don't really have brain to track all the details in this giant block of text and that's probably influencing my opinion...

I think as long as the interface is provisional, we should keep both options; even for ndarray itself, we will want to optimize things and might well find it is easier in a slightly different way.

Clarify the role of `array_ufunc` in overriding Python numeric operators.

shoyer · 2017-05-05T21:52:07Z

I guess we're taking silence from @pv and @cowlicks as consent? :)

shoyer mentioned this pull request Apr 30, 2017

BUG: __array_ufunc__= None -> TypeError #9014

Merged

3 tasks

shoyer added this to the 1.13.0 release milestone Apr 30, 2017

DOC: update binary-op/ufunc sections in ufunc-overides NEP

490578e

shoyer force-pushed the ufunc-overrides-nep-update branch from 6b9d342 to 490578e Compare April 30, 2017 05:18

charris reviewed Apr 30, 2017

View reviewed changes

charris added 04 - Documentation component: documentation labels Apr 30, 2017

DOC: fixups per charris's review

07dab4e

charris reviewed Apr 30, 2017

View reviewed changes

Further clarifications

d9bc680

charris reviewed May 1, 2017

View reviewed changes

DOC: simply -> simplify

9e5786b

njsmith reviewed May 2, 2017

View reviewed changes

mhvk mentioned this pull request May 2, 2017

MAINT: post __array_ufunc__ merger to-do list #8892

Closed

11 tasks

DOC: Improve wording in ufunc_override NEP.

eccfe73

Clarify the role of `array_ufunc` in overriding Python numeric operators.

charris merged commit b7d3097 into numpy:master May 5, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DOC: update binary-op / ufunc interactions and recommendations in ufunc overrides NEP #9027

DOC: update binary-op / ufunc interactions and recommendations in ufunc overrides NEP #9027

shoyer commented Apr 30, 2017 •

edited by njsmith

Loading

charris Apr 30, 2017

shoyer Apr 30, 2017

charris Apr 30, 2017

shoyer Apr 30, 2017

charris Apr 30, 2017

shoyer Apr 30, 2017

charris Apr 30, 2017

shoyer Apr 30, 2017

mhvk commented Apr 30, 2017

charris Apr 30, 2017

shoyer May 1, 2017

charris May 1, 2017

charris commented May 1, 2017

njsmith May 2, 2017

charris May 3, 2017

njsmith May 2, 2017

njsmith May 2, 2017

mhvk May 2, 2017

shoyer commented May 5, 2017

		@@ -482,12 +506,12 @@ are not compatible, i.e., implementations should be something like::

		# Option 1: call ufunc directly

DOC: update binary-op / ufunc interactions and recommendations in ufunc overrides NEP #9027

DOC: update binary-op / ufunc interactions and recommendations in ufunc overrides NEP #9027

Conversation

shoyer commented Apr 30, 2017 • edited by njsmith Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mhvk commented Apr 30, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

charris commented May 1, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shoyer commented May 5, 2017

shoyer commented Apr 30, 2017 •

edited by njsmith

Loading