#30333 closed Bug (duplicate)
__hash__ is not inherited from model.Model if __eq__ is defined
| Reported by: | Adam Janik | Owned by: | Carlton Gibson |
|---|---|---|---|
| Component: | Database layer (models, ORM) | Version: | 2.2 |
| Severity: | Normal | Keywords: | |
| Cc: | Sergey Fedoseev | Triage Stage: | Accepted |
| Has patch: | no | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
In my model, that inherits directly from models.Model, I defined my own __eq__:
class Meta:
ordering = ('pk', )
def __eq__(self, other):
return self.as_data() == other.as_data()
What was strange, when updating to 2.2, classes like above (and instances) do not have __hash__ defined anymore.
>>> from myapp.account.models import Address >>> Address.__hash__ is None True >>> import django >>> django.VERSION (2, 2, 0, 'final', 0)
while it was certainly present in 2.1
>>> from myapp.account.models import Address >>> Address.__hash__ <function Model.__hash__ at 0x7f013aee4d08> >>> import django >>> django.VERSION (2, 1, 8, 'final', 0)
I am not sure if it is intended, as I did not find (or maybe missed) mentions about it in 2.2 release notes.
Change History (4)
comment:1 by , 7 years ago
| Cc: | added |
|---|---|
| Severity: | Normal → Release blocker |
| Summary: | __hash__ in model is None when __eq__ is defined → __hash__ not inherited from model.Model if __eq__ is defined |
| Triage Stage: | Unreviewed → Accepted |
comment:2 by , 7 years ago
| Summary: | __hash__ not inherited from model.Model if __eq__ is defined → __hash__ is not inherited from model.Model if __eq__ is defined |
|---|
comment:3 by , 7 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:4 by , 7 years ago
| Resolution: | → duplicate |
|---|---|
| Severity: | Release blocker → Normal |
| Status: | assigned → closed |
Ultimately this is a slightly different version of #30254, which was raised after a68ea231012434b522ce45c513d84add516afa60.
(Will resolve as a duplicate.)
Prior to the changes in a68ea231012434b522ce45c513d84add516afa60 the model would
inherit both __eq__() and __hash__() from models.Model during __new__().
(With __eq__() then being replaced in the add_to_class() call later.)
This was incorrect. The Python docs are quite specific:
A class that overrides
__eq__()and does not define__hash__()will have its__hash__()implicitly set toNone. When the__hash__()method of a class isNone, instances of the class will raise an appropriateTypeErrorwhen a program attempts to retrieve their hash value, and will also be correctly identified as unhashable when checkingisinstance(obj, collections.abc.Hashable).
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__.
By passing the defined __eq__() (as well as __hash__() if defined) into __new__(), the change is bringing Django's behaviour into line with Python's.
The changes here were a bug fix, rather than a breaking change per se. (Yes, it is a
change in behaviour, but so are all bugfixes, and we don't list them as
breaking changes in the release notes.)
I've added a couple test cases demonstrating the expected behaviour in a PR. In particular the test_missing_hash_not_inherited() cases fails on Django 2.1.
The fix for your project is to explicitly declare the __hash__ attribute as per the docs above:
__hash__ = models.Model.__hash__
I hope that helps.
Reproduced at 755673e1bca7edb6bee7a958f40d9ae54d85d44c.
Regression in a68ea231012434b522ce45c513d84add516afa60.