Allow changing form field properties after form creation

[Chris Wilson]

Hi all,

I've added this as a ticket but I wanted to make sure that the core
and forms developers have a chance to see and interact with it, so I'm
posting it here too. You can find the ticket at:
<https://code.djangoproject.com/ticket/18064>

Currently, if I want to tweak the properties of some fields in a
ModelForm, I have to replace them in the subclass like this:

{{{
class DocumentForm(ModelForm):
title =
models.Document._meta.get_field('title').formfield(required=False)
programs =
models.Document._meta.get_field('programs').formfield(required=False)
authors =
models.Document._meta.get_field('authors').formfield(required=False)
confidential =
models.Document._meta.get_field('confidential').formfield(widget=AdminYesNoWidget)
uploader =
models.Document._meta.get_field('uploader').formfield(required=False)
file =
models.Document._meta.get_field('file').formfield(widget=AdminFileWidgetWithSize)
}}}

This is very bulky to just change some properties. The problem is that the
field copies some of its properties into weird places like the widget
during initialisation:

{{{
class Field(object):
...
def __init__(...)
...
if help_text is None:
self.help_text = u''
else:
self.help_text = smart_unicode(help_text)
widget = widget or self.widget
if isinstance(widget, type):
widget = widget()

# Trigger the localization machinery if needed.
self.localize = localize
if self.localize:
widget.is_localized = True

# Let the widget know whether it should display as required.
widget.is_required = self.required

# Hook into self.widget_attrs() for any Field-specific HTML
attributes.
extra_attrs = self.widget_attrs(widget)
if extra_attrs:
widget.attrs.update(extra_attrs)

self.widget = widget
}}}

If we refactored this so that all the property initialisation was done in
setters, then we could just write:

{{{
class DocumentForm(ModelForm):
def __init__(...)
self['title'].required = False
self['programs'].help_text = 'Hold down Ctrl to select multiple
options'
self['authors'].required = False
self['confidential'].widget = AdminYesNoWidget
self['uploader'].required = False
self['file'].widget = AdminFileWidgetWithSize
}}}

Which is more concise, much clearer, and does not override things
unnecessarily.

I can write the code, but first I want to:

* see if any core developers object, to avoid wasting effort
* know how to run the Django tests and which suite(s) I should be running
* know where to put the tests.

Cheers, Chris.
--
Aptivate | http://www.aptivate.org | Phone: +44 1223 967 838
Future Business, Cam City FC, Milton Rd, Cambridge, CB4 1UY, UK

Aptivate is a not-for-profit company registered in England and Wales
with company number 04980791.

[Nate Bragg]

They don't "have" to be replaced in a subclass in the way you showed.

Perhaps it isn't perfectly DRY, but whats so bad about naming the field

explicitly?  Anyhow, when it comes specifically to widgets, the Meta

class already has a 'widgets' attribute already that lets you specify that.

I would sooner have "smart" Meta class attributes to perform this

behavior declaratively than to have to do it at the __init__ level:

class Meta:

    title__required = False
    programs__help_text = 'Hold down Ctrl to select multiple options'
    authors__required = False
    confidential__widget = AdminYesNoWidget
    uploader__required = False
    file__widget = AdminFileWidgetWithSize

... and I don't like *that* particularly either.

Cheers,

Nate

--
You received this message because you are subscribed to the Google Groups "Django developers" group.
To post to this group, send email to django-d...@googlegroups.com.
To unsubscribe from this group, send email to django-develop...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.

[Chris Wilson]

Hi Nate,

On Thu, 5 Apr 2012, Nate Bragg wrote:

> They don't "have" to be replaced in a subclass in the way you showed.
> Perhaps it isn't perfectly DRY, but whats so bad about naming the field
> explicitly?

The fact that we're building reusable components and we want people to be
able to subclass our forms and:

* change field and widget properties
* do so depending on run-time configuration (e.g. the database or
settings)
* even if the inherited field type changes
* or the inherited options change
* and get an error if they reference a field that doesn't exist any more

Besides which, it's unnecessary, repetitive and unclear what one is doing.

> Anyhow, when it comes specifically to widgets, the Meta class already
> has a 'widgets' attribute already that lets you specify that.

Thanks for reminding me about that. It's a bit difficult to inherit
through multiple levels of subclasses, and doesn't generate errors if one
refers to a field or property that doesn't exist, but it meets most of my
requirements.

> I would sooner have "smart" Meta class attributes to perform this
> behavior declaratively than to have to do it at the __init__ level:
>
> class Meta:
>     title__required = False
>     programs__help_text = 'Hold down Ctrl to select multiple options'
>     authors__required = False
>     confidential__widget = AdminYesNoWidget
>     uploader__required = False
>     file__widget = AdminFileWidgetWithSize

Yes, that would work too, although it also doesn't generate errors if one
refers to a field or property that doesn't exist,

> ... and I don't like *that* particularly either.

Do you really dislike the idea of making form field properties settable
after creation using @property and behave sensibly? It seems neat, simple,
small, powerful and pythonic to me.

[Brian Neal]

