Getters and setters on fields
jerf
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.
[1]:
http://groups-beta.google.com/group/django-users/browse_thread/thread/c784d17b20c2bfcc
Jacob Kaplan-Moss
> 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
Jeremy Bowers
> 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.
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.
Adrian Holovaty
> 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
Jeremy Bowers
> 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.)
Jeremy Bowers
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.