ModelForms

20 views
Skip to first unread message

Joseph Kocherhans

unread,
Nov 7, 2007, 11:26:13 PM11/7/07
to django-d...@googlegroups.com
form_for_model and form_for_instance seem like complicated and clever
ways to accomplish what basically boils down to a form that has a save
method and can accept a model instance in its constructor method.

I propose we (or I rather) actually build it that way before 1.0.

Form declaration:

class MyForm(ModelForm):
model = MyModel

#optional
def save(self, commit=True):
# do custom save stuff here if needed

Usage:

def add_view(request):
if request.POST:
form = MyForm(request.POST)
if form.is_valid()
obj = form.save()
...
else:
form = MyForm()
...

def change_view(request, id):
obj = MyModel.objects.get(pk=id)
if request.POST:
form = MyForm(request.POST, obj=obj)
if form.is_valid()
obj = form.save()
...
else:
form = MyForm(obj=obj)
...

ModelForm would be a declarative class that requires, at minimum, a
model attribute. Other attributes would include:

- formfield_callback (a function that takes a db field and **kwargs
and returns a form field or None)
- fields (a list of field names to include in the form)
- exclude (a list of field names to exclude from the form)

The biggest problem I see is that this would be entirely backwards
incompatible with the way form_for_model and form_for_instance work
now. (especially the latter) It *may* be possible to change form_for_X
into some sort of hackish wrappers, but it wouldn't be pretty.

If we don't actually do this, I'll eventually release it as a 3rd
party package, but having it around would make some things in
newforms-admin a lot more sane to implement.

Joseph

James Bennett

unread,
Nov 8, 2007, 12:23:46 AM11/8/07
to django-d...@googlegroups.com
On Nov 7, 2007 10:26 PM, Joseph Kocherhans <jkoch...@gmail.com> wrote:
> ModelForm would be a declarative class that requires, at minimum, a
> model attribute. Other attributes would include:
>
> - formfield_callback (a function that takes a db field and **kwargs
> and returns a form field or None)
> - fields (a list of field names to include in the form)
> - exclude (a list of field names to exclude from the form)

I've been +1 on something like this for a long time, as you know :)

> The biggest problem I see is that this would be entirely backwards
> incompatible with the way form_for_model and form_for_instance work
> now. (especially the latter) It *may* be possible to change form_for_X
> into some sort of hackish wrappers, but it wouldn't be pretty.

I wouldn't lose any sleep if form_for_* died horrible flaming deaths;
in fact, I'd show up to toast marshmallows. They're not really
powerful enough to do what people want them to do, and so folks end up
spending inordinate amounts of time trying to shoehorn extra
functionality or customizations into them when (at the moment) the
path of least resistance is to just write a form class.

I've also felt for a while that the form_for_* helpers don't really
"fit" into newforms, because the big win in newforms from the
perspective of form authoring is the declarative class style; moving
to that style for generated forms would make it feel much more natural
to me.


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

Malcolm Tredinnick

unread,
Nov 8, 2007, 8:02:50 PM11/8/07
to django-d...@googlegroups.com
On Wed, 2007-11-07 at 22:26 -0600, Joseph Kocherhans wrote:
> form_for_model and form_for_instance seem like complicated and clever
> ways to accomplish what basically boils down to a form that has a save
> method and can accept a model instance in its constructor method.
>
> I propose we (or I rather) actually build it that way before 1.0.
>
> Form declaration:
>
> class MyForm(ModelForm):
> model = MyModel
>
> #optional
> def save(self, commit=True):
> # do custom save stuff here if needed
>
[...]

> ModelForm would be a declarative class that requires, at minimum, a
> model attribute. Other attributes would include:
>
> - formfield_callback (a function that takes a db field and **kwargs
> and returns a form field or None)
> - fields (a list of field names to include in the form)
> - exclude (a list of field names to exclude from the form)

I'm mostly +1 on this idea.

It would remove my real problem with form_for_*, which is that they
can't possibly make everybody happy and so we spend a lot of time
discussing, or, worse, adding extra features to them to the point they
become very difficult to maintain and still not particularly easy to use
for anything other than the trivial case.

