On Wednesday, August 7, 2013 10:45:01 AM UTC+3, Anssi Kääriäinen wrote: > > The way Model.__eq__ works currently should be improved. There is one > definite bug in __eq__, that is deferred instances aren't never equal to > non-deferred instances with same PK. There are also two possible > improvements: make proxy models equal to their concrete parents (this also > fixes deferred __eq__) and make multitable inherited models equal to their > parents. > > There are a couple of tickets about these issues: > https://code.djangoproject.com/ticket/11892, > https://code.djangoproject.com/ticket/16458 and > https://code.djangoproject.com/ticket/14492 > > A patch for proxy equality is available at > https://github.com/akaariai/django/tree/model_eq_proxy and for model > inheritance equality at > https://github.com/akaariai/django/tree/model_eq_inherit. > > I think proxy equality is the right way to go. The reason is backwards > compatibility. A proxy model always represents the same data as its parent, > so change in proxy equality seems somewhat safe. This is not true for > multitable inheritance equality. For one breakage caused by inheritance > equality see the admin_views tests in model_eq_inherit branch. It seems > pretty much impossible to have a deprecation period for this change. > > So, I am planning to go forward with the model_eq_proxy branch. Any > comments or objections? Is even proxy equality too big of a change from > backwards compatibility perspective? >
The model_eq_proxy branch just went in. There are still two tickets that are related to Model.__eq__. These are #18250 and #18864. They both deal with instances which do not yet have any primary key value. Currently Django gives them hash value of hash(self.pk) == hash(None), and compares them as self.pk == other.pk, that is all pk == None instances are equal. This isn't helpful at all. The obvious choice is to make model.__eq__ use "self is other" in case of no primary key value (or equivalently id(self) == id(other)). If __eq__ is based on id(), then __hash__ should be too. Unfortunately this leads to mutable hash value when an instance is saved. So, for __hash__ there seems to be at least three choices: 1) Just don't care about mutable hash value problem. It is there already, in the form of hash(None) to hash(pk value). But the current situation isn't that bad as no-pk-value instance's aren't that useful in hash based containers (you can have only one instance without pk value in them). 2) Prevent hashing of no pk value instances. 3) Allow hashing of no pk value instances, but throw an error if the same instance is hashed both with and without primary key value. I think option 2) is the way to go. While it can cause backwards compatibility problems, putting instances without primary key value into containers hasn't been that useful in previous Django versions. You could after all have just one instance without pk value in the container. It is extremely likely that any current use of no-pk-value instances in containers contains a bug or unwanted behaviour. There is one backwards incompatibility for option 2) in Django. A test tries to validate an inline formset with a base instance that doesn't have a primary key value. While the test makes sense the overall situation doesn't - the formset does validate but saving it leads to exception. The validation code is trying to check unique_together conditions by putting the instance together with the other field's value into a set. This seems suspicious, as all instances without pk value hash to same bucket. I am unsure if the validation code actually contains a bug or not. Option 1) will very likely lead to hard to solve bugs. Option 3) has the same problem but is isn't as bad. For example this will not work: a = MyModel() mydict[a] = 'foo' a.save() MyModel.objects.get(pk=a.pk) in mydict > False Alex Gaynor also gave support for approach 2) in https://code.djangoproject.com/ticket/18864#comment:7 a year ago. A branch implementing id() based __eq__ and preventing __hash__ for no primary key value instances is available at https://github.com/akaariai/django/tree/model_eq_nopk. - Anssi -- You received this message because you are subscribed to the Google Groups "Django developers" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. Visit this group at http://groups.google.com/group/django-developers. For more options, visit https://groups.google.com/groups/opt_out.
