classification
Title: Instance methods compare equal when their self's are equal
Type: behavior Stage: resolved
Components: Interpreter Core Versions: Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: _doublep, ajaksu2, belopolsky, fniessink, jdemeyer, josh.r, loewis, serhiy.storchaka, westley.martinez
Priority: normal Keywords: patch

Created on 2006-12-16 21:36 by fniessink, last changed 2018-07-31 06:43 by serhiy.storchaka. This issue is now closed.

Files
File name Uploaded Description Edit
instancemethod.py fniessink, 2006-12-16 21:36 Example
method_compare.diff arigo, 2008-03-13 09:21 use identity instead of equality of 'self' review
1617161_test_update.diff westley.martinez, 2014-03-12 06:07 review
Pull Requests
URL Status Linked Edit
PR 7848 merged serhiy.storchaka, 2018-06-21 18:47
Messages (20)
msg30822 - (view) Author: Frank Niessink (fniessink) Date: 2006-12-16 21:36
Python 2.5 was changed so that instance methods compare equal when their im_self attributes compare equal. Here's a link to that change: http://svn.python.org/view?rev=46739&view=rev

This is a problem if we want to distinguish between methods of instances that compare equal, for example when methods can be registered as callbacks (see attached example).

It seems unlogical to me that whether or not the instance methods of two different instances are equal or not depends on the equality of the instance.

Thanks, Frank
msg30823 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2006-12-16 23:07
Armin, can you please comment?
msg30824 - (view) Author: Armin Rigo (arigo) * (Python committer) Date: 2006-12-17 12:19
I see.  Indeed, in the callback situation the 2.5 change
is not very good.  On the other hand, I have a (possibly
more obscure) case where the new equality makes more
sense.  Note also that the change was meant to unify
the behavior of built-in and user method objects; e.g.
if you use callbacks as dict keys, then already in
previous Python versions it was impossible to use say
'mylist.append' as a callback.  Moreover, the hash of
user methods was strangely based on the hash of the
object itself already, which made dict key collisions
likely.

All in all I think that this part was an accident
and never designed; I won't pronounce myself and
suggest that python-dev should decide which of the
two behaviors is realy "expected".
msg63376 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2008-03-08 01:25
> the change was meant to unify
> the behavior of built-in and
> user method objects

I don't think it achieved that.  Consider:

>>> [].index == [].index
False

but
>>> UserList().index == UserList().index         
True
msg63413 - (view) Author: Armin Rigo (arigo) * (Python committer) Date: 2008-03-09 12:15
My mistake, you are right.  I forgot about one of the many types that
CPython uses internally for built-in methods.  Indeed:

    >>> [].index == [].index
    False
    >>> [].__add__ == [].__add__
    True

I can fix this so that the answer is True in all cases; alternatively,
considering Frank Niessink's original issue, we could bring this
discussion to python-dev and decide what the correct behavior should be
and implement it consistently for all method types.
msg63414 - (view) Author: Frank Niessink (fniessink) Date: 2008-03-09 13:13
Just to reiterate the original bug report: the issue (for me) is that
currently (python 2.5):
>>> [].__add__  == [].__add__
True
>>> [].__add__  == [1].__add__
False

Or, using a non-builtin class:
>>> class C(object):
...   def __eq__(self, other):
...     return False
...   def foo(self):
...      pass
...
>>> C().foo == C().foo
False
>>> class C(object):
...   def __eq__(self, other):
...     return True
...   def foo(self):
...     pass
...
>>> C().foo == C().foo
True

I think it makes little sense that the equality test for the instance
methods takes the equality of the instances into account. Imho, this
behaviour is inconsistent with the principle of no surprises. The
correct behaviour (again imho of course) is that instance methods only
compare equal to the same instance method of the same instance, where 
'same instance' is based on 'is' not on '=='.  

Cheers, Frank
msg63415 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2008-03-09 13:14
Armin,

I think this should be discussed on python-dev.  I don't understand your 
motivation for comparing methods of distinct but equal objects as equal.  
The callback argument on the other hand, is compelling.
msg63421 - (view) Author: Paul Pogonyshev (_doublep) Date: 2008-03-09 15:20
Since I'm not on python-dev, I'll mention here that the new behavior
makes no sense to me at all.  Which is best highlighted by Frank in
msg63414.
msg63500 - (view) Author: Armin Rigo (arigo) * (Python committer) Date: 2008-03-13 09:21
Attached patch for python trunk with tests.
It makes all three method types use the identity of their 'self'.
msg84597 - (view) Author: Daniel Diniz (ajaksu2) (Python triager) Date: 2009-03-30 18:03
Confirmed in trunk and py3k.
msg110512 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2010-07-16 22:06
msg84597 is ambiguous.  At first readng I thought it meant "the patches have been committed to trunk and py3k".  Second time I think it means "yes this is still a problem".  What is the status?
msg110518 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-07-16 22:29
I believe "Confirmed in trunk and py3k." means that the issue was verified to exist in trunk and py3k branches at the time of the message.
msg196476 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-08-29 18:25
Could you please update the patch for tip? I looks too desynchronized.
msg213201 - (view) Author: Westley Martínez (westley.martinez) * Date: 2014-03-12 06:07
I updated the tests to be in sync, but the implementation of the fix is not so trivial.  The conversion from cmp() to rich comparison is the primary culprit, so it will take time for me to get familiar enough with the C source to update the fix.  I couldn't seem to get the patch to apply even to the 2.x branch (I think it's because it's an SVN patch...) to see if the fix actually works.

