Per a discussion in django-users[1], I'm requesting that getters and setters be added to fields in Django models, for the same basic reasons that "property" is a useful keyword in Python.
Ticket 3418[2] contains a proposed patch. The code is small, but I post it here because the potential impact is large. Theoretically, this patch should have no affect on any prior code and should not have any problems with any fields, but I could be wrong.
> Per a discussion in django-users[1], I'm requesting that getters and > setters be added to fields in Django models, for the same basic reasons > that "property" is a useful keyword in Python.
> Ticket 3418[2] contains a proposed patch. The code is small, but I post > it here because the potential impact is large. Theoretically, this > patch should have no affect on any prior code and should not have any > problems with any fields, but I could be wrong.
The patch looks pretty good to me; it applies cleanly and all the unit tests pass (you get major props for including 'em on the first pass :).
However, I do have one quibble: the addition of ``self._django_initializing`` inside ``Model.__init__`` has a pretty nasty "smell" to it. Is that there to prevent the setter from being called during initialization? If so, why?
Also, a few lines of documentation need to be added (to docs/model-api.txt) before this can be checked in. If you can remove the ``_django_initializing`` flag (or at least explain why you need it) and write a few lines of docs, I'll happily check this in.
Jacob Kaplan-Moss wrote: > The patch looks pretty good to me; it applies cleanly and all the unit tests > pass (you get major props for including 'em on the first pass :).
> However, I do have one quibble: the addition of ``self._django_initializing`` > inside ``Model.__init__`` has a pretty nasty "smell" to it. Is that there to > prevent the setter from being called during initialization? If so, why?
> Also, a few lines of documentation need to be added (to docs/model-api.txt) > before this can be checked in.
It is there to prevent application of the setters upon initialization, as you guess.
Running the setters at initialization causes two major problems:
* First, using the code in my unit test, during the initial construction of the GetSet, "has_both" was set first. This caused the getter to fire and correctly set updated_length_field to 1. Then, as the Django initialization proceded, updated_length_field was set to the default of 0 since no value was passed in, and the computed value was blown away. (Amusingly, the current end result is the same, the updated_length_field is set to the default of 0. But now it's *documented* that setters aren't called :) , rather than having setters run in an indeterminate order on partially-constructed objects.)
* Suppose you have a setter that performs a transform T(x). The first time you set it, you'll get T(x) in the database. The next time you retrieve it, the setter applies the transform again, and you end up with T(T(x)). Or to put it another way, loading an object and immediately saving it is no longer an effective no-op. That's bad. (Putting it lightly.)
I started down a path to try to "solve" these problems, but everything was complicated and made me nervous about performance issues.
I agree that adding that flag is sort of bad, but everything else I came up with was worse. I'm open to suggestions.
A revised patch including documentation has been added; it supercedes the previous one.
On 12/15/06, Jacob Kaplan-Moss <ja...@jacobian.org> wrote:
> Also, a few lines of documentation need to be added (to docs/model-api.txt) > before this can be checked in. If you can remove the ``_django_initializing`` > flag (or at least explain why you need it) and write a few lines of docs, I'll > happily check this in.
Before checking in this new functionality, I'd be very interested in seeing whether it would be possible to allow for "normal" property usage on models, rather than inventing a new way of doing it.
Seems like it would be tricky to figure it out, but that shouldn't stop us from experimenting... Who wants to try?
Adrian
-- Adrian Holovaty holovaty.com | djangoproject.com
Adrian Holovaty wrote: > Before checking in this new functionality, I'd be very interested in > seeing whether it would be possible to allow for "normal" property > usage on models, rather than inventing a new way of doing it.
> Seems like it would be tricky to figure it out, but that shouldn't > stop us from experimenting... Who wants to try?
The normal way is impossible. A single attribute of the class can not be both a property instance and a *Field instance.
The metaclass only gets a crack at the class after the class statement is complete.
The only other alternative is the moral equivalent of:
class X(models.Model): field1 = models.BooleanField(class_property='field')
def setter(self, val): self.field1 = val def getter(self): return self.field1 field = proprerty(getter, setter)
But I don't think that's any cleaner, and may raise some issues with the field metadata (now fields have three names: the attribute of the class that defines them, the real database field, and the class attribute that you actually use to access the value on instances). There's no way to avoid an attribute on the models.*Field that I can see.
The other somewhat nice thing about my approach is that Django manages the secondary attribute on the class behind the scenes, which is one less thing for people to have to worry about. With direct use of the property descriptor, you have to do your own management of where the real data is stored.
(I'm not sure I'm totally happy with "_%s" % fieldname. "_django_%s" might be better.)
I got burned this weekend with authentication users, setting passwords on them directly during construction. (I knew I had to call set_password to change the password but I had gotten the idea I could pass 'password' into the constructor and get it handled correctly.)
If my getters/setters patch goes in, is there any reason in particular not to re-write the password field to use the existing set_password code as the setter? Then that part of the documentation could be simplified.
I could also do an eyeball scan looking for similar setters in the codebase. If we're lucky, we could end up with a net reduction in documentation.
|
