Opened 8 years ago

Closed 8 years ago

#6500 closed (fixed)

Make models hashable

Reported by: Bastian Kleineidam <calvin@…> Owned by: nobody
Component: Uncategorized Version: master
Severity: Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: UI/UX:

Description

The Model instances should be hashable so that tests like
"instance in set([instance1, instance2])" actually succeed.

Attachments (1)

0055-Make-models-hashable.patch (853 bytes) - added by Bastian Kleineidam <calvin@…> 8 years ago.

Download all attachments as: .zip

Change History (7)

Changed 8 years ago by Bastian Kleineidam <calvin@…>

comment:1 Changed 8 years ago by pytechd <pytechd@…>

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Before adding this, please consider adding the app and model name into the hash as well as the PK. Otherwise the hashes of Article(pk=1) and Author(pk=1) are identical.

comment:2 follow-up: Changed 8 years ago by Bastian Kleineidam <calvin@…>

I don't think hash collisions for different models have a big performance penalty (if any). You usually end up having the same type of objects in a set/list (populated from MyModel.objects.all() or similar). The pk gives a good hash data in this case.

comment:3 Changed 8 years ago by Simon Greenhill <dev@…>

  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Ready for checkin

comment:4 in reply to: ↑ 2 Changed 8 years ago by Eric

Replying to Bastian Kleineidam <calvin@debian.org>:

I don't think hash collisions for different models have a big performance penalty (if any). You usually end up having the same type of objects in a set/list (populated from MyModel.objects.all() or similar). The pk gives a good hash data in this case.

That's not entirely true when dealing with the content types framework.

comment:5 Changed 8 years ago by mtredinnick

I implemented a hash function independent of having seen this ticket and arrived at basically the same logic Bastian's using. The common case will be objects of the same type, using the pk as a hash is very fast (quite a bit faster than hashing a tuple of class and pk) and satisfies Python's requirements, and even if you do get the odd collision in the case of mixed content types, it doesn't matter that much. Simplicity wins here.

comment:6 Changed 8 years ago by mtredinnick

  • Resolution set to fixed
  • Status changed from new to closed

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

Note: See TracTickets for help on using tickets.
Back to Top