Django

Code

Ticket #6162 (closed: fixed)

Opened 7 months ago

Last modified 7 months ago

ModelForm.__init__() should match argument signature of "standard" forms

Reported by: ubernostrum Assigned to: nobody
Milestone: Component: django.newforms
Version: SVN Keywords: modelform
Cc: Triage Stage: Design decision needed
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

Description

Currently, ModelForm.__init__() differs from the __init__() of a "standard" newforms form in two ways:

  1. It requires an instance of the model to be passed in, and cannot fall back to a default.
  2. It requires this to be the first positional argument, while on "standard" forms the first positional argument is data.

A number of use cases -- particularly generic form-handling code -- would be made much simpler if instead instance was optional (with ModelForm falling back to creating an instance based on _meta.model if it's not supplied), and if it was moved further back in the list of arguments so as to allow code to work with the standard argument signature of BaseForm.__init__() as much as possible.

Attachments

modelform-init.diff (13.2 kB) - added by ubernostrum on 12/08/07 22:13:40.
Patch which "fixes" ModelForm.__init__()
modelform-init-2.diff (17.2 kB) - added by ubernostrum on 12/09/07 01:33:11.
Updated patch

Change History

12/08/07 22:13:40 changed by ubernostrum

  • attachment modelform-init.diff added.

Patch which "fixes" ModelForm.__init__()

12/09/07 01:33:11 changed by ubernostrum

  • attachment modelform-init-2.diff added.

Updated patch

(follow-up: ↓ 2 ) 12/09/07 01:34:38 changed by ubernostrum

  • needs_better_patch changed.
  • stage changed from Unreviewed to Design decision needed.
  • needs_tests changed.
  • needs_docs changed.

New patch ensures that a ModelForm without a model or instance throws an error, and tests and documents the now-possible case of a ModelForm working with any of multiple models at runtime.

Discussion for anyone who's only watching here is taking place on django-dev.

(in reply to: ↑ 1 ) 12/12/07 01:02:41 changed by bear330

I think the below code block which is for form.save method in meta class should be changed.

        # Don't allow more than one Meta model defenition in bases. The fields
        # would be generated correctly, but the save method won't deal with
        # more than one object.
        base_models = []
        for base in bases:
            base_opts = getattr(base, '_meta', None)
            base_model = getattr(base_opts, 'model', None)
            if base_model is not None:
                base_models.append(base_model)
        if len(base_models) > 1:
            raise ImproperlyConfigured("%s's base classes define more than one model." % name)

        # If a model is defined, extract form fields from it and add them to base_fields
        if attrs['_meta'].model is not None:
            # Don't allow a subclass to define a Meta model if a parent class has.
            # Technically the right fields would be generated, but the save 
            # method will not deal with more than one model.
            for base in bases:
                base_opts = getattr(base, '_meta', None)
                base_model = getattr(base_opts, 'model', None)
                if base_model is not None:
                    raise ImproperlyConfigured('%s defines more than one model.' % name)
            model_fields = fields_for_model(opts.model, opts.fields, opts.exclude)
            # fields declared in base classes override fields from the model
            model_fields.update(declared_fields)
            attrs['base_fields'] = model_fields
        else:
            attrs['base_fields'] = declared_fields
        return type.__new__(cls, name, bases, attrs)

(Maybe I should send a patch here, but I am not familiar to patch tool. I will learn it soon...)

Remove those checks, will make mulitiple inherit from ModelForm? possible.

Thanks.

12/12/07 20:48:05 changed by jkocherhans

  • status changed from new to closed.
  • resolution set to fixed.

(In [6915]) Fixed #6162. ModelForm?'s init signature now matches Form's. This is a backwards incompatbile change. Based largely on a patch by ubernostrum.


Add/Change #6162 (ModelForm.__init__() should match argument signature of "standard" forms)




Change Properties
Action