[issue29986] Documentation recommends raising TypeError from tp_richcompare
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 <report@bugs.python.org> <http://bugs.python.org/issue29986> _______________________________________
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 <report@bugs.python.org> <http://bugs.python.org/issue29986> _______________________________________
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 <report@bugs.python.org> <http://bugs.python.org/issue29986> _______________________________________
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 <report@bugs.python.org> <http://bugs.python.org/issue29986> _______________________________________
Jeroen Demeyer <J.Demeyer@UGent.be> 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 <report@bugs.python.org> <https://bugs.python.org/issue29986> _______________________________________
Change by Julien Palard <julien+python@palard.fr>: ---------- keywords: +patch pull_requests: +15716 stage: -> patch review pull_request: https://github.com/python/cpython/pull/16095 _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue29986> _______________________________________
Julien Palard <julien+python@palard.fr> 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/375a3e2bdbeb4dce69aba4b5bc90f55fe27... ---------- nosy: +mdk _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue29986> _______________________________________
Change by miss-islington <mariatta.wijaya+miss-islington@gmail.com>: ---------- pull_requests: +15717 pull_request: https://github.com/python/cpython/pull/16097 _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue29986> _______________________________________
miss-islington <mariatta.wijaya+miss-islington@gmail.com> 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/4556b1d35c352c975f3cf066362cb6e24ef... ---------- nosy: +miss-islington _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue29986> _______________________________________
Irit Katriel <iritkatriel@yahoo.com> added the comment: Looks like this issue can be closed. ---------- nosy: +iritkatriel _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue29986> _______________________________________
Change by Irit Katriel <iritkatriel@yahoo.com>: ---------- resolution: -> fixed stage: patch review -> resolved status: open -> closed _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue29986> _______________________________________
participants (6)
-
Devin Jeanpierre -
Irit Katriel
-
Jeroen Demeyer
-
Julien Palard
-
miss-islington
-
R. David Murray