On Dec 8, 2007 11:29 PM, Joseph Kocherhans <[EMAIL PROTECTED]> wrote:
> So, I'm going to pretend you didn't say anything about rearranging
> positional arguments because a) it doesn't work, and b) your patch
> doesn't do that.
It does, it just makes 'instance' the last positional argument.
Whether anyone will ever actually instantiate a ModelForm with all the
arguments in the right order is another matter ;)
> So, my response will assume that you are suggesting
> that instance become a keyword argument rather than the first
> positional argument, and that if 'instance' isn't given, the ModelForm
> will create one.
That's the general effect of the patch, yes, but the positional aspect
is important: getting 'instance' out of a position that's commonly
used for other purposes on non-ModelForm form classes is going to be
necessary if we want it to be easy to write generic form-handling
code.
> The ModelForm doesn't *always* know what the model is though. You can
> build a ModelForm *without* using an inner Meta class. The use case
> may be a bit of a stretch, but here it is: Imagine a few models that
> have latitude and longitude fields. You could build a ModelForm for
> lat/long that would work with any one of these objects since the
> ModelForm isn't bound to a particular model until it's instantiated.
> The only requirement is that the fields names are the same. Yes, I'm
> aware this isn't an argument to *always* require an empty model, you
> could get the same effect and only require an empty instance for
> ModelForms that haven't already been tied to a particular model.
That's exactly what I'd imagined. Your example can be solved right now
by the following:
class LatLongForm(ModelForm):
class Meta:
fields = ('latitude', 'longitude')
model_class = figure_out_latlong_model()
form = LatLongForm(instance=model_class())
> We can't always do so for the reasons stated above.
As ModelForm is currently constituted, the code which uses it
*already* has to figure out what model the form will use, because it
has to pass an instance of the model into ModelForm.__init__().
Changing ModelForm as I've proposed would simply mean that code which
uses it for the sort of generic case you've outlined still has to
figure out what model the form will use and pass an instance of the
model into ModelForm.__init__(). There's no net change for this use
case, aside from probably wanting to pass 'instance' as a keyword
argument.
> Turning instance into a keyword argument optimizes the syntax for the
> add case over the change case (right now it's vice versa). Meh. Code
> churn. But, it does have the benefit of making the constructor
> signature of Form and ModelForm the same. I can see people tripping up
> on the differences... a lot in fact. My original intent with making it
> positional was to encourage people to assign defaults to a new model
> object before passing it to the form rather than doing the
> form.save(commit=False) dance. I knew it was heavy handed, but it
> feels even more so now.
I agree that it optimizes somewhat for the "add" case, but for
consistency with how we've handled that case in the past, and the
larger goal of consistency with the more common newforms case, I think
it's worth doing.
> I'm fine with changing the method signature, but I really think you
> patch should handle the use case I mentioned before though. It
> shouldn't be that difficult.
See the reasoning above ;)
--
"Bureaucrat Conrad, you are technically correct -- the best kind of correct."
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Django developers" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en
-~----------~----~----~----~------~----~------~--~---
