Issue1549
Created on 2007-12-03 13:51 by therve, last changed 2022-04-11 14:56 by admin. This issue is now closed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| 1549_1.diff | therve, 2007-12-04 11:02 | |||
| Messages (15) | |||
|---|---|---|---|
| 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! |
|||
| History | |||
|---|---|---|---|
| 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 | |