So, yeah, without having seen the code, I'm pretty much in favour (don't
screw it up by writing ugly code now!).

>
> The biggest problem I see is that this would be entirely backwards
> incompatible with the way form_for_model and form_for_instance work
> now. (especially the latter) It *may* be possible to change form_for_X
> into some sort of hackish wrappers, but it wouldn't be pretty.

It's probably possible to write compatibility wrappers as a once-off. I
suspect it won't be too much more complex than what we've got now. The
current code already calls type() with arguments, so this is going to be
similar: constructing a class object dynamically.

Regards,
Malcolm

--
If at first you don't succeed, destroy all evidence that you tried.
http://www.pointy-stick.com/blog/

Russell Keith-Magee

unread,
Nov 9, 2007, 6:04:22 AM11/9/07
to django-d...@googlegroups.com
On Nov 8, 2007 1:26 PM, Joseph Kocherhans <jkoch...@gmail.com> wrote:
>
> form_for_model and form_for_instance seem like complicated and clever
> ways to accomplish what basically boils down to a form that has a save
> method and can accept a model instance in its constructor method.
> ModelForm would be a declarative class that requires, at minimum, a
> model attribute. Other attributes would include:
...

> - formfield_callback (a function that takes a db field and **kwargs
> and returns a form field or None)
> - fields (a list of field names to include in the form)
> - exclude (a list of field names to exclude from the form)

Regular readers will note that I've been an advocate for expanding the
capabilities of form_for_model/instance. This isn't because I believe
that the form_for_* syntax has any particular strengths, but because I
have a lot of use for dynamic form generation based on a model. I know
that it is possible to dynamically generate a form without using
form_for_* (by overriding fields lists in __init__, etc), but those
approaches aren't that elegant, IMHO - certainly not as elegant as
just specifying a list of fields that you want to include on a form.

I was under the impression that the form_for_* syntax was BDFL
blessed; as a result, I was concentrating on how to maximize the
utility of the blessed format. However, if an alternate class-based
syntax is an available option, I'm certainly +1 to persuing those
options.

The sort of syntax you are proposing has the potential to be much more
'legible' than cramming everything into arguments on a function
declaration, and has the potential to make the simple tasks (such as
adding a formfield callback, or a custom save method) much more
intuitive and pythonic.

If we're kicking around ideas on this, here's a slightly different
suggestion. First, an example:

class MyForm(Form):
extra_field = forms.CharField()

class Meta:
model = MyModel
fields = ('a','b')
def formfield(f):
return ...

def save():
...

i.e., rather than writing a new ModelForm base class, put the
'form_for_*' stuff into a Meta class (Meta probably isn't the right
name, but it will do for now), which is used by the BaseForm metaclass
as a set of assembly instructions for how to add model/instance based
fields to MyForm.

If you provide a Meta class, any of the field/exclude declarations are
used to add new fields to the model; if there is a formfield() method
on the Meta class, it is used to determine which Field class will be
used.

The fields added from the Meta class augment those provided manually
(useful for the 'form_for_model plus these extra fields' use case,
such as is required for password validation). If you need custom
save/clean methods, you put them right in the form definition where
they are required, which is exactly what you would do if you were
manually defining the form from scratch.

> The biggest problem I see is that this would be entirely backwards
> incompatible with the way form_for_model and form_for_instance work
> now. (especially the latter) It *may* be possible to change form_for_X
> into some sort of hackish wrappers, but it wouldn't be pretty.

If we decide to go down this path, I would prefer to see the
form_for_* APIs deprecated and removed, rather than maintained as
wrappers. If a wrapper is trivial, there's no real harm in keeping it
around, but if its going to take a lot of effort to build and
maintain, we could end up with a bigger bug hole and image problem
than if we just grit our teeth and rip the band-aid off in one swift
action.

This obviously means preparing the path well - lots of prior warning
that the change will happen, maybe a release before the change comes
into effect, plus public commentary on _why_ we're making such a big
change, a mea culpa for getting it wrong in the first place, and good
documentation on how to manage the change. However, my gut reaction is
that one well managed large change will cause less drama than lots of
little bug fixes to an API wrapper.

Yours,
Russ Magee %-)