That said, this enhancement is so old that it might not warrant a fix at all.  Maybe it should be brought up on python-dev?
msg305155 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-10-28 10:56
Armin, do you mind to create a pull request on GitHub?
msg308106 - (view) Author: Armin Rigo (arigo) * (Python committer) Date: 2017-12-12 10:29
Sorry, I think it is pointless to spend efforts to keep a relatively uncontroversial and small patch up-to-date, when it was not accepted in 9 years.  Someone else would need to take it up.
msg320168 - (view) Author: Jeroen Demeyer (jdemeyer) * (Python triager) Date: 2018-06-21 11:30
I just posted to python-dev about this issue:

https://mail.python.org/pipermail/python-dev/2018-June/153959.html

However, I think that comparing using "==" is the right thing to do. So I think that

>>> [].append == [].append
False

should really return True.
msg320200 - (view) Author: Josh Rosenberg (josh.r) * (Python triager) Date: 2018-06-22 00:48
If [].append == [].append is True in the "unique set of callbacks" scenario, that implies that it's perfectly fine to not call one of them when both are registered. But this means that only one list ends up getting updated, when you tried to register both for updates. That's definitely surprising, and not in a good way.
msg320212 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-06-22 07:06
PR 7848 is based on method_compare.diff and 1617161_test_update.diff, but also makes types.MethodWrapperType unorderable, adds more tests and fixes some outdated tests.

This change can be considered as a bugfix, but since it can break the user code (unlikely), it may be safer to merge it only in 3.8 and expose as a new feature.

But changes to the hash of types.BuiltinMethodType may be backported. Currently the hash is not consistent with the equality.
msg322729 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-07-31 06:18
New changeset ac20e0f98d6727ba97a9575bfa2a11b2f6247c35 by Serhiy Storchaka in branch 'master':
bpo-1617161: Make the hash and equality of methods not depending on the value of self. (GH-7848)
https://github.com/python/cpython/commit/ac20e0f98d6727ba97a9575bfa2a11b2f6247c35
History
Date User Action Args
2018-07-31 06:43:10serhiy.storchakasetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2018-07-31 06:18:26serhiy.storchakasetmessages: + msg322729
2018-06-22 07:06:07serhiy.storchakasetmessages: + msg320212
versions: + Python 3.8, - Python 3.6, Python 3.7
2018-06-22 00:48:31josh.rsetnosy: + josh.r
messages: + msg320200
2018-06-21 20:42:19arigosetnosy: - arigo
2018-06-21 18:47:03serhiy.storchakasetstage: needs patch -> patch review
pull_requests: + pull_request7458
2018-06-21 11:30:33jdemeyersetnosy: + jdemeyer
messages: + msg320168
2018-06-21 10:55:07serhiy.storchakalinkissue33925 superseder
2017-12-12 10:29:01arigosetmessages: + msg308106
2017-10-28 10:56:13serhiy.storchakasettype: enhancement -> behavior
messages: + msg305155
versions: + Python 3.6, Python 3.7, - Python 3.4
2014-03-13 11:35:46arigosetassignee: arigo ->
2014-03-12 06:07:51westley.martinezsetfiles: + 1617161_test_update.diff

messages: + msg213201
2014-03-11 20:29:34westley.martinezsetnosy: + westley.martinez
2014-02-03 17:02:15BreamoreBoysetnosy: - BreamoreBoy
2013-08-29 18:25:27serhiy.storchakasetmessages: + msg196476
stage: patch review -> needs patch
2013-01-04 18:42:11serhiy.storchakasetnosy: + serhiy.storchaka

versions: + Python 3.4, - Python 3.2
2010-08-09 04:20:14terry.reedysetversions: + Python 3.2, - Python 3.1, Python 2.7
2010-07-16 22:29:20belopolskysetmessages: + msg110518
2010-07-16 22:06:36BreamoreBoysetnosy: + BreamoreBoy
messages: + msg110512
2009-03-30 18:03:54ajaksu2setversions: + Python 3.1, Python 2.7, - Python 2.5
nosy: + ajaksu2

messages: + msg84597

type: enhancement
stage: patch review
2008-03-13 09:21:04arigosetfiles: + method_compare.diff
keywords: + patch
messages: + msg63500
2008-03-09 15:20:46_doublepsetnosy: + _doublep
messages: + msg63421
2008-03-09 13:14:30belopolskysetmessages: + msg63415
2008-03-09 13:13:49fniessinksetmessages: + msg63414
2008-03-09 12:15:03arigosetmessages: + msg63413
2008-03-08 01:25:41belopolskysetnosy: + belopolsky
messages: + msg63376
2006-12-16 21:36:32fniessinkcreate