msg58124 - (view) |
Author: Thomas Herve (therve) * |
Date: 2007-12-03 13:51 |
A new behavior was introduced in r59106 in python trunk, which look
suspicious to me. Basically, every time a class defines a comparison
operator, it's __hash__ method is defined to None. The simple following
example shows it:
>>> class A(object):
... def __init__(self, b):
... self.b = b
... def __cmp__(self, other):
... if not isinstance(other, A):
... return -1
... return cmp(self.b, other.b)
...
>>> hash(A(2))
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: 'NoneType' object is not callable
The problematic change is here:
http://svn.python.org/view/python/trunk/Objects/typeobject.c?rev=59106&r1=58032&r2=59106
And mainly the overrides_hash call in inherit_slots.
FWIW, I've encounter the problem because zope.interface is now usable on
trunk.
|
msg58125 - (view) |
Author: Thomas Herve (therve) * |
Date: 2007-12-03 13:52 |
Of course, I meant unusable.
|
msg58126 - (view) |
Author: Thomas Herve (therve) * |
Date: 2007-12-03 14:15 |
Also, to be more clear, that happens when you define any of the
functions in the name_op array (__lt__, __le___, __gt__, __ge__, ...).
It's not just a problem about the object being non hashable.
|
msg58140 - (view) |
Author: Guido van Rossum (gvanrossum) *  |
Date: 2007-12-03 19:09 |
This was done intentionally -- if you define a comparison without
defining a hash, the default hash will not match your comparison, and
your objects will misbehave when used as dictionary keys.
Can you give more details about the Zope issue? I agree that needs to
be resolved before we release 2.6.
|
msg58141 - (view) |
Author: Jean-Paul Calderone (exarkun) *  |
Date: 2007-12-03 19:15 |
zope.interface.interface.InterfaceClass implements __lt__ and __gt__.
It doesn't override any of the other comparison methods, and it relies
on inheriting an identity-based __hash__ from its parent.
Disabling __hash__ when __cmp__ or __eq__ is defined makes sense, but it
does not seem like it needs to be disabled if only __lt__ or __gt__ is
defined.
|
msg58148 - (view) |
Author: Guido van Rossum (gvanrossum) *  |
Date: 2007-12-03 19:32 |
You have a point. Can you suggest a patch that fixes this?
Question though -- what semantics are used for __lt__ and __gt__ that
they don't require overriding __eq__?
|
msg58153 - (view) |
Author: Jean-Paul Calderone (exarkun) *  |
Date: 2007-12-03 20:31 |
It implements ordering based on the interface's fully-qualified Python
name (ie, (__module__, __name__)). This is compatible with the default
__eq__ implementation.
|
msg58154 - (view) |
Author: Guido van Rossum (gvanrossum) *  |
Date: 2007-12-03 20:51 |
Fair enough.
Still, this would go much faster if someone other than myself could be
coerced into researching how to fix this (at least the Zope use -- I
haven't heard back from the OP).
|
msg58156 - (view) |
Author: Thomas Herve (therve) * |
Date: 2007-12-03 21:23 |
There are different ways to fix this. Reverting r59016 is one of them,
but I guess it's not an acceptable solution :).
From what I understand of the changes, removing the operations (__lt__,
__gt__, __le__, __ge__) from the name_op array would fix this. But I
have to understand why they have been put in there in the first place.
I'll try tomorrow, you assign this to me if you want.
|
msg58157 - (view) |
Author: Guido van Rossum (gvanrossum) *  |
Date: 2007-12-03 21:32 |
Well, the name_op array is used for other purposes. Perhaps the main
issue is that the presence of any of these causes the "rich comparison"
functionality to be overridden. But please do look into this if you can!
|
msg58182 - (view) |
Author: Thomas Herve (therve) * |
Date: 2007-12-04 11:02 |
I gave it a try. The following patch corrects 2 problems:
* classes with only __lt__ and __gt__ defined are hashable
* classes defining __eq__ get a meaningful exception.
I've restricted the hash_name_op to the bare minimum, but no tests
failed, so I supposed it was enough :).
Please review.
|
msg58803 - (view) |
Author: Thomas Herve (therve) * |
Date: 2007-12-19 10:23 |
This is probably not the best moment (holidays etc), but please take
this issue seriously. This problem completely block the community
builders at pybots.org, and make any tests on current python trunk
impossible. This also means that if in the meantime another regression
appear, it will be even more difficult to solve it afterwards.
Also, zope is not the only one affected, storm seems to suffer from the
problem too (not exacly the same, though). As twisted is also unusable
without zope.interface, that makes several project that can't follow
python developpement at all (zope.interface error happens at import
time, for the record).
If this problem isn't solved in a reasonable time, it really questions
the future of the community builders and their purpose.
Thanks.
|
msg58817 - (view) |
Author: Guido van Rossum (gvanrossum) *  |
Date: 2007-12-19 18:33 |
To be honest, I have no idea what pybots is, and how it's affected.
Maybe you can get someone else interested in reviewing the code? I'm
focusing mostly on Py3k.
|
msg58836 - (view) |
Author: Guido van Rossum (gvanrossum) *  |
Date: 2007-12-19 22:51 |
Committed revision 59576.
I found some time.
Happy pybotting!
|
msg58866 - (view) |
Author: Thomas Herve (therve) * |
Date: 2007-12-20 09:31 |
Thanks a lot!
|
|
Date |
User |
Action |
Args |
2022-04-11 14:56:28 | admin | set | github: 45890 |
2007-12-20 09:31:22 | therve | set | messages:
+ msg58866 |
2007-12-19 22:51:37 | gvanrossum | set | status: open -> closed resolution: accepted messages:
+ msg58836 |
2007-12-19 18:33:40 | gvanrossum | set | messages:
+ msg58817 |
2007-12-19 10:23:57 | therve | set | messages:
+ msg58803 |
2007-12-18 22:37:08 | gvanrossum | set | keywords:
+ patch |
2007-12-18 22:37:03 | gvanrossum | set | priority: high -> normal |
2007-12-04 11:02:52 | therve | set | files:
+ 1549_1.diff messages:
+ msg58182 |
2007-12-03 21:32:48 | gvanrossum | set | messages:
+ msg58157 |
2007-12-03 21:23:39 | therve | set | messages:
+ msg58156 |
2007-12-03 20:51:28 | gvanrossum | set | messages:
+ msg58154 |
2007-12-03 20:31:57 | exarkun | set | messages:
+ msg58153 |
2007-12-03 19:32:33 | gvanrossum | set | messages:
+ msg58148 |
2007-12-03 19:15:04 | exarkun | set | messages:
+ msg58141 |
2007-12-03 19:09:46 | gvanrossum | set | messages:
+ msg58140 |
2007-12-03 14:40:53 | christian.heimes | set | priority: high assignee: gvanrossum nosy:
+ gvanrossum |
2007-12-03 14:15:45 | therve | set | messages:
+ msg58126 |
2007-12-03 13:52:35 | therve | set | nosy:
+ exarkun messages:
+ msg58125 |
2007-12-03 13:51:31 | therve | create | |