[patch]override __hash__ method of django.db.base.Model

add hash to django.db.model.base

There's no need to stringify the primary key before hashing, is there?

hmm... I just remember some strange situation that I meet. anyway it seems no need.

I was totally wrong to open this ticket, since http://docs.python.org/ref/customization.html:

  If a class defines mutable objects and implements a __cmp__() or __eq__() method, 
it should not implement __hash__()

And django model is a mutable object. so we can't implement __hash__().

I think in this case the Python docs were mis-interpreted. The key point to consider when writing a hash function is that two objects that compare equal must hash equally, and the hash value cannot change. Just because an object is mutable doesn't mean that these rules will be violated. A mutable object may have a hash that uses its mutable data as part of the hash implementation, which would be unacceptable. But a mutable object can have a carefully planned hash which only uses the immutable data in its calculation.

In particular, Django model instances will usually simply use their id attribute as the hash, and so will be perfectly acceptable as dictionary keys.

I agree with Ned here, I would expect len(set([Model.objects.get(pk=1), Model.objects.get(pk=1)])) to be 1, but it's 2. I'd be inclined to consider it as a bug.

I concur. Using sets to filter duplicates is a common, efficient idiom. We should be able to do the same with model instances, mutable or not. The very fact that model equality is determined by their id alone makes the question of mutability irrelevant.

Consider a contrived example:

x = set(Foo.objects.all())
for f in Foo.objects.all():
  x.add(f)

As it now stands, you'll end up with two copies of each Foo model in the set, a clear violation of the "principle of least surprise", IMHO.

Please, could you send an email to django-developers? I think this has been rejected by the core developers, but your arguments are valid.

It hasn't been rejected. There's no need for an email, it's a reasonable request. The original ticket burned and died based on mistaken assumptions (as Ned correctly notes) and just never got onto the radar.

(In [7132]) Fixed #2936, #6500 -- Added a hash() method to Models (since we implement our own eq method).