About ModelForms implicitness and ways to wrap ModelForms

5 views
Skip to first unread message

msaelices

unread,
Apr 2, 2008, 8:06:51 PM4/2/08
to Django developers
This mail maybe can be splitted in two, but I write only one because
both are related.

Ok, ModelForms is a very very wonderful thing, but I want to talk
about (maybe) excessive implicitness.

Look at this form declaration:

class AuthorForm(forms.Form):
name = myforms.MyCharField(max_length=100)
email = myforms.MyEmailField()

What is the first you find out? Ok, it's a forms with two fields.

Now look at this:

class AuthorForm(forms.ModelForm):
name = myforms.MyCharField(max_length=100)
email = myforms.MyEmailField()

class Meta:
model = Author

Now, what do you see? You first think again in a form with two fields,
however maybe is a form with five fields. What is happening? You
explicity define two fields, and the real action is a _redefinition_
of two fields.

Django philosophy talk about "Explicit better than implicit", but in
this point ModelForms is not very intuitive.

I don't know if exists a better solution for this. A colleage mine
develops (before implementation of ModelForms) a form wrapper solution
that now we published [1], with ideas from Zope3 formlib, with a
FormField class for collecting fields of a model (another way to
implement something like ModelForms).

After ModelForms was implemented, we've migrated this wrapper to
ModelForms [2], because now is the best practice, but I missed a mixed
solution, using something like FormFields class.

On the other hand (now begin second part of mail), I want to talk
about cmsutils/forms.py file. I think GenericAddForm [3] and
GenericEditForm [4] implementation is a good way to simplify 90% of
your Django views. Typical form management view would be:

def author_edit(request, author_slug):
author = get_object_or_404(Author, slug=author_slug)
if request.method == 'POST':
form = AuthorEditForm(request.POST, instance=author)
if form.is_valid():
new_author = form.save()
return HttpResponseRedirect(new_author.get_absolute_url())
else:
form = AuthorEditForm(instance=author)
return render_to_response('authors/edit.html', {'form': form} )

But if you define AuthorEditForm extending GenericEditForm like this,
views are simpler:

class AuthorEditForm(GenericEditForm):
template = 'authors/edit.html'
class Meta:
... # the same as usual

def author_edit(request, author_slug):
author = get_object_or_404(Author, slug=author_slug)
form = AuthorEditForm(request, author)
return form.run()

form.run() call if get, render template, if post, validate and
redirect or render (depends on validation errors). Ok, maybe now _this
solution is less explicit_, but I think less code advantage is
important (70% less code in our projects views). Besides, you always
can use raw ModelForms :-)

¿Do you think something like Generic*Form can be needed in Django
core?

¿Any opinion, criticism or suggestion for both parts of email?

Regards,
Lin

[1] https://tracpub.yaco.es/cmsutils/browser/trunk/forms.py?rev=1#L216
[2] https://tracpub.yaco.es/cmsutils/browser/trunk/forms.py
[3] https://tracpub.yaco.es/cmsutils/browser/trunk/forms.py#L54
[4] https://tracpub.yaco.es/cmsutils/browser/trunk/forms.py#L85

James Bennett

unread,
Apr 2, 2008, 8:55:00 PM4/2/08
to django-d...@googlegroups.com
On Wed, Apr 2, 2008 at 7:06 PM, msaelices <msae...@gmail.com> wrote:
> Now, what do you see? You first think again in a form with two fields,
> however maybe is a form with five fields. What is happening? You
> explicity define two fields, and the real action is a _redefinition_
> of two fields.
>
> Django philosophy talk about "Explicit better than implicit", but in
> this point ModelForms is not very intuitive.

Obligatory counterpoint: it's not "intuitive", but that's an awfully
loaded word, since no two programmers ever born will agree on what is
and isn't "intuitive". And I'm not sure "intuitive" is even a good
thing to aim for, because...

A better counterpoint would be to consider this class:

class MyForm(forms.Form):
name = forms.CharField(max_length=25)
favorite_color = forms.CharField(max_length=10)

A simple form with two fields, right?

OK, how about this:

class MyOtherForm(MyForm):
favorite_food = forms.CharField(max_length=20)

