[issue29986] Documentation recommends raising TypeError from tp_richcompare

[Irit Katriel]

Change by Irit Katriel :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29986] Documentation recommends raising TypeError from tp_richcompare

[Irit Katriel]

Irit Katriel  added the comment:

Looks like this issue can be closed.

--
nosy: +iritkatriel

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29986] Documentation recommends raising TypeError from tp_richcompare

[miss-islington]

miss-islington  added the comment:


New changeset 4556b1d35c352c975f3cf066362cb6e24efe0668 by Miss Islington (bot) 
in branch '3.8':
bpo-29986: Doc: Delete tip to raise TypeError from tp_richcompare. (GH-16095)
https://github.com/python/cpython/commit/4556b1d35c352c975f3cf066362cb6e24efe0668


--
nosy: +miss-islington

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29986] Documentation recommends raising TypeError from tp_richcompare

[Julien Palard]

Julien Palard  added the comment:


New changeset 375a3e2bdbeb4dce69aba4b5bc90f55fe27e81b4 by Julien Palard in 
branch 'master':
bpo-29986: Doc: Delete tip to raise TypeError from tp_richcompare. (GH-16095)
https://github.com/python/cpython/commit/375a3e2bdbeb4dce69aba4b5bc90f55fe27e81b4


--
nosy: +mdk

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29986] Documentation recommends raising TypeError from tp_richcompare

[miss-islington]

Change by miss-islington :


--
pull_requests: +15717
pull_request: https://github.com/python/cpython/pull/16097

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29986] Documentation recommends raising TypeError from tp_richcompare

[Julien Palard]

Change by Julien Palard :


--
keywords: +patch
pull_requests: +15716
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/16095

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29986] Documentation recommends raising TypeError from tp_richcompare

[Jeroen Demeyer]

Jeroen Demeyer  added the comment:

The consensus is clearly to return NotImplemented in this case, also because 
that's what most builtins do, like the object() example that you mentioned.

However, I would rather keep that note and change it to say return 
NotImplemented. It's an important difference between tp_richcompare and the 6 
Python methods __eq__ and friends. Explicitly saying what do you if you only 
want __eq__ and __ne__ but no other operators (which is not exceptional at all) 
looks useful to me.

--
nosy: +jdemeyer

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29986] Documentation recommends raising TypeError from tp_richcompare

[Devin Jeanpierre]

Devin Jeanpierre added the comment:

Yeah, I agree there might be a use-case (can't find one offhand, but in 
principle), but I think it's rare enough that you're more likely to be led 
astray from reading this note -- almost always, NotImplemented does what you 
want.

In a way this is a special case of being able to raise an exception at all, 
which is mentioned earlier ("if another error occurred it must return NULL and 
set an exception condition.")

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29986] Documentation recommends raising TypeError from tp_richcompare

[R. David Murray]

R. David Murray added the comment:

The documentation is technically correct, as far as I can see.  Issue 8743 is 
not about disallowing certain comparison operations, but rather incorrectly 
implementing the earlier sentence in that same doc section: "If the comparison 
is undefined, it must return Py_NotImplemented".  Perhaps the wording should be 
changed so that instead of saying "only a limited set of comparisons makes 
sense" it says "if you wish to disallow certain comparison operations while 
allowing others".  It should also probably be demoted from being a '.. note', 
since it is, as you note, an exceptional case, not a common one.

But you might be right that it would be better to delete it altogether, since 
it is generally better to let the comparison machinery raise the error if none 
of the types implements the comparison...what would be the rationale for a 
blanket *dis*allowing of a particular comparison operation?  

Let's see what Raymond thinks.

--
nosy: +r.david.murray, rhettinger

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29986] Documentation recommends raising TypeError from tp_richcompare

[Devin Jeanpierre]

Devin Jeanpierre added the comment:

Sorry, forgot to link to docs because I was copy-pasting from the PR:

https://docs.python.org/2/c-api/typeobj.html#c.PyTypeObject.tp_richcompare

https://docs.python.org/3/c-api/typeobj.html#c.PyTypeObject.tp_richcompare

> Note: If you want to implement a type for which only a limited set of 
> comparisons makes sense (e.g. == and !=, but not < and friends), directly 
> raise TypeError in the rich comparison function.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29986] Documentation recommends raising TypeError from tp_richcompare

[Devin Jeanpierre]

New submission from Devin Jeanpierre:

am not sure when TypeError is the right choice. Definitely, most of the time 
I've seen it done, it causes trouble, and NotImplemented usually does something 
better.

For example, see the work in https://bugs.python.org/issue8743 to get set to 
interoperate correctly with other set-like classes --- a problem caused by the 
use of TypeError instead of returning NotImplemented (e.g. 
https://hg.python.org/cpython/rev/3615cdb3b86d).

This advice seems to conflict with the usual and expected behavior of objects 
from Python: e.g. object().__lt__(1) returns NotImplemented rather than raising 
TypeError, despite < not "making sense" for object. Similarly for file objects 
and other uncomparable classes. Even complex numbers only return NotImplemented!


>>> 1j.__lt__(1j)
NotImplemented


If this note should be kept, this section could use a decent explanation of the 
difference between "undefined" (should return NotImplemented) and "nonsensical" 
(should apparently raise TypeError). Perhaps a reference to an example from the 
stdlib.

--
assignee: docs@python
components: Documentation
messages: 291144
nosy: Devin Jeanpierre, docs@python
priority: normal
pull_requests: 1167
severity: normal
status: open
title: Documentation recommends raising TypeError from tp_richcompare

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com