Any reason why positions should *not* be hashable?

24 views
Skip to first unread message

Edward K. Ream

unread,
Oct 15, 2019, 6:58:26 AM10/15/19
to leo-editor
#1394 suggests making positions hashable. The "hash" branch enables hashing and all seems well.

There is evidence (including a unit test) that disabling hashing for positions was done on purpose.

Can anyone tell me why hashing positions is a bad idea?

Edward

vitalije

unread,
Oct 15, 2019, 7:10:28 AM10/15/19
to leo-editor
We have discussed this before. I remember writing a few examples where it brakes user expectations. From the Python documentation about object.__hash__ method:

If a class defines mutable objects and implements an __eq__() method, it should not implement __hash__(), since the implementation of hashable collections requires that a key’s hash value is immutable (if the object’s hash value changes, it will be in the wrong hash bucket).
 
Positions are mutable objects and therefore they should not implement this method. 

It used to be implemented in the past, but as a result of our discussion it was removed.

Vitalije

Edward K. Ream

unread,
Oct 15, 2019, 7:51:22 AM10/15/19
to leo-editor


On Tue, Oct 15, 2019 at 6:10 AM vitalije <vita...@gmail.com> wrote:
We have discussed this before. I remember writing a few examples where it breaks user expectations. From the Python documentation about object.__hash__ method:

If a class defines mutable objects and implements an __eq__() method, it should not implement __hash__(), since the implementation of hashable collections requires that a key’s hash value is immutable (if the object’s hash value changes, it will be in the wrong hash bucket).
 
Positions are mutable objects and therefore they should not implement this method. 

Thanks for this.  I've added a comment in devel explaining why p.__hash__ should be None.  I've also closed #1394 and marked it Won'tDo.

Edward
Reply all
Reply to author
Forward
0 new messages