By the same argument, this is "unintuitive" because you have to notice
that it's subclassing another form and adding a field, so that it has
three fields instead of just the one that's explicitly defined. And,
in fact, this sort of thing is not uncommon at all in Python; any time
you see something being subclassed, it's a sign you should find out
what the parent class does, because the subclass definition may not
tell the whole story.

In the case of a ModelForm, it's even a bit easier because you know --
from the inner 'Meta' class -- what model it's going to draw its base
field definitions from.

So I'm not convinced that there's any problem here that needs fixing;
you can't just skim over a piece of code without paying attention to
what it's doing (rather than being "intuitive", I like to call that
"dangerous"), so you see that it's subclassing ModelForm and either go
look up what that means (if you don't know what the ModelForm class
does) or look up the model it draws its fields from (if you do), and
there are no surprises.


--
"Bureaucrat Conrad, you are technically correct -- the best kind of correct."

davi...@gmail.com

unread,
Apr 3, 2008, 6:28:31 PM4/3/08
to Django developers
What about replacing the idea of a ModelForm with a ModelField? Then
for shorter
forms you could list all the fields explicitly, without having to
recode all the
default field definitions.

e.g.:

class MyForm(forms.Form):
foo = forms.ModelField(SomeModel, 'foo', widget=forms.Textarea)
bar = forms.ModelField(AnotherModel, 'bar')
baz = forms.CharField(max_length=100)

class Meta:
fields = (
(YetAnotherModel, '*') # field names will be
'yet_another_model_field1', etc.
)

exclude = (
(YetAnotherModel, 'internal_field')
)


# init_field_<field> is called for each field when the form is
instantiated
def init_field_yet_another_model_field1(self, field):
field.widget = forms.Textarea
return field

~David

On Apr 2, 5:55 pm, "James Bennett" <ubernost...@gmail.com> wrote:

msaelices

unread,
Apr 3, 2008, 6:39:46 PM4/3/08
to Django developers
On 4 abr, 00:28, "da...@silvermoon.org" <david...@gmail.com> wrote:
> What about replacing the idea of a ModelForm with a ModelField? Then
> for shorter
> forms you could list all the fields explicitly, without having to
> recode all the
> default field definitions.

I think should be hard to implement. In example, ¿How do you
implement form.save() ?

Malcolm Tredinnick

unread,
Apr 4, 2008, 9:16:58 PM4/4/08
to django-d...@googlegroups.com

On Wed, 2008-04-02 at 17:06 -0700, msaelices wrote:
> This mail maybe can be splitted in two, but I write only one because
> both are related.
>
> Ok, ModelForms is a very very wonderful thing, but I want to talk
> about (maybe) excessive implicitness.
>
> Look at this form declaration:
>
> class AuthorForm(forms.Form):
> name = myforms.MyCharField(max_length=100)
> email = myforms.MyEmailField()
>
> What is the first you find out? Ok, it's a forms with two fields.
>
> Now look at this:
>
> class AuthorForm(forms.ModelForm):
> name = myforms.MyCharField(max_length=100)
> email = myforms.MyEmailField()
>
> class Meta:
> model = Author
>
> Now, what do you see? You first think again in a form with two fields,

Not really. I first notice it's a ModelForm subclass because I start
reading from the first line, not the second one. I remember that a
ModelForm class means the fields are initially derived from the model.
The same as "class Foo(Bar):..." tells me that there's behaviour coming
from the Bar class and the Foo class doesn't tell me everything.

> however maybe is a form with five fields. What is happening? You
> explicity define two fields, and the real action is a _redefinition_
> of two fields.

Yes, just as with every other case of subclassing.

>
> Django philosophy talk about "Explicit better than implicit", but in
> this point ModelForms is not very intuitive.

You're really arguing against Python (and most other languages')
subclassing behaviour. I don't find that particularly supportable.

IT sounds like you want a big sign that says "this isn't a normal Form".
We have that sign... it's spelt "ModelForm". A different type of sign
would be somewhat repetitive and non-Python at this point.

If you have an alternative proposal, pitch it, but your arguments so far
are really saying "I don't like Python's subclassing", so it's a bit
hard to go further on generalisations.

Regards,
Malcolm

--
Tolkien is hobbit-forming.
http://www.pointy-stick.com/blog/

Reply all
Reply to author
Forward
0 new messages