classification
Title: functools.total_ordering does not correctly implement not equal behaviour
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: David Seddon, ebarry, ncoghlan, rhettinger, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2015-11-25 13:12 by David Seddon, last changed 2017-09-25 11:48 by serhiy.storchaka. This issue is now closed.

Files
File name Uploaded Description Edit
total_ordering_ne.diff rhettinger, 2015-11-28 08:13
total_ordering_ne2.diff rhettinger, 2015-11-28 09:26
total_ordering_ne3.diff rhettinger, 2015-11-28 22:32 Add more tests.
Pull Requests
URL Status Linked Edit
PR 3748 merged serhiy.storchaka, 2017-09-25 10:12
Messages (13)
msg255339 - (view) Author: David Seddon (David Seddon) Date: 2015-11-25 13:12
The documentation for functools.total_ordering states that rich comparison can be enabled on a class by specifying an __eq__ method, and one of __lt__(), __le__(), __gt__(), or __ge__().  If these instructions are followed, this results in an incorrect evaluation of the not equal operator:

Here's an example:

    from functools import total_ordering

    @total_ordering
    class Value(object):

        def __init__(self, value):
            self.value = value
    
        def __eq__(self, other):
            return self.value == other.value
    
        def __lt__(self, other):
            return self.value < other.value

 
    >>> a = Value(3)
    >>> b = Value(3)
    >>> a == b
    True
    >>> a != b
    True

I've tested this on 2.7.10.
 
Either the documentation or the behaviour should be corrected.

https://docs.python.org/2/library/functools.html#functools.total_ordering
msg255350 - (view) Author: Emanuel Barry (ebarry) * (Python triager) Date: 2015-11-25 13:39
This is due to the fact that Python 3 added the ability to define only __eq__ and get a free __ne__ defined. If my memory serves me right, functools.total_ordering was added in 3.2 and then backported to 2.x - where the relationship with __eq__ and __ne__ is not present. total_ordering doesn't do anything to touch __ne__ as it expects Python itself to do so (which it doesn't in 2.x).
msg255530 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2015-11-28 08:13
The docs are correct but are apparently misleading ("rich comparison methods" means all six comparison methods while "rich comparison ordering methods" means only the four that provide order).  That said, I think it would be perfectly reasonable to amend the code to supply __ne__ if it is missing.  That would be reduce the likelihood of bugs arising when supplied only __eq__ and one of the four ordering methods but not __ne__.
msg255531 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-11-28 08:43
Now defined __ne__ always silently overridden.
msg255537 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-11-28 11:19
This doesn't work with new-style classes that always have __ne__ inherited from object.

I afraid that the only way to fix this issue is to backport Python 3 behavior of default __ne__, automatically fallback to __eq__.
msg255545 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2015-11-28 16:04
In Python 2.7, __ne__ isn't inherited from object.
msg255546 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-11-28 16:25
Oh, sorry. Then the patch LGTM.
msg255554 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2015-11-28 22:31
Nick, I'm inclined to put this in.  Only bugs can come out of the current arrangement.   Do you have any further thoughts?
msg267330 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2016-06-04 22:07
Sorry for the delayed review. I agree it makes sense to fix this for 2.7, and the proposed solution looks good to me.

However, the latest patch looks it was only partway through editing - the additional test cases seem to be copies of the existing test cases, rather than new scenarios.
msg277819 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-10-01 14:35
Ping.
msg302935 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2017-09-25 09:29
Serhiy, do you want to bring this to fruition?
msg302946 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-09-25 11:41
New changeset d94a65a0694a188713f91ba7c9fffded090247dc by Serhiy Storchaka in branch '2.7':
bpo-25732: Make functools.total_ordering implementing __ne__. (#3748)
https://github.com/python/cpython/commit/d94a65a0694a188713f91ba7c9fffded090247dc
msg302947 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-09-25 11:47
I committed the patch from Raymond's name, but GitHub set me as the author of the commit. :(
History
Date User Action Args
2017-09-25 11:48:21serhiy.storchakasetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2017-09-25 11:47:53serhiy.storchakasetmessages: + msg302947
2017-09-25 11:41:37serhiy.storchakasetmessages: + msg302946
2017-09-25 10:12:27serhiy.storchakasetstage: commit review -> patch review
pull_requests: + pull_request3735
2017-09-25 09:29:21rhettingersetassignee: rhettinger -> serhiy.storchaka
messages: + msg302935
2016-10-01 14:35:40serhiy.storchakasetmessages: + msg277819
2016-06-04 22:07:51ncoghlansetassignee: ncoghlan -> rhettinger
messages: + msg267330
2015-11-28 22:32:26rhettingersetfiles: + total_ordering_ne3.diff
2015-11-28 22:31:54rhettingersetassignee: ncoghlan
messages: + msg255554
2015-11-28 16:25:57serhiy.storchakasetmessages: + msg255546
stage: needs patch -> commit review
2015-11-28 16:04:53rhettingersetmessages: + msg255545
2015-11-28 15:51:37rhettingersetassignee: ncoghlan -> (no value)
2015-11-28 11:19:51serhiy.storchakasetmessages: + msg255537
2015-11-28 09:26:01rhettingersetfiles: + total_ordering_ne2.diff
assignee: rhettinger -> ncoghlan
2015-11-28 09:18:23rhettingersetassignee: ncoghlan -> rhettinger
2015-11-28 08:43:23serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg255531
2015-11-28 08:13:15rhettingersetfiles: + total_ordering_ne.diff
assignee: ncoghlan
messages: + msg255530

keywords: + patch
2015-11-25 14:52:04ebarrysetassignee: docs@python -> (no value)

components: - Documentation
nosy: - docs@python
2015-11-25 14:44:08r.david.murraysetassignee: docs@python

nosy: + docs@python
components: + Documentation
stage: needs patch
2015-11-25 14:03:13serhiy.storchakasetnosy: + rhettinger, ncoghlan
components: + Library (Lib)
2015-11-25 13:39:44ebarrysetnosy: + ebarry
messages: + msg255350
2015-11-25 13:12:49David Seddoncreate