subclassed ModelForms initial values behavior changed from 1.1beta to 1.1

4 views
Skip to first unread message

David Haas

unread,
Aug 1, 2009, 7:28:53 PM8/1/09
to Django users
Hi:

I'm hoping to get some feedback on a little change in behavior I'm
seeing when initializing ModelForms between 1.1-beta & the official
1.1 release.

The quick and dirty explanation is this: I have a ModelForm which is a
subclass of another ModelForm. The parent ModelForm has the fields
attribute defined in its inner Meta class - the child ModelForm does
not. In 1.1, when I pass in an instance of an object to initialize
the form, only the fields appearing in the parent ModelForm's field
attribute list end up with initial values - other fields are blank.
In 1.1-beta, all fields (whether or not they're in the parent
ModelForm's field list) get initial values.

In my case, I've worked around this new behavior by adding some code
in the child ModelForm __init__ method to set the initial values of
the relevant fields. I'm just curious if the change in behavior was a
deliberate decision, and if not, does it merit a bug report?

And yes, I know that when you specify the fields attribute in an inner
Meta class, only those fields get saved to the db. I have a system
with separate data entry & review, and I'm seeing this problem on the
review side. During data review, no matter what gets sent out, the
only thing that I want to save in the database is "accept" or
"reject", which lives on the parent form.

Thanks in advance,

- D

Ramiro Morales

unread,
Aug 1, 2009, 9:56:35 PM8/1/09
to django...@googlegroups.com

I'm not seeing this, I've modified Django model_forms modeltest to
exercise what I interpret from your report and the new test passes
and shows identical behavior with both 1.1beta (r10133) and 1.1:

http://dpaste.de/Yr0f/

Please modify these tests to describe what you are seeing if I've
missed anything.

Regards,

PS: Maybe you are not seein the difference in behavior at model
instance save time as per what's described in the docs in the note
right above this?:

http://docs.djangoproject.com/en/dev/topics/forms/modelforms/#overriding-the-default-field-types

--
Ramiro Morales
http://rmorales.net

PyCon 2009 Argentina - Vie 4 y Sab 5 Setiembre
Buenos Aires, Argentina
http://ar.pycon.org/2009/about/

David Haas

unread,
Aug 2, 2009, 12:58:50 AM8/2/09
to Django users
Ramiro:

I've set up some models / forms / formsets which demonstrate the
change here:

http://dpaste.de/YhrI/

My initial report wasn't correct - the change in behavior is only seen
when initalizing ModelFormSets . . . the ModelForm behavior is
unchanged. With a formset, the value use to get initialized; now it
doesn't. With a form, the value has never gotten initialized.

I think 1.1 & 1.0.3 have the same behavior (no initialization in
formsets); and SVN 10132 (1.1 beta) & 1.0.2 have the same behavior
(initialization in formsets).

On Aug 1, 6:56 pm, Ramiro Morales <cra...@gmail.com> wrote:
> http://docs.djangoproject.com/en/dev/topics/forms/modelforms/#overrid...
>
> --
> Ramiro Moraleshttp://rmorales.net

Russell Keith-Magee

unread,
Aug 2, 2009, 9:24:53 AM8/2/09
to django...@googlegroups.com
On Sun, Aug 2, 2009 at 12:58 PM, David Haas<david....@gmail.com> wrote:
>
> Ramiro:
>
> I've set up some models / forms / formsets which demonstrate the
> change here:
>
> http://dpaste.de/YhrI/
>
> My initial report wasn't correct - the change in behavior is only seen
> when initalizing ModelFormSets . . . the ModelForm behavior is
> unchanged.  With a formset, the value use to get initialized; now it
> doesn't.  With a form, the value has never gotten initialized.
>
> I think 1.1 & 1.0.3 have the same behavior (no initialization in
> formsets); and SVN 10132 (1.1 beta) & 1.0.2 have the same behavior
> (initialization in formsets).

Hi David,

Thanks for this report. I'll have to dig a bit deeper to see exactly
what is going on here. If you want to help out, working out which
changeset between 10132 (beta 1) and 11365 (1.1 final) introduced this
discrepancy would be extremely helpful.

I'd also be interested to hear which behaviour - history and personal
uses notwithstanding - you think is correct. On first inspection, I'm
not completely convinced that the 'new' behaviour is actually
incorrect - or, at least, that there might be a larger bug lurking
here with regard to the interpretation of inherited Meta.field
arguments. However, this is entirely based on first impressions, late
at night, with a mild headache, so I could be completely off base.

Yours
Russ Magee %-)

David Haas

unread,
Aug 2, 2009, 3:46:19 PM8/2/09
to Django users
It looks like the change happened between rev. 10189 & 10190: 10189
displays the value, 10190 doesn't.

Unfortunately, I don't think I can give you a completely unbiased
answer as to what behavior I think
is more correct, because of history and personal use :)

With that out on the table, though, it seems to me that if you are
using a model form, and you're initializing
it with a model instance, and that model instance has a current value
for a field on your form, the form
ought to get initialized with that value, regardless of whether or not
you plan on eventually saving the
field in the database or not. If you're going to display it, it ought
to reflect what's currently in the database.
I think I've read some stuff about eventually making 'read only'
forms, or marking fields on a form as 'read only', which seems like it
could tie into this somehow, eventually, maybe.

