This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Documentation recommends raising TypeError from tp_richcompare
Type: Stage: resolved
Components: Documentation Versions:
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: docs@python Nosy List: Devin Jeanpierre, docs@python, iritkatriel, jdemeyer, mdk, miss-islington, r.david.murray, rhettinger
Priority: normal Keywords: patch

Created on 2017-04-04 22:51 by Devin Jeanpierre, last changed 2022-04-11 14:58 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 987 closed Devin Jeanpierre, 2017-04-04 22:51
PR 16095 merged mdk, 2019-09-13 12:57
PR 16097 merged miss-islington, 2019-09-13 13:07
Messages (8)
msg291144 - (view) Author: Devin Jeanpierre (Devin Jeanpierre) * Date: 2017-04-04 22:51
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.
msg291145 - (view) Author: Devin Jeanpierre (Devin Jeanpierre) * Date: 2017-04-04 23:01
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.
msg291146 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2017-04-04 23:32
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.
msg291149 - (view) Author: Devin Jeanpierre (Devin Jeanpierre) * Date: 2017-04-05 00:15
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.")
msg338940 - (view) Author: Jeroen Demeyer (jdemeyer) * (Python triager) Date: 2019-03-27 06:35
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.
msg352315 - (view) Author: Julien Palard (mdk) * (Python committer) Date: 2019-09-13 13:07
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
msg352317 - (view) Author: miss-islington (miss-islington) Date: 2019-09-13 13:14
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
msg377898 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2020-10-03 18:25
Looks like this issue can be closed.
History
Date User Action Args
2022-04-11 14:58:44adminsetgithub: 74172
2020-10-20 10:19:15iritkatrielsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2020-10-03 18:25:02iritkatrielsetnosy: + iritkatriel
messages: + msg377898
2019-09-13 13:14:45miss-islingtonsetnosy: + miss-islington
messages: + msg352317
2019-09-13 13:07:49miss-islingtonsetpull_requests: + pull_request15717
2019-09-13 13:07:40mdksetnosy: + mdk
messages: + msg352315
2019-09-13 12:57:15mdksetkeywords: + patch
stage: patch review
pull_requests: + pull_request15716
2019-03-27 06:35:00jdemeyersetnosy: + jdemeyer
messages: + msg338940
2017-04-05 00:15:20Devin Jeanpierresetmessages: + msg291149
2017-04-04 23:32:03r.david.murraysetnosy: + rhettinger, r.david.murray
messages: + msg291146
2017-04-04 23:01:22Devin Jeanpierresetmessages: + msg291145
2017-04-04 22:51:53Devin Jeanpierrecreate