Jacob Kaplan-Moss

unread,
Nov 9, 2007, 9:11:05 AM11/9/07
to django-d...@googlegroups.com
Hi folks --

Just to make sure everyone's on the same page: Joseph and I have
talked about this in person quite a bit, and I'm a strong +1 (if
that's possible).

I don't feel strongly about the particulars -- Russell's proposal is
interesting, but I haven't thought it through fully -- but I fully
support the general idea. I think this leads to a good place.

Jacob

Marty Alchin

unread,
Nov 9, 2007, 9:57:24 AM11/9/07
to django-d...@googlegroups.com
Fair warning: I haven't been following this very closely, so I'm
probably way out in left field here. Feel free to look strangely at me
after I mention this. Would it be possible to use a factory pattern
for ModelForm?

class Article(models.Model):
title = models.CharField(max_length=255)
body = models.TextField()

class ArticleForm(forms.ModelForm(Article)):
body = myapp.ReStructuredTextField()

That way, there's a sort of custom form that can be easily subclassed
to customize the form. This wouldn't be all that much different from
form_for_model, except that we could make sure - perhaps during
metaclass processing - that the Form.for_model class did in fact get
subclassed. It could simply raise an exception if instantiated without
being subclassed.

This just seems much cleaner than dealing with model attributes or
inner Meta classes. Fields and methods could be overridden like any
other class, with fields left out simply being inherited from the
parent class. The simple case would just look like this:

class ArticleForm(forms.Form.for_model(Article)):
pass

And everything would Just Work, as long as they don't need more
flexibility. The only thing it doesn't handle yet is how to remove
fields from the customized form, but this might be as simple as
assigning the field to None or some new ExcludedField class or
something.

I also wholeheartedly agree with Joseph's use of a keyword argument to
accept an object instance, avoiding the need for a separate
for_instance method.

So, I like the overall idea, and I'm not particular on exactly how it
gets done. I just thought I'd throw my thoughts in on the subject.

-Gul

Smel

unread,
Nov 9, 2007, 10:07:10 AM11/9/07
to Django developers
+1, I agree, since I no longer use form_for_X method for a while

I use something like that:

from forms import SimpleForm
from django.contrib.auth.models import User
import django.newforms as forms
class UserForm(SimpleForm):
model = User
password2 = forms.CharField(label='Confirm password',
widget=forms.PasswordInput())

class Render:
fields = (('first_name', 'last_name'), 'username',
('password','password2'), 'email',
('is_active', 'is_staff', 'is_superuser'))

PS : http://smelaatifi.blogspot.com/2007/08/declarative-forms-django-newforms.html

Marty Alchin

unread,
Nov 9, 2007, 10:31:27 AM11/9/07
to django-d...@googlegroups.com
On Nov 9, 2007 9:57 AM, Marty Alchin <gulo...@gamemusic.org> wrote:
> The only thing it doesn't handle yet is how to remove
> fields from the customized form, but this might be as simple as
> assigning the field to None or some new ExcludedField class or
> something.

I actually like the idea of an ExcludedField. ExcludedFields wouldn't
be output in HTML, and wouldn't be expected in the incoming data, but
would be set in the constructor instead, using keyword arguments. Take
the following example.

class Article(models.Model):
author = models.ForeignKey(User)


title = models.CharField(max_length=255)
body = models.TextField()

class ArticleForm(forms.ModelForm(Article)):
author = forms.ExcludedField()
body = myapp.ReStructuredTextField()

def new_article(request):
if request.method == 'POST':
form = ArticleForm(request.POST, author=request.user)


if form.is_valid():
obj = form.save()
...
else:

form = ArticleForm(author=request.user)

Using this, is_valid would return False if author wasn't set in the
constructor, unless ExcludedField was declared using required=False. I
don't think it'd be necessary to deal with validating the data on an
ExcludedField though, since it would only get set in Python code.
Setting it to the wrong type would be a code error, not an input
error.

-Gul

Marty Alchin

unread,
Nov 9, 2007, 10:42:07 AM11/9/07
to django-d...@googlegroups.com
On Nov 9, 2007 10:07 AM, Smel <elaa...@gmail.com> wrote:
> PS : http://smelaatifi.blogspot.com/2007/08/declarative-forms-django-newforms.html