If you agree with that, though, then currently both ModelForms &
ModelFormsets initialization is broken, because
neither fills in the values.

- D

On Aug 2, 6:24 am, Russell Keith-Magee <freakboy3...@gmail.com> wrote:

Russell Keith-Magee

unread,
Aug 4, 2009, 10:23:49 AM8/4/09
to django...@googlegroups.com
On Mon, Aug 3, 2009 at 3:46 AM, David Haas<david....@gmail.com> wrote:
>
> It looks like the change happened between rev. 10189 & 10190: 10189
> displays the value, 10190 doesn't.
>
> Unfortunately, I don't think I can give you a completely unbiased
> answer as to what behavior I think
> is more correct, because of history and personal use :)
>
> With that out on the table, though, it seems to me that if you are
> using a model form, and you're initializing
> it with a model instance, and that model instance has a current value
> for a field on your form, the form
> ought to get initialized with that value, regardless of whether or not
> you plan on eventually saving the
> field in the database or not.  If you're going to display it, it ought
> to reflect what's currently in the database.
> I think I've read some stuff about eventually making 'read only'
> forms, or marking fields on a form as 'read only', which seems like it
> could tie into this somehow, eventually, maybe.
>
> If you agree with that, though, then currently both ModelForms &
> ModelFormsets initialization is broken, because
> neither fills in the values.

Thanks for taking the time to do the analysis and explanation. I've
now had a closer look at this - unfortunately, I'm inclined to say the
exact opposite. I think ModelForm is doing the right thing.
My reasoning stems from the fact that we are dealing with a ModelForm,
not just a Form, and the 'Meta' definition is the highest source of
authority.

DataParentForm defines a Meta relationship with the DataParent model,
and explicitly defines that only the 'name' field is to be exposed.

DataChildForm inherits from DataParentForm, and while it overrides the
model relationship, it doesn't override the fields definition - so at
this point, we have a ModelForm on DataChild that only exposes the
'name' field.

Then, you have added a 'value' field to the model. While this does
share the name of a field on the model, this is a coincidence - you
have explicitly stated that you don't want the 'value' field from the
model to be involved on the form. The 'value' FloatField could be
called anything else without error - by virtue of the inherited Meta
definition, it bears no relationship to the underlying model.

If, on DataChildForm, you add a fields definition:

fields = ['name','value']

the explicitly defined FloatField becomes an override of the default
form element rather than a standalone form element, and as a result
the initial data is populated.

The behaviour for ModelFormSet is then consistent with that seen in
the ModelForm.

So - to my mind, the v1.1 behaviour of ModelForm is doing the right
thing. I agree that this is a nasty edge case though, and I'd be
interested to hear if anyone has any differing opinions or reasoning.

Yours,
Russ Magee %-)

Malcolm Tredinnick

unread,
Aug 5, 2009, 2:28:54 AM8/5/09
to django...@googlegroups.com

For what it's worth, I agree with Russell's analysis here. If you're
inheriting from a parent class, you are saying your subclass "is-a"
instance of the parent and you can't put back things from the model the
parent has already excluded. That isn't "is-a", it's
"same-but-different".

Regards,
Malcolm


David Haas

unread,
Aug 7, 2009, 10:17:06 PM8/7/09
to Django users
Fair enough. I've got some style questions about the best way to do
what I want to do, but they're probably not so relevant here.
Something that might be relevant is this . . . is it worth submitting
a documentation patch for this issue? I was thinking something on
this page:

http://docs.djangoproject.com/en/dev/topics/forms/modelforms/#using-a-subset-of-fields-on-the-form

... in the note box along the lines of "If you specify fields or
exclude when creating a form with ModelForm, then the fields that are
not in the resulting form will not be <new stuff>initialized by an
model instance or </newstuff> set by the form's save() method.

Thanks,

- D

On Aug 4, 11:28 pm, Malcolm Tredinnick <malc...@pointy-stick.com>
wrote:
> On Tue, 2009-08-04 at 22:23 +0800, Russell Keith-Magee wrote:

Russell Keith-Magee

unread,
Aug 7, 2009, 11:12:14 PM8/7/09
to django...@googlegroups.com
On Sat, Aug 8, 2009 at 10:17 AM, David Haas<david....@gmail.com> wrote:
>
> Fair enough.  I've got some style questions about the best way to do
> what I want to do, but they're probably not so relevant here.
> Something that might be relevant is this . . . is it worth submitting
> a documentation patch for this issue?  I was thinking something on
> this page:
>
> http://docs.djangoproject.com/en/dev/topics/forms/modelforms/#using-a-subset-of-fields-on-the-form
>
> ... in the note box along the lines of "If you specify fields or
> exclude when creating a form with ModelForm, then the fields that are
> not in the resulting form will not be <new stuff>initialized by an
> model instance or </newstuff> set by the form's save() method.

Absolutely. We aren't striving for ambiguity, so any suggestions on
how we can clarify the docs in this regard are most welcome. Please
open a ticket for the general suggestion (perhaps with a summary of
the problem so we know what it is we're trying to explain); if you
have some draft text, drop it in a patch or a comment.

Yours,
Russ Magee %-)

Reply all
Reply to author
Forward
0 new messages