Instance methods compare equal when their self's are equal

[fniessink]

BPO	1617161
Nosy	@loewis, @abalkin, @devdanzin, @serhiy-storchaka, @jdemeyer, @MojoVampire
PRs	bpo-1617161: Make the hash and equality of methods not depending on the value of self. #7848
Files	instancemethod.py: Example method_compare.diff: use identity instead of equality of 'self' 1617161_test_update.diff

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2018-07-31.06:43:10.168>
created_at = <Date 2006-12-16.21:36:32.000>
labels = ['interpreter-core', 'type-bug', '3.8']
title = "Instance methods compare equal when their self's are equal"
updated_at = <Date 2018-07-31.06:43:10.167>
user = 'https://bugs.python.org/fniessink'

bugs.python.org fields:

activity = <Date 2018-07-31.06:43:10.167>
actor = 'serhiy.storchaka'
assignee = 'none'
closed = True
closed_date = <Date 2018-07-31.06:43:10.168>
closer = 'serhiy.storchaka'
components = ['Interpreter Core']
creation = <Date 2006-12-16.21:36:32.000>
creator = 'fniessink'
dependencies = []
files = ['2254', '9666', '34367']
hgrepos = []
issue_num = 1617161
keywords = ['patch']
message_count = 20.0
messages = ['30822', '30823', '30824', '63376', '63413', '63414', '63415', '63421', '63500', '84597', '110512', '110518', '196476', '213201', '305155', '308106', '320168', '320200', '320212', '322729']
nosy_count = 9.0
nosy_names = ['loewis', 'belopolsky', 'fniessink', 'ajaksu2', '_doublep', 'westley.martinez', 'serhiy.storchaka', 'jdemeyer', 'josh.r']
pr_nums = ['7848']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue1617161'
versions = ['Python 3.8']

[fniessink]

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

[loewis]

Armin, can you please comment?

[arigo]

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".

[abalkin]

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

[arigo]

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.

[fniessink]

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

[abalkin]

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.

[doublep]

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.

[arigo]

Attached patch for python trunk with tests.
It makes all three method types use the identity of their 'self'.

[devdanzin]

Confirmed in trunk and py3k.

[BreamoreBoy]

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?

[abalkin]

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.

[serhiy-storchaka]

Could you please update the patch for tip? I looks too desynchronized.

[westleymartinez]

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?

[serhiy-storchaka]

Armin, do you mind to create a pull request on GitHub?

[arigo]

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.

[jdemeyer]

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.

[MojoVampire]

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.

[serhiy-storchaka]

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.

[serhiy-storchaka]

New changeset ac20e0f by Serhiy Storchaka in branch 'master':
bpo-1617161: Make the hash and equality of methods not depending on the value of self. (GH-7848)
ac20e0f