I'd say I'm -1 on including admin-style field declarations to forms.
I'm of the mind that, if you need that level of design on a custom
form, just write a template to handle it. Forms seem to me to be more
about handling the input than defining the output. Sure, they have
some convenience methods for that, but if we start trying to get those
to do everything, it's form_for_model all over again. Writing
templates is easy enough, I think.

-Gul

EL AATIFI Sidi Mohamed

unread,
Nov 9, 2007, 12:29:19 PM11/9/07
to django-d...@googlegroups.com
I'm agree with you, but this example is just for demonstration purposes,
but concerning the proposition, we really need something to reuse our
model definitions into forms without rewrite(DRY), since that a form
field, in many cases, has the same validation rules in both, back end
and front end.

Joseph Kocherhans

unread,
Nov 9, 2007, 12:35:14 PM11/9/07
to django-d...@googlegroups.com

The reason I like the ModelForm class is that it provides a clear
separation between a form that knows about models and one that
doesn't. I love that the basic newforms Form has no clue what a Django
model is. Using ModelForm makes it possible to write a SQLAlchemyForm
or a SQLObjectForm. I don't see that happening with the Meta syntax.

I like that your proposed syntax makes it very clear that this is just
a form, it's ok to go ahead and define clean methods on it, etc. The
ModelForm syntax would take documentation, but would behave the same
way.

The ModelForm syntax the rough implementation would look like this (I
emphasize rough):

class BaseModelForm(BaseForm):
# leaving a bunch of other kwargs out here for brevity
def __init__(self, data=None, files=None, obj=None, initial=None):
# get_initial_data pulls data out of the obj as a dict
obj_data = get_initial_data(obj)
# possibly do this:
# obj_data.update(initial)
self.obj = obj
super(BaseModelForm, self).__init__(data, files, initial=obj_data)

def save(self, commit=True):
# mostly a copy of save_instance here.

I'd hate to see the obj arg get put into the constructor of a
BaseForm, so Form's metaclass would end up generating a subclass of
BaseForm or BaseModelForm depending on whether or not the Form had an
inner Meta class (or whatever we call it). The implementation of the
ModelForm syntax seems cleaner to me, but there's most likely a better
way to implement your syntax that I haven't thought of.

Also, I'd like to see more attributes added to ModelForm, but I didn't
want to pollute the core proposal with a bunch of other ideas that may
not sit as well. I'd love to see something like this, but I'd like to
consider these after the foundation is laid:

class MyForm(ModelForm):
model = MyModel

extra_fields = {
'some_field': IntegerField()
}
widgets = {
'existing_field': CustomWidget()
}

def clean(self):
# do some custom cleaning
return self.cleaned_data

The whole idea of a formfield callback is powerful, but it really
lacks in readability for the common tasks of specifying custom widgets
and excluding specific fields.

> > The biggest problem I see is that this would be entirely backwards
> > incompatible with the way form_for_model and form_for_instance work
> > now. (especially the latter) It *may* be possible to change form_for_X
> > into some sort of hackish wrappers, but it wouldn't be pretty.
>
> If we decide to go down this path, I would prefer to see the
> form_for_* APIs deprecated and removed, rather than maintained as
> wrappers. If a wrapper is trivial, there's no real harm in keeping it
> around, but if its going to take a lot of effort to build and
> maintain, we could end up with a bigger bug hole and image problem
> than if we just grit our teeth and rip the band-aid off in one swift
> action.

I don't think the maintenance would be particularly difficult, but the
implementation would be ugly (probably not difficult, but ugly), and
it leaves "more that one way to do it". I may change my mind once I
actually *write* said wrappers, so I'd rather postpone this discussion
until we're talking about some actual code.

> This obviously means preparing the path well - lots of prior warning
> that the change will happen, maybe a release before the change comes
> into effect, plus public commentary on _why_ we're making such a big
> change, a mea culpa for getting it wrong in the first place, and good
> documentation on how to manage the change. However, my gut reaction is
> that one well managed large change will cause less drama than lots of
> little bug fixes to an API wrapper.

