classification
Title: NotImplemented doc section needs update
Type: Stage: resolved
Components: Versions: Python 3.4, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Arfrever, eric.araujo, ethan.furman, ezio.melotti, georg.brandl, gvanrossum, python-dev, r.david.murray, rhettinger, tim.peters
Priority: normal Keywords: patch

Created on 2014-10-31 23:11 by ethan.furman, last changed 2014-11-27 05:20 by ethan.furman. This issue is now closed.

Files
File name Uploaded Description Edit
issue22780.stoneleaf.01.patch ethan.furman, 2014-11-23 16:09 review
Messages (18)
msg230412 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2014-10-31 23:11
https://docs.python.org/3/library/constants.html

current
-------
    Special value which can be returned by the “rich comparison” special methods (__eq__(), __lt__(), and friends), to indicate that the comparison is not implemented with respect to the other type.


more accurate
-------------
    Special value which should be returned by the __dunder__ methods to indicate the requested operation is not implemented with respect to the other type.
msg230414 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2014-10-31 23:49
New changeset 26d0a17affb5 by Ethan Furman in branch 'default':
issue22780: update NotImplemented description
https://hg.python.org/cpython/rev/26d0a17affb5
msg230424 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-11-01 02:06
The replacement of the term  'special methods' and the examples with the jargon (used nowhere else in the docs that I'm aware of) of '__dunder__' makes the text very confusing.  Please restore 'special methods' and the examples...you could include non-comparision methods in the list of examples.

Oh, I see, you use __dunder__ in the enum docs as well.  It should be replaced with our standard terminology there, as well.
msg230447 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2014-11-01 14:25
Here's the actual change:

+   Special value which should be returned by the special methods
+   (:meth:`__eq__`, :meth:`__lt__`, :meth:`__add__`, etc.) to indicate
+   that the operation is not implemented with respect to the other type.

I'll update the Enum docs as well.
msg230561 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2014-11-03 20:37
+   Special value which should be returned by the special methods
+   (:meth:`__eq__`, :meth:`__lt__`, :meth:`__add__`, etc.) to indicate
+   that the operation is not implemented with respect to the other type.

After a discussion on python-dev, I think this wording could be even clearer.  How about:

    Special value which should be returned by the binary special methods
    (e.g. :meth:`__eq__`, :meth:`__lt__`, :meth:`__add__`, :meth:`__rsub__`,
    etc.) to indicate that the operation is not implemented with respect to
    the other type; may be returned by the in-place binary special methods
    (e.g. :meth:`__imul__`, :meth:`__iand__`, etc.) for the same purpose.
msg230564 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-11-03 20:56
Sounds OK to me.  There should already be a discussion of the consequences of returning it (I don't remember where, though), and it would be nice to link to that discussion.

Note that any doc change should be applied to 3.4 first, and then merged to 3.5.
msg230566 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2014-11-03 21:49
I found these items:

Doc/c-api/object.rst
--------------------
.. c:var:: PyObject* Py_NotImplemented

   The ``NotImplemented`` singleton, used to signal that an operation is
   not implemented for the given type combination.


Doc/extending/newtypes.rst
---------------------------
where the operator is one of ``Py_EQ``, ``Py_NE``, ``Py_LE``, ``Py_GT``,
``Py_LT`` or ``Py_GT``.  It should compare the two objects with respect to the
specified operator and return ``Py_True`` or ``Py_False`` if the comparison is
successful, ``Py_NotImplemented`` to indicate that comparison is not
implemented and the other object's comparison method should be tried, or *NULL*
if an exception was set.


Doc/Library/numbers.rst
-----------------------
Implementing the arithmetic operations
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

We want to implement the arithmetic operations so that mixed-mode
operations either call an implementation whose author knew about the
types of both arguments, or convert both to the nearest built in type
and do the operation there. For subtypes of :class:`Integral`, this
means that :meth:`__add__` and :meth:`__radd__` should be defined as::

    class MyIntegral(Integral):

        def __add__(self, other):
            if isinstance(other, MyIntegral):
                return do_my_adding_stuff(self, other)
            elif isinstance(other, OtherTypeIKnowAbout):
                return do_my_other_adding_stuff(self, other)
            else:
                return NotImplemented

        def __radd__(self, other):
            if isinstance(other, MyIntegral):
                return do_my_adding_stuff(other, self)
            elif isinstance(other, OtherTypeIKnowAbout):
                return do_my_other_adding_stuff(other, self)
            elif isinstance(other, Integral):
                return int(other) + int(self)
            elif isinstance(other, Real):
                return float(other) + float(self)
            elif isinstance(other, Complex):
                return complex(other) + complex(self)
            else:
                return NotImplemented


There are 5 different cases for a mixed-type operation on subclasses
of :class:`Complex`. I'll refer to all of the above code that doesn't
refer to ``MyIntegral`` and ``OtherTypeIKnowAbout`` as
"boilerplate". ``a`` will be an instance of ``A``, which is a subtype
of :class:`Complex` (``a : A <: Complex``), and ``b : B <:
Complex``. I'll consider ``a + b``:

    1. If ``A`` defines an :meth:`__add__` which accepts ``b``, all is
       well.
    2. If ``A`` falls back to the boilerplate code, and it were to
       return a value from :meth:`__add__`, we'd miss the possibility
       that ``B`` defines a more intelligent :meth:`__radd__`, so the
       boilerplate should return :const:`NotImplemented` from
       :meth:`__add__`. (Or ``A`` may not implement :meth:`__add__` at
       all.)
    3. Then ``B``'s :meth:`__radd__` gets a chance. If it accepts
       ``a``, all is well.
    4. If it falls back to the boilerplate, there are no more possible
       methods to try, so this is where the default implementation
       should live.
    5. If ``B <: A``, Python tries ``B.__radd__`` before
       ``A.__add__``. This is ok, because it was implemented with
       knowledge of ``A``, so it can handle those instances before
       delegating to :class:`Complex`.


Doc/library/datetime.rst
------------------------
   In other words, ``date1 < date2`` if and only if ``date1.toordinal() <
   date2.toordinal()``. In order to stop comparison from falling back to the
   default scheme of comparing object addresses, date comparison normally raises
   :exc:`TypeError` if the other comparand isn't also a :class:`date` object.
   However, ``NotImplemented`` is returned instead if the other comparand has a
   :meth:`timetuple` attribute.  This hook gives other kinds of date objects a
   chance at implementing mixed-type comparison. If not, when a :class:`date`
   object is compared to an object of a different type, :exc:`TypeError` is raised
   unless the comparison is ``==`` or ``!=``.  The latter cases return
   :const:`False` or :const:`True`, respectively.


Doc/reference/expressions.rst
-----------------------------
Comparisions
============
...
Comparison of objects of differing types depends on whether either of the
types provide explicit support for the comparison.  Most numeric types can be
compared with one another.  When cross-type comparison is not supported, the
comparison method returns ``NotImplemented``.


Ahha!  I think I found it (nearly at the end, of course):

Doc/reference/datamodel.rst
---------------------------
The standard type hierarchy
===========================
...
NotImplemented
   .. index:: object: NotImplemented

   This type has a single value.  There is a single object with this value. This
   object is accessed through the built-in name ``NotImplemented``. Numeric methods
   and rich comparison methods may return this value if they do not implement the
   operation for the operands provided.  (The interpreter will then try the
   reflected operation, or some other fallback, depending on the operator.)  Its
   truth value is true.
msg230567 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-11-03 22:09
I was actually thinking of the Implementing the arithmetic operations section.  Maybe we should copy the parenthetical from the datamodel description into the text you are modifying, and then link to the implementing section.
msg230582 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2014-11-04 07:06
Thank you, Raymond, both for your concern and your discretion.

My interest in changing the "can" or "may" to "should" is that, whatever the original intent of the PEP, the way Python works /now/ is that any class that doesn't return NotImplemented when it /should/ is not going to interoperate well with other compatible classes.

At the heart of the issue is what happens when

  def bin_op(self, other):
     ...

is called, but the left-hand operand doesn't know how to work with the right-hand operand?

We have three choices:

  - do nothing, letting any exceptions boil up or errors propagate

  - do a check on 'other' to determine if it's usable, and raise an exception
    if it is not

  - do a check on 'other' to determine if it's usable, and return NotImplemented
    if it is not

Only the last choice allows 'other' to also try the operation.  Except for the special-case of in-place bin-ops, why would we not want choice three?
msg230584 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2014-11-04 07:38
How about:

    Special value which should be returned by the binary special methods
    (e.g. :meth:`__eq__`, :meth:`__lt__`, :meth:`__add__`, :meth:`__rsub__`,
    etc.) to indicate that the operation is not implemented with respect to
    the other type; may be returned by the in-place binary special methods
    (e.g. :meth:`__imul__`, :meth:`__iand__`, etc.) for the same purpose.
    Its truth value is true.

    Note::
    When NotImplemented is returned, the interpreter will then try the
    reflected operation on the other type, or some other fallback, depending
    on the operator.  If all attempted operations return NotImplemented, the
    interpreter will raise an appropriate exception.


I have no idea how to create a link to the 'Implementing the arithmetic operations' section.  Any clues?
msg230585 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2014-11-04 07:46
You add a label before that section and then reference it with :ref:.
msg230589 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-11-04 08:08
"try the reflected operation" is not our standard terminology.  There is a reason I suggested *copying* the parenthetical statement.  We essentially have two places where NotImplemented is described (language reference and library reference), and the parenthetical is the only substantial piece of information present in one that is not present in the other.
msg230634 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2014-11-04 16:17
R. David Murray said:
--------------------
> "try the reflected operation" is not our standard terminology.

Parenthetical under discussion:
-------------------------------
> (The interpreter will then try the reflected operation, or some other fallback,
> depending on the operator.)
msg230642 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-11-04 17:14
OK, you got me there :)
msg230652 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2014-11-04 19:16
Whew!

If a different wording is better, I'm happy to change both places.  :)
msg231567 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2014-11-23 16:09
Here's the latest patch.  Thoughts?
msg231748 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2014-11-27 05:18
New changeset ebb8865dcf54 by Ethan Furman in branch '3.4':
(3.4) Issue22780: reword NotImplemented docs to emphasise should
https://hg.python.org/cpython/rev/ebb8865dcf54

New changeset b6ee02acaae9 by Ethan Furman in branch 'default':
Issue22780: reword NotImplemented docs to emphasise should
https://hg.python.org/cpython/rev/b6ee02acaae9
msg231749 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2014-11-27 05:20
Thank you, Berker.
History
Date User Action Args
2014-11-27 05:20:44ethan.furmansetstatus: open -> closed
resolution: fixed
messages: + msg231749

stage: patch review -> resolved
2014-11-27 05:18:16python-devsetmessages: + msg231748
2014-11-23 16:09:22ethan.furmansetfiles: + issue22780.stoneleaf.01.patch
keywords: + patch
messages: + msg231567

stage: resolved -> patch review
2014-11-04 19:16:11ethan.furmansetmessages: + msg230652
2014-11-04 17:14:00r.david.murraysetmessages: + msg230642
2014-11-04 16:17:17ethan.furmansetmessages: + msg230634
2014-11-04 11:49:49Arfreversetnosy: + Arfrever
2014-11-04 08:08:34r.david.murraysetmessages: + msg230589
2014-11-04 07:46:13georg.brandlsetmessages: + msg230585
2014-11-04 07:38:36ethan.furmansetmessages: + msg230584
2014-11-04 07:06:21ethan.furmansetmessages: + msg230582
2014-11-04 05:41:27rhettingersetmessages: - msg230579
2014-11-04 05:38:17rhettingersetnosy: + rhettinger, tim.peters, gvanrossum
messages: + msg230579
2014-11-03 22:09:42r.david.murraysetmessages: + msg230567
2014-11-03 21:49:22ethan.furmansetmessages: + msg230566
2014-11-03 20:56:26r.david.murraysetmessages: + msg230564
versions: + Python 3.4
2014-11-03 20:37:18ethan.furmansetstatus: closed -> open

messages: + msg230561
2014-11-01 14:25:01ethan.furmansetmessages: + msg230447
2014-11-01 02:06:08r.david.murraysetnosy: + r.david.murray
messages: + msg230424
2014-10-31 23:50:56ethan.furmansetstatus: open -> closed
stage: needs patch -> resolved
versions: + Python 3.5
2014-10-31 23:49:13python-devsetnosy: + python-dev
messages: + msg230414
2014-10-31 23:11:11ethan.furmancreate