On Thursday, April 5, 2012 6:49:20 AM UTC-5, Chris Wilson wrote:

Hi all,

I've added this as a ticket but I wanted to make sure that the core
and forms developers have a chance to see and interact with it, so I'm
posting it here too. You can find the ticket at:
<https://code.djangoproject.com/ticket/18064>

Currently, if I want to tweak the properties of some fields in a
ModelForm, I have to replace them in the subclass like this:

{{{
class DocumentForm(ModelForm):
     title =
models.Document._meta.get_field('title').formfield(required=False)
     programs =
models.Document._meta.get_field('programs').formfield(required=False)
     authors =
models.Document._meta.get_field('authors').formfield(required=False)
     confidential =
models.Document._meta.get_field('confidential').formfield(widget=AdminYesNoWidget)
     uploader =
models.Document._meta.get_field('uploader').formfield(required=False)
     file =
models.Document._meta.get_field('file').formfield(widget=AdminFileWidgetWithSize)
}}}

You can already replace and tweak the fields in a way similar to what you are proposing. For example, you can create a Form or ModelForm, and in the __init__() you can do:

def __init__(self, *args, **kwargs):
    super(MyForm, self).__init__(*args, **kwargs)
    self.fields['title'] = models.Charfield(max_length=30, required=False)
 

[Chris Wilson]

Hi Brian,

On Thu, 5 Apr 2012, Brian Neal wrote:

> On Thursday, April 5, 2012 6:49:20 AM UTC-5, Chris Wilson wrote:
>
> class DocumentForm(ModelForm):
>      title =
> models.Document._meta.get_field('title').formfield(required=False)
>

> You can already replace and tweak the fields in a way similar to what you are proposing. For example, you can create a Form or ModelForm, and in the __init__() you can do:
>
> def __init__(self, *args, **kwargs):
>     super(MyForm, self).__init__(*args, **kwargs)
>     self.fields['title'] = models.Charfield(max_length=30, required=False)

Thanks, yes, I can replace the entire field but I can't tweak its
properties. I.e. I can do exactly what I was doing before, in __init__,
like this:

class DocumentForm(ModelForm):

def __init__(self, *args, **kwargs):
super(MyForm, self).__init__(*args, **kwargs)
self.fields['title'] =

models.Document._meta.get_field('title').formfield(required=False)

But that's even longer. I'm trying to propose a simple way to tweak
certain properties of a field after it's created, without replacing the
field, in a way that meets my requirements as described in the previous
email and that's pythonic and simple and likely to be accepted by Django.

[Beres Botond]

Hi Chris,

Isn't it this what you are trying to do?

class DocumentForm(ModelForm):
def __init__(self, *args, **kwargs):
super(MyForm, self).__init__(*args, **kwargs)

self.fields['title'].required = False

Which works perfectly fine. You can tweak pretty much any property at
runtime like that, without replacing the field entirely.


Regards,

Boti

[Simon Meers]

On 6 April 2012 07:26, Beres Botond <boto...@gmail.com> wrote:
> Hi Chris,
>
> Isn't it this what you are trying to do?
>
> class DocumentForm(ModelForm):
>        def __init__(self, *args, **kwargs):
>                super(MyForm, self).__init__(*args, **kwargs)
>                self.fields['title'].required = False
>
> Which works perfectly fine. You can tweak pretty much any property at
> runtime like that, without replacing the field entirely.

This discussion is sounding more like something that belongs in
django-users. I agree that there are some improvements that can be
made to make field-tweaking easier and more elegant, as proposed in
[1] which Keryn pointed out in [2]. It would probably be better to
continue this conversation in those tickets.

Simon

[1] https://code.djangoproject.com/ticket/17924
[2] https://code.djangoproject.com/ticket/18064

[Chris Wilson]

Hi Simon,

Now unfortunately both tickets have been closed by a core developer, so I
apparently have no choice but to try to persuade people here if I want to
reverse that.

There is an important use case on my ticket [2] which is not covered by
[1], which is controlling properties depending on run-time information.
For example:

* we have drop-down list controls whose choices depend on database
queries, and which must not be cached in the form class, because we can't
restart the server just in order to show changes in those lists.

* similarly, whether a field is required or not might depend on the value
of another field, or in another related model.

We CAN use __init__ to either replace the fields, or poke values into
fields and their widgets, but it's not clean: it's excessive code and the
excess code is entirely Django internals that are subject to change. For
example, we currently have to know that we need to change the widget's
values as well:

self['title'].required = False

self['title'].widget.is_required = False

whereas if that was handled by a property setter, we'd only have to do
this:

self['title'].required = False

and the amount of code in Field is the same, but much of the code from
__init__ is separated out into individual setters. I hope that you will
agree that this is cleaner, more reusable design, which allows fields to
respond intelligently to changes to their properties after creation.

(If you don't agree, is it because you're in the "everything should be
declarative" camp? I have issues with that position: mainly that sometimes
some things cannot be known until run time; so we should support doing
everything at run time, even if we have some magic sugar for doing it
declaratively as well, when that's possible).