Agreed.

Joseph

Joseph Kocherhans

unread,
Nov 9, 2007, 12:42:33 PM11/9/07
to django-d...@googlegroups.com

Why not just do this? No need for any special new fields at all.

class Article(models.Model):
author = models.ForeignKey(User)
title = models.CharField(max_length=255)
body = models.TextField()

class ArticleForm(ModelForm):
model = Article
exclude = ['author']

def new_article(request):
article = Article(author=request.user)

if request.method == 'POST':
form = ArticleForm(request.POST, obj=article)


if form.is_valid():
obj = form.save()
...
else:

form = ArticleForm(obj=article)

It's the business of the form to know what the user is supposed to
provide. In this case, author needs to be provided by the programmer,
and if the programmer screwed up and saving the Article raises an
error due to a missing author field, there's nothing the user can do.
Adding extra machinery to handle stuff like that in the form seems
overly complicated to me.

Joseph

Marty Alchin

unread,
Nov 9, 2007, 12:50:54 PM11/9/07
to django-d...@googlegroups.com
On Nov 9, 2007 12:42 PM, Joseph Kocherhans <jkoch...@gmail.com> wrote:
> Why not just do this? No need for any special new fields at all.
>
> class Article(models.Model):
> author = models.ForeignKey(User)
> title = models.CharField(max_length=255)
> body = models.TextField()
>
> class ArticleForm(ModelForm):
> model = Article
> exclude = ['author']
>
> def new_article(request):
> article = Article(author=request.user)
>
> if request.method == 'POST':
> form = ArticleForm(request.POST, obj=article)
> if form.is_valid():
> obj = form.save()
> ...
> else:
> form = ArticleForm(obj=article)
>
> It's the business of the form to know what the user is supposed to
> provide. In this case, author needs to be provided by the programmer,
> and if the programmer screwed up and saving the Article raises an
> error due to a missing author field, there's nothing the user can do.
> Adding extra machinery to handle stuff like that in the form seems
> overly complicated to me.

True. The only potential advantage of my approach over yours is the
ability to display information about the author in the template,
alongside the other forms, but if that's really necessary, the author
can be passed separately in the context. I hadn't considered just
creating a new object preloaded with the necessary information and
treating it as an incomplete instance form. That's quite reasonable.

I'm not sure about the "exclude = ['author']" syntax though. It seems
to me that setting "author" to None would make more sense, rather than
having a reserved name ("exclude") that takes a list of strings. Part
of the reason I proposed some Form.for_model(Article) syntax for the
base class was to eliminate the need for a reserved name ("model" or
"Meta") just for handling this process. Someone putting together a
form for a Lego set should be able to use "model" as a field name
without a problem, just like somebody writing an email form that mails
to a distribution list should be able to use "exclude" as a field for
addresses to exclude from delivery.

-Gul

Joseph Kocherhans

unread,
Nov 9, 2007, 1:02:56 PM11/9/07
to django-d...@googlegroups.com
On 11/9/07, Marty Alchin <gulo...@gamemusic.org> wrote:
>

ModelForms have an obj (or maybe 'instance' or 'original') attribute, so:

{{ form.obj.author }}

:)

> I'm not sure about the "exclude = ['author']" syntax though. It seems
> to me that setting "author" to None would make more sense, rather than
> having a reserved name ("exclude") that takes a list of strings. Part
> of the reason I proposed some Form.for_model(Article) syntax for the
> base class was to eliminate the need for a reserved name ("model" or
> "Meta") just for handling this process. Someone putting together a
> form for a Lego set should be able to use "model" as a field name
> without a problem, just like somebody writing an email form that mails
> to a distribution list should be able to use "exclude" as a field for
> addresses to exclude from delivery.

I see attributes of ModelForm as more like configuration directives.
They wouldn't clash with the Form's namespace at all and are
essentially the same thing as the keyword args that get passed to
form_for_X right now. There's no reason you can't have a field named
'model' or a field named 'exclude' using the ModelForm syntax.

Joseph

Russell Keith-Magee

