classification
Title: confusing docs with regard to __hash__
Type: behavior Stage: resolved
Components: Documentation, Interpreter Core Versions: Python 3.2, Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: docs@python Nosy List: Arfrever, cvrebert, docs@python, eric.araujo, ethan.furman, ncoghlan, python-dev, r.david.murray, rhettinger
Priority: normal Keywords: patch

Created on 2012-04-18 17:55 by ethan.furman, last changed 2012-09-12 12:01 by rhettinger. This issue is now closed.

Files
File name Uploaded Description Edit
__hash__.diff ethan.furman, 2012-04-18 17:55 review
__hash__2.diff ethan.furman, 2012-04-18 21:38 review
__hash__3.diff ethan.furman, 2012-05-25 16:35 review
Messages (13)
msg158644 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2012-04-18 17:55
From http://docs.python.org/py3k/reference/datamodel.html#object.__hash__
-----------------------------------------------------------------------
Classes which inherit a __hash__() method from a parent class but change the meaning of __eq__() such that the hash value returned is no longer appropriate (e.g. by switching to a value-based concept of equality instead of the default identity based equality) can explicitly flag themselves as being unhashable by setting __hash__ = None in the class definition. Doing so means that not only will instances of the class raise an appropriate TypeError when a program attempts to retrieve their hash value, but they will also be correctly identified as unhashable when checking isinstance(obj, collections.Hashable) (unlike classes which define their own __hash__() to explicitly raise TypeError).

If a class that overrides __eq__() needs to retain the implementation of __hash__() from a parent class, the interpreter must be told this explicitly by setting __hash__ = <ParentClass>.__hash__. Otherwise the inheritance of __hash__() will be blocked, just as if __hash__ had been explicitly set to None.
-----------------------------------------------------------------------

The first paragraph says the user has to change __hash__ if it's different because of changes to __eq__, the second paragraph says __hash__ is automatically removed if __eq__ is changed;  the second paragraph reflects reality.

Proposed change:
-----------------------------------------------------------------------
Classes which change the meaning of __eq__() (thus losing automatic delegation to the parent class' __hash__) can explicitly flag themselves as being unhashable by setting __hash__ = None in the class definition (which is otherwise done implicity). Having __hash__ set to None, either explicitly or implicitly, means that not only will instances of the class raise an appropriate TypeError when a program attempts to retrieve their hash value, but they will also be correctly identified as unhashable when checking isinstance(obj, collections.Hashable) (unlike classes which define their own __hash__() to explicitly raise TypeError).

If a class that overrides __eq__() needs to retain the implementation of __hash__() from a parent class, the interpreter must be told this explicitly by setting __hash__ = <ParentClass>.__hash__.
-----------------------------------------------------------------------

Patch attached.
msg158651 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-04-18 19:30
Some of the sentence phrasing still sounds a bit awkward to me (“[...], means that not only will instances” for example, and I would also remove the parens at the end), but globally I think this is an improvement.
msg158658 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2012-04-18 20:34
Éric Araujo wrote:
> Changes by Éric Araujo <merwok@netwok.org>:
> 
> 
> ----------
> versions: +Python 2.7 -Python 3.1

The docs for 2.7 are correct, as __hash__ is not automatically 
suppressed in that version.
--
~Ethan~
msg158667 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2012-04-18 21:38
More re-writing...
msg161283 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2012-05-21 17:30
Are the changes good?  Can they be commited?
msg161589 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2012-05-25 16:35
Newest changes uploaded.
msg168707 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2012-08-20 22:02
Any problems with the current doc patch?  If not, can it be applied before RC1?
msg170323 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2012-09-11 16:20
RC2 has just been released.  Any chance of this getting in to the final release?  Nobobdy has pointed out any problems with the last update...
msg170326 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2012-09-11 17:02
New changeset 957e1eef3296 by R David Murray in branch '3.2':
#14617: clarify discussion of interrelationship of __eq__ and __hash__.
http://hg.python.org/cpython/rev/957e1eef3296

New changeset c8d60d0c736b by R David Murray in branch 'default':
Merge #14617: clarify discussion of interrelationship of __eq__ and __hash__.
http://hg.python.org/cpython/rev/c8d60d0c736b
msg170327 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-09-11 17:05
I rewrote the section a bit differently than you did in your patch...if you think my changes are not an improvement please let me know.
msg170334 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2012-09-11 17:57
R. David Murray wrote:
> I rewrote the section a bit differently than you did in your patch...if you think my changes are not an improvement please let me know.

This line is incorrect:

A class which defines its own :meth:`__hash__` that explicitly raises a 
:exc:`TypeError` would be incorrectly identified as hashable by an 
``isinstance(obj, collections.Hashable)`` call.

It should be "would not be correctly identified as hashable".

Otherwise, looks great.
msg170336 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2012-09-11 18:01
Ethan Furman wrote:
> Ethan Furman added the comment:
> 
> R. David Murray wrote:
>> I rewrote the section a bit differently than you did in your patch...if you think my changes are not an improvement please let me know.
> 
> This line is incorrect:
> 
> A class which defines its own :meth:`__hash__` that explicitly raises a 
> :exc:`TypeError` would be incorrectly identified as hashable by an 
> ``isinstance(obj, collections.Hashable)`` call.
> 
> It should be "would not be correctly identified as hashable".
> 
> Otherwise, looks great.

Argh.  Nevermind!  It all looks good.  :)
msg170370 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2012-09-12 12:01
This can wait until after the release.  We will have periodic point releases where further doc updates can go in.
History
Date User Action Args
2012-09-20 14:59:39r.david.murraylinkissue15981 superseder
2012-09-12 12:01:43rhettingersetnosy: + rhettinger
messages: + msg170370
2012-09-11 18:01:00ethan.furmansetmessages: + msg170336
2012-09-11 17:57:57ethan.furmansetmessages: + msg170334
2012-09-11 17:05:54r.david.murraysetstatus: open -> closed

nosy: + r.david.murray
messages: + msg170327

resolution: fixed
stage: resolved
2012-09-11 17:02:27python-devsetnosy: + python-dev
messages: + msg170326
2012-09-11 16:20:25ethan.furmansetmessages: + msg170323
2012-08-20 22:06:25pitrousetnosy: + ncoghlan
2012-08-20 22:02:33ethan.furmansetmessages: + msg168707
2012-05-25 16:35:13ethan.furmansetfiles: + __hash__3.diff

messages: + msg161589
2012-05-22 21:01:44cvrebertsetnosy: + cvrebert
2012-05-21 17:30:54ethan.furmansetmessages: + msg161283
2012-04-20 06:45:49Arfreversetnosy: + Arfrever
2012-04-18 21:38:07ethan.furmansetfiles: + __hash__2.diff

messages: + msg158667
2012-04-18 21:01:21eric.araujosetversions: - Python 2.7
2012-04-18 20:34:48ethan.furmansetmessages: + msg158658
2012-04-18 19:30:02eric.araujosetnosy: + eric.araujo
messages: + msg158651
2012-04-18 19:13:43eric.araujosetversions: + Python 2.7, - Python 3.1
2012-04-18 17:55:27ethan.furmancreate