[issue10912] PyObject_RichCompare differs in behaviour from PyObject_RichCompareBool ; difference not noted in documentation
New submission from Devin Jeanpierre <jeanpierreda@gmail.com>: PyObject_RichCoareBool is, according to the documentation in trunk (Doc\c-api\object.rst), exactly the same as PyObject_RichCompare, except it 1, 0, or -1 for error, false, and true respectively. However, it differs in behavior by both that, and also by assuming that identity implies equality. This noted in a two year-old ML post (sadly, no bug report was posted as best as I can find): http://mail.python.org/pipermail/python-list/2009-March/1195170.html Ideally PyObject_RichCompareBool should probably be named something else, since it can no longer be used, strictly, as "PyObject_RichCompare, but returning a C bool" (or, rather, a C int). Some suggestions were offered in the mailing list thread above. I'm filing this as a documentation bug because I find that outcome unlikely. At least the documentation should note the difference in behavior, so that people do not accidentally write C code that does not behave as intended. This issue is related to, but different from issue 4296, which objected to the new container behavior created by the change to PyObject_RichCompareBool. My only objection here is that the latter change does not appear to be documented. I would supply a patch for the tests, but PyObject_RichCompareBool is apparently not directly tested anywhere, just tested obliquely through testing the containment operator, and this was changed by the same commit that changed PyObject_RichCompareBool. I don't know how to word the (very small!) change to the docs. ---------- assignee: docs@python components: Documentation messages: 126306 nosy: Devin Jeanpierre, docs@python priority: normal severity: normal status: open title: PyObject_RichCompare differs in behaviour from PyObject_RichCompareBool ; difference not noted in documentation _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue10912> _______________________________________
Eli Bendersky <eliben@gmail.com> added the comment: I agree that the difference in behavior between the two functions is unfortunate, but that's unlikely to change now. The least we can do is make the documentation precise. I'm attaching a proposed patch to Doc/c-api/object.rst Nick/Terry/Georg - if this looks OK I can commit. ---------- keywords: +easy, patch nosy: +eli.bendersky, ncoghlan, terry.reedy -Devin Jeanpierre type: -> behavior Added file: http://bugs.python.org/file20413/issue10912.py3k.1.patch _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue10912> _______________________________________
Georg Brandl <georg@python.org> added the comment: Sure. ---------- nosy: +georg.brandl _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue10912> _______________________________________
Eli Bendersky <eliben@gmail.com> added the comment: Committed the fix to py3k in r88009 ---------- _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue10912> _______________________________________
Eli Bendersky <eliben@gmail.com> added the comment: Backport to release31 branch in r88010 ---------- resolution: -> fixed status: open -> closed _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue10912> _______________________________________
Nick Coghlan <ncoghlan@gmail.com> added the comment: For the record, the gory details as to *why* RichCompareBool makes more assumptions as to the meaning of equality than the basic RichCompare function can be found in issue 4296 (I just found that issue myself by looking at Mark's response to the python-list thread). It may be worth explicitly pointing out that use cases where this assumption is unacceptable would be better served by direct invocation RichCompare function. ---------- nosy: +mark.dickinson _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue10912> _______________________________________
Eli Bendersky <eliben@gmail.com> added the comment: Nick, I read the protocol of issue 4296 and I understand the reasoning behind the code, but I still think the naming is mightily confusing. Two distinct changes went into a single function (A) Return -1/0/+1 instead of PyObject and (B) the id() shortcut, and its name is inappropriate. Were an API change feasible, some other naming would be better.
"It may be worth explicitly pointing out that use cases where this assumption is unacceptable would be better served by direct invocation RichCompare function."
Do you mean write it down in the docs? ---------- _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue10912> _______________________________________
Nick Coghlan <ncoghlan@gmail.com> added the comment: Yeah, to prevent perfectly reasonable "why" questions, it is probably worth providing a little extra justification as an addendum to your new note (which is already an improvement on the complete silence on the topic that existed before). A possible addition: "... This assumption allows invariants such as "x in [x]" to be more easily guaranteed by the interpreter. If the assumption is not valid for a given use case, call PyObject_RichCompare() directly instead of using this function." For 3.3, it *may* make sense to provide a PyObject_RichCompareBoolEx() function which includes an additional "reflexive" parameter. Then the existing PyObject_RichCompareBool() semantics would just be the new function with the reflexive argument set to 1. ---------- _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue10912> _______________________________________
Changes by Éric Araujo <merwok@netwok.org>: ---------- nosy: +Devin Jeanpierre _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue10912> _______________________________________
Changes by Raymond Hettinger <rhettinger@users.sourceforge.net>: ---------- nosy: +rhettinger _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue10912> _______________________________________
Raymond Hettinger <rhettinger@users.sourceforge.net> added the comment: -1 on PyObject_RichCompareBoolEx() for 3.3 -- it is simply an invitation to shoot yourself (or others) in the foot. I've not seen real world code using bool(a==b) or its C equivalent, so it's hard to believe that there is a use case here (unless you're trying to commit atrocities with NaN-like objects). ---------- _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue10912> _______________________________________
Nick Coghlan <ncoghlan@gmail.com> added the comment: Indeed. I was actually wondering if it would be worth trying to write up a section for the language reference to describe the cases where a Python implementation is *expected* to assume reflexive equality. We (IMO) have a problem at the moment due to situations like:
class PyContains(list): ... def __contains__(self, obj): ... return any(x==obj for x in self) ... nan = float("nan") nan in [nan] True nan in PyContains([nan]) False
This is a bug in the __contains__ definition (it should use "x is obj or x == obj" rather than just the latter expression) but there isn't anything in the language reference to point that out. Assuming reflexive equality in some places and not in others based on the underlying implementation language is going to be a source of subtle bugs relating to types like float and decimal.Decimal. ---------- _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue10912> _______________________________________
Raymond Hettinger <rhettinger@users.sourceforge.net> added the comment: If something needs to be written about NaNs or other invariant destroying objects, perhaps a FAQ entry would suffice (perhaps referencing http://bertrandmeyer.com/2010/02/06/reflexivity-and-other-pillars-of-civiliz... ). ---------- assignee: docs@python -> rhettinger keywords: -easy _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue10912> _______________________________________
Eli Bendersky <eliben@gmail.com> added the comment: Raymond, I initially set "easy" on this issue because I considered it a documentation issue, not the place to resolve the harder debate of the semantics of these functions. Perhaps I was wrong? Also, I agree with Nick that the difference must be further clarified in "stronger words". If there's no debate on this issue, I will commit a fix after the freeze is over. ---------- _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue10912> _______________________________________
Raymond Hettinger <rhettinger@users.sourceforge.net> added the comment: Yes, it is a doc issue but sometimes those aren't easy to get right (in terms of being beneficial to the reader and in good alignment with the design intentions). Please attach the doc patch you want to go in and I'll review it sometime the freeze. ---------- priority: normal -> low _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue10912> _______________________________________
Nick Coghlan <ncoghlan@gmail.com> added the comment: Reopening, as a 2.7 backport of this would be a nice thing to have. ---------- assignee: rhettinger -> status: closed -> open versions: +Python 2.7 _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue10912> _______________________________________
Roundup Robot <devnull@devnull> added the comment: New changeset d27f95e3b52f by Eli Bendersky in branch '2.7': Issue #10912: add clarification for PyObject_RichCompareBool comparing identical objects http://hg.python.org/cpython/rev/d27f95e3b52f ---------- nosy: +python-dev _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue10912> _______________________________________
Changes by Eli Bendersky <eliben@gmail.com>: ---------- status: open -> closed _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue10912> _______________________________________
participants (7)
-
Devin Jeanpierre -
Eli Bendersky
-
Georg Brandl
-
Nick Coghlan
-
Raymond Hettinger
-
Roundup Robot
-
Éric Araujo