unread,
Nov 11, 2007, 11:40:11 PM11/11/07
to django-d...@googlegroups.com
On 11/10/07, Joseph Kocherhans <jkoch...@gmail.com> wrote:
>
> On 11/9/07, Russell Keith-Magee <freakb...@gmail.com> wrote:
> >
> I'd hate to see the obj arg get put into the constructor of a
> BaseForm, so Form's metaclass would end up generating a subclass of
> BaseForm or BaseModelForm depending on whether or not the Form had an
> inner Meta class (or whatever we call it). The implementation of the
> ModelForm syntax seems cleaner to me, but there's most likely a better
> way to implement your syntax that I haven't thought of.

I haven't given much thought to the implementation either. If there is
an objection to having the instance in the argument list of for the
BaseForm constructor, modifying the base class certainly sounds like
one option.

However, I can't say I have a problem with allowing an instance to be
provided as initial data. This would essentially allow you to manually
construct a form for an instance, while retaining many of the
'instance bound' benefits of a ModelForm. This also supports the idea
that the Meta block is just a set of assembly instructions. The form
is just a collection of fields that can be populated (however the
fields were constructed), and one source of initial data for those
fields is an instance.

The problem I have with ModelForm is that it doesn't feel like it has
any parallels with the existing class-based formdefinitions. Manual
Form definitions have a very similar flavour to Model definitions -
each class attribute is a part of the model/form, etc. On one level,
ModelForm tries to follow this lead by using a class-based syntax and
allowing save() and clean() methods - but then it has a completely
different approach to defining form fields, etc. In contrast,
form_for_* doesn't looks anything like a manual form definition, so
it's obvious that you're dealing with a different beast.

> The whole idea of a formfield callback is powerful, but it really
> lacks in readability for the common tasks of specifying custom widgets
> and excluding specific fields.

Agreed. I would also add dynamically modifying the list of choices in
a Select to this list of deficiencies.

Russ %-)

Joseph Kocherhans

unread,
Nov 12, 2007, 3:37:23 PM11/12/07
to django-d...@googlegroups.com
On 11/11/07, Russell Keith-Magee <freakb...@gmail.com> wrote:
>
> The problem I have with ModelForm is that it doesn't feel like it has
> any parallels with the existing class-based formdefinitions. Manual
> Form definitions have a very similar flavour to Model definitions -
> each class attribute is a part of the model/form, etc. On one level,
> ModelForm tries to follow this lead by using a class-based syntax and
> allowing save() and clean() methods - but then it has a completely
> different approach to defining form fields, etc. In contrast,
> form_for_* doesn't looks anything like a manual form definition, so
> it's obvious that you're dealing with a different beast.

I see your point and I've been increasingly unhappy with my original
syntax for exactly those reasons, though you put it a lot more
eloquently than I would have. :) I came up with a new hybrid this
morning that I think addresses both of our concerns, and to me feels a
lot better than either of them.

class MyForm(ModelForm):
extra_field = SomeField()

class Options:
model = MyModel
fields = ['list', 'of', 'field', 'names']

def formfield_callback(db_field):
# return a formfield for the given db field

def save(self, commit=True):
# override save if you need to

def clean(self):
# override clean if you need to

This also allows for forms that have all their fields built by hand,
and not bound to any particular model class until instantiation:

class LatLongForm(ModelForm):
latitude = SomeCustomField()
longitude = SomeCustomField()

# Note there is no inner Options class and thus no model specified
# This form will work on any model instance with fields named
# 'latitude' and 'longitude'

def save(self, commit=True):
# override save if you need to

def clean(self):
# override clean if you need to

This way a Form still doesn't need its constructor changed, only the
ModelForm takes and object and has a save method.

To go along with this, I'd like to slightly change the signature of
the ModelForm constructor to this:

def __init__(self, obj, data=None, files=None...):

This would force people to instantiate and pass in an empty object for
add forms, and hopefully encourage a pattern of setting any defaults
on a model instance before you pass it to the form. I'm hoping this
will mean less people need to muck around with form.save(commit=False)
and the form.save_m2m() method.

> > The whole idea of a formfield callback is powerful, but it really
> > lacks in readability for the common tasks of specifying custom widgets
> > and excluding specific fields.
>
> Agreed. I would also add dynamically modifying the list of choices in
> a Select to this list of deficiencies.

I think during the last sprint we decided to add another custom method
type to forms to handle this. I thought there was a different ticket
that dealt with forms, but #3987 [1] is the closest I could find.
Maybe I'm just dreaming things up though.

[1] http://code.djangoproject.com/ticket/3987

Joseph

Marty Alchin

unread,
Nov 12, 2007, 3:52:01 PM11/12/07
to django-d...@googlegroups.com
On Nov 12, 2007 3:37 PM, Joseph Kocherhans <jkoch...@gmail.com> wrote:
> class MyForm(ModelForm):
> extra_field = SomeField()
>
> class Options:
> model = MyModel
> fields = ['list', 'of', 'field', 'names']
>
> def formfield_callback(db_field):
> # return a formfield for the given db field
>
> def save(self, commit=True):
> # override save if you need to
>
> def clean(self):
> # override clean if you need to
>

Call me crazy, but why would formfield_callback even be needed in this
case? Wouldn't it be possible supply customized fields as attributes
of the class, alongside the extra_field assignments? It just seems
counterintuitive to have some fields defined one way, with other
fields defined in another way.

-Gul

Joseph Kocherhans

unread,
Nov 12, 2007, 4:05:24 PM11/12/07
to django-d...@googlegroups.com

The admin (at least in newforms-admin) is still going to need
*something* like formfield_callback, but I haven't figured out the
details yet. In most cases though, you're right. I'll spell out the
details quickly though:

class Article(models.Model):
title = models.CharField(max_length=100)
body = models.TextField

class ArticleForm(models.Model)
body = MyCustomBodyField() # override the default formfield

class Options:
model = Article

The resulting form here would still have 2 fields, title and body.
body would have a custom form field, and title would have the default
one.

Joseph

Marty Alchin

unread,
Nov 12, 2007, 4:16:11 PM11/12/07
to django-d...@googlegroups.com
On Nov 12, 2007 4:05 PM, Joseph Kocherhans <jkoch...@gmail.com> wrote:
> class Article(models.Model):
> title = models.CharField(max_length=100)
> body = models.TextField
>
> class ArticleForm(models.Model)
> body = MyCustomBodyField() # override the default formfield
>
> class Options:
> model = Article
>
> The resulting form here would still have 2 fields, title and body.
> body would have a custom form field, and title would have the default
> one.

Yes, that's exactly what I had in mind. You could have a
formfield_callback internally, I suppose, which just looks at the
ModelForm class to see if the field name is specified. If it is, use
that one, otherwise, use the default associated with the model Field.
But I don't see any reason to require Joe Developer to deal with
formfield_callback anymore.

-Gul

Russell Keith-Magee

unread,
Nov 13, 2007, 7:58:02 PM11/13/07
to django-d...@googlegroups.com
On Nov 13, 2007 5:37 AM, Joseph Kocherhans <jkoch...@gmail.com> wrote:
>
> On 11/11/07, Russell Keith-Magee <freakb...@gmail.com> wrote:
> >
> > The problem I have with ModelForm is that it doesn't feel like it has
> > any parallels with the existing class-based formdefinitions.
...

> I see your point and I've been increasingly unhappy with my original
> syntax for exactly those reasons, though you put it a lot more
> eloquently than I would have. :) I came up with a new hybrid this
> morning that I think addresses both of our concerns, and to me feels a
> lot better than either of them.

This looks a lot better to me. +1.

Russ %-)

Joseph Kocherhans

unread,
Nov 27, 2007, 11:09:01 PM11/27/07
to django-d...@googlegroups.com

Implemented in #6042 [1]. I've left form_for_model and
form_for_instance alone for now, and tried to make use of the existing
code as much as possible. I'd like to see form_for_instance die, and
for form_for_model to be scaled back a lot, essentially turning into a
convenience function for a bare-bones, nothing custom, form for a
model. I'll see if I can pick Malcolm's/Jacob's brains during the
sprint to come up with an acceptable way forward.

Joseph

[1] http://code.djangoproject.com/ticket/6042

Reply all
Reply to author
Forward
0 new messages