Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#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 Mariusz Felisiak, 6 years ago

Cc: Sergey Fedoseev added
Severity: NormalRelease blocker
Summary: __hash__ in model is None when __eq__ is defined__hash__ not inherited from model.Model if __eq__ is defined
Triage Stage: UnreviewedAccepted

comment:2 by Mariusz Felisiak, 6 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 Carlton Gibson, 6 years ago

Owner: changed from nobody to Carlton Gibson
Status: newassigned

comment:4 by Carlton Gibson, 6 years ago

Resolution: duplicate
Severity: Release blockerNormal
Status: assignedclosed

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 to None. When the __hash__() method of a class is None, instances of the class will raise an appropriate TypeError when a program attempts to retrieve their hash value, and will also be correctly identified as unhashable when checking isinstance(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.

Last edited 6 years ago by Carlton Gibson (previous) (diff)
Note: See TracTickets for help on using tickets.
Back to Top