Call for comment: Forms/manipulator replacement, take 1
Adrian Holovaty
I've attached the first draft implementation of the manipulator
replacement framework -- it's just a single Python module. This
handles validation, form display/redisplay and rendering of HTML form
elements. It's mostly similar to what came out of the discussion here:
It's not 100% finished. Not all the common validators are implemented,
nor are all of the HTML form widgets. But the basic design is there.
I'm trying to "release" early and often to get plenty of critiques
before things get stable.
I don't have the time to document the whole thing, but...You guys are
Django developers, you're smart! :) But really, there are 134 unit
tests at the top of the module, and they should give you a pretty good
idea of what's going on.
Here's a basic example of form use:
class ContactForm(Form):
your_name = CharField(max_length=100)
your_email = EmailField()
message = CharField(max_length=2000)
def my_view(request):
form = ContactForm(request.POST)
if form.is_valid():
send_email(form.to_python())
return HttpResponseRedirect('/success/')
return render_to_response('foo', {'form': form})
And in the template:
{{ form.your_name.errors }}
<p>Your name: {{ form.your_name }}</p>
{{ form.your_email.errors }}
<p>Your e-mail: {{ form.your_email }}</p>
{{ form.message.errors }}
<p>Message: {{ form.message }}</p>
Notes:
* There's no distinction between a Form and a BoundForm, as previously
discussed. A form is always passed a dictionary of form data, and that
dictionary can be empty.
* All output is Unicode. I did this because eventually Django's
internal strings will all be Unicode, and we might as well do that
from the start with this.
* Each Field has a 'widget' attribute, which is set to the Widget
class to use when rendering it. This is nice and decoupled but, more
importantly, practical.
* Two of the unit tests are failing at the moment, because of a
subtlety: When a BoundField is rendered, the BoundField.__str__()
method uses the *original* data, not the data that has possibly been
altered by to_python(). The unit tests are written in a way that
expects the field to render the post-to_python() data. Specifically,
the input for the 'birthday' field is the string '1940-10-9', and
to_python() converts that to datetime.date(1940, 10, 9), which
resolves to the string '1940-10-09' (note the zero) when run through
str().
I'm not sure what the ideal behavior is here. The (at least) three
possibilities are:
- Always render the original input data, even if the field was
valid and got converted to something slightly different by
to_python().
- Check whether the form has any errors (in all fields). If it
doesn't, then render the to_python() data. Otherwise, render the
original input data.
- Check whether the form has any errors (in just the particular
field). If it doesn't, then render the to_python() data. Otherwise,
render the original input data.
* There's a to-do list at the top. As I said, this is just a work in
progress at this point. I welcome any thoughts, comments and patches!
Adrian
--
Adrian Holovaty
holovaty.com | djangoproject.com
orestis
a) I think that one should be able to get both empty and filled-in Form
instances by using models. This way, the simple case will be made dead
easy, DRY, and the more complex case will be easy enough.
b) There is need for a technique for rendering the complete form with a
simple for tag. For example, how can I get the human readable name of a
field ? Since in most cases it's defined in the model, it makes sense
to use that transparently in a Form. (see (a)). Then one can either do:
{% render_form form "/app/action/" %}{% endform %}
or
<form action="/app/action/">
{% for field in form %} {% render_field field %} {% endfor %}
</form>
or
<form action="/app/action/">
{% for field in form %}
<p> <label for="{{ field.name }}">{{ field.human_name }}</label> : {{
field }} {{ field.errors }} </p>
{% endfor %}
</form>
One can even take advantage of the advantage of the admin metadata
defined in each model...
Gábor Farkas
> Hello all,
>
> I've attached the first draft implementation of the manipulator
>
> * All output is Unicode. I did this because eventually Django's
> internal strings will all be Unicode, and we might as well do that
> from the start with this.
>
> # Default encoding for input byte strings.
> DEFAULT_ENCODING = 'utf-8' # TODO: First look at django.conf.settings, then fall back to this.
>
> def smart_unicode(s):
> if not isinstance(s, unicode):
> s = unicode(s, DEFAULT_ENCODING)
> return s
>
hi,
regarding the fallback-logic...
what is the goal of the unicode conversion?
should it be a never-failing operation?
because if yes, then we need to fall-back to a never-failing charset,
like iso-8859-1.
means we would have a logic like:
- try django.conf.settings
- if fails, try UTF-8
- if fails, try ISO-8859-1
but, if we allow the situation where the form-processing can fail with
an unicode-error, then imho we should not fall-back at all. just use the
setting in django.conf.settings.
or, what about the following approach:
- add an optional "encoding" parameter to the Form-constructor, which
would default to django.conf.settings.
- and don't have any unicode-fallback
what do you think?
gabor
Russell Keith-Magee
> It's not 100% finished. Not all the common validators are implemented,
> nor are all of the HTML form widgets. But the basic design is there.
> I'm trying to "release" early and often to get plenty of critiques
> before things get stable.
Looks pretty good so far. Some comments below.
> * Each Field has a 'widget' attribute, which is set to the Widget
> class to use when rendering it. This is nice and decoupled but, more
> importantly, practical.
+1 on this. I like the separation of widget from field; it allows easy
wrapping of the basic widgets, but also provides a mechanism by which
different widgets can share to_python implementations (e.g., different
color selection widgets)
However, I'm not so keen on the combination of Field.widget with the
hardcoded as_text, as_textarea approach suggested by the commented
sections of BoundField. I would suggest that these methods are
describing which widget should be used, and as such, should be dynamic
based on the widgets available on a Field:
- Field should hold a dictionary of the widgets that can be used for
display, and the name of the default widget.
- BoundField should implement __getitem__, with the value providing
the key into the widget dictionary.
- __str__ on BoundField returns the default widget from the Field
widget dictionary
This means each Field can define the widget types that are useful and
expose them through the template. The constructor for a Field might
also be extended to allow users to easily add their own widgets to the
available list.
I would also suggest that the coupling between to_python and the POST
dictionary be loosened. Specifically:
- Add an extract method be provided on the Widget to describe how to
pull POST data for the Widget out of the POST dictionary.
def extract(self, name, data):
return { 'value': data.get(name,None) }
- Modify the call to field.to_python to use extract():
value = field.to_python(**widget.extract(name, post_data)])
Why? So that you can implement widgets that use multiple input fields
to produce a single output value. For example:
- specifying color as three separate R,G,B text fields/sliders, but
stored as "#RRGGBB" in the database
- a latitude/longitude entered as degrees, minutes and seconds but
stored as a float in radians in the database.
Using the color example, extract would be something like:
def extract(self, name, data):
return {
'red': data.get(name + '_red', None),
'green': data.get(name + '_green', None),
'blue': data.get(name + '_blue', None)
}
and to_python becomes:
def to_python(self, red, green, blue):
return (...calculated value...)
However, this means that to_python must be smart enough to use the
provided kwargs to identify what type of data has been provided. If
two different widgets on a field extract different data, to_python has
to be extra smart. To overcome this, let the widget define the
to_python implementation that is compatible with the widget:
class RGBSliderWidget(Widget):
to_python = 'to_python_from_triple'
def extract() ...
def render() ...
class TextWidget(Widget):
to_python = 'to_python'
def extract() ...
def render() ...
class ColorField(Field):
widgets = {
'rgbslider' : RGBSliderWidget,
'text': TextWidget
}
default_widget = 'text'
def to_python(self, value):
...
def to_python_from_triple(self, red, green, blue)
...
and use getattr to dynamically load the to_python implementation based
on the widget requested.
> I'm not sure what the ideal behavior is here. The (at least) three
> possibilities are:
>
> - Always render the original input data, even if the field was
> valid and got converted to something slightly different by
> to_python().
+1 to this option for me. The BoundField is bound to raw data. It
should display that raw data. Especially in the case of __str__, since
it is providing the default widget rendering for templates.
The other two options smell like magic. You will never know whether
you have converted data or not; predictability goes right out the
window.
> * There's a to-do list at the top. As I said, this is just a work in
> progress at this point. I welcome any thoughts, comments and patches!
One management issue that concerns me: Is there a plan for getting
this into trunk without messing with installations that are currently
using trunk? This is a biggish API change if it overlaps the existing
django.forms. Were you planning on tagging a 0.96 (or a 0.95.1)
release to capture bugfixes in the post-0.95 trunk?
Yours,
Russ Magee %-)
Istvan Albert
> <p>Message: {{ form.message }}</p>
A very common use case is to format/highlight the error message or
input area upon error. I can already see myself extending the Form
class to include something of the sorts
{{ form.message.errors }}
<p class="{{form.css}}">Message: {{ form.message }}</p>
where the css would be selected based on the state or valid/invalid.
i.
Michael Radziej
> class Widget(object):
> def __init__(self, name, value, label=None, attrs=None):
> if value in EMPTY_VALUES: value = ''
> self.name, self.value, self.label = name, value, label
> self.attrs = attrs or {}
>
> def render(self):
> raise NotImplementedError
I'd rather remove value from __init__() and use render(self, data).
It's just a feeling, but it looks more appealing to me not to
put the data into the Widget object since I consider 'render' a function
of the value. Perhaps it would allow to instantiate the Widget earlier
and independently from actual rendering.
> class BoundField(object):
> "A Field plus data"
> def __init__(self, form, field, name):
> self._form = form
> self._field = field
> self._name = name
>
> def __str__(self):
> "Renders this field as an HTML widget."
> # Use the 'widget' attribute on the field to determine which type
> # of HTML widget to use.
> widget = self._field.widget(self._name, self._form.data.get(self._name, None))
> return widget.render()
so here the last 2 lines go
widget = self._field.widget(self._name)
return widget.render(self._form.data.get(self._name, None))
Perhaps I can make my point more clear when I have had more time to
think about it.
Michael
--
noris network AG - Deutschherrnstrasse 15-19 - D-90429 NÃŒrnberg -
Tel +49 911 9352-0 - Fax +49 911 9352-100
http://www.noris.de - The IT-Outsourcing Company
Adrian Holovaty
> I'd rather remove value from __init__() and use render(self, data).
> It's just a feeling, but it looks more appealing to me not to
> put the data into the Widget object since I consider 'render' a function
> of the value. Perhaps it would allow to instantiate the Widget earlier
> and independently from actual rendering.
I hacked on the forms thing a bit more on a 3-hour plane ride today
and came to the same conclusion. I've attached the newest version of
this file (gee, progressing like this is going to get unwieldy unless
I check this into SVN), which has changed Widget.render() to take the
name and value. I've also made some other changes, mostly in
additional unit tests that show off some of the cool features.
Adrian Holovaty
> However, I'm not so keen on the combination of Field.widget with the
> hardcoded as_text, as_textarea approach suggested by the commented
> sections of BoundField. I would suggest that these methods are
> describing which widget should be used, and as such, should be dynamic
> based on the widgets available on a Field:
> This means each Field can define the widget types that are useful and
> expose them through the template. The constructor for a Field might
> also be extended to allow users to easily add their own widgets to the
> available list.
Check out my latest version (which I attached to another message to
this thread). It lets you pass a 'widget' parameter to any Field to
designate the default widget. It can be either a Widget class or
instance. Does that do the trick for ya?
> I would also suggest that the coupling between to_python and the POST
> dictionary be loosened. Specifically:
>
> - Add an extract method be provided on the Widget to describe how to
> pull POST data for the Widget out of the POST dictionary.
> [...]
> - Modify the call to field.to_python to use extract():
> [...]
> Why? So that you can implement widgets that use multiple input fields
> to produce a single output value. For example:
> - specifying color as three separate R,G,B text fields/sliders, but
> stored as "#RRGGBB" in the database
> - a latitude/longitude entered as degrees, minutes and seconds but
> stored as a float in radians in the database.
> [...]
> However, this means that to_python must be smart enough to use the
> provided kwargs to identify what type of data has been provided. If
> two different widgets on a field extract different data, to_python has
> to be extra smart. To overcome this, let the widget define the
> to_python implementation that is compatible with the widget:
>
> class RGBSliderWidget(Widget):
> to_python = 'to_python_from_triple'
> def extract() ...
> def render() ...
This is quite complicated, and I can smell the YAGNI from about a mile
away, so I'm -1 on this proposal. Let's add it only if there's a
strong desire/need for it in the future; it wouldn't be a
backwards-incompatible change.
> > - Always render the original input data, even if the field was
> > valid and got converted to something slightly different by
> > to_python().
>
> +1 to this option for me. The BoundField is bound to raw data. It
> should display that raw data. Especially in the case of __str__, since
> it is providing the default widget rendering for templates.
>
> The other two options smell like magic. You will never know whether
> you have converted data or not; predictability goes right out the
> window.
Good point about the magic. This option sounds like the right solution.
> One management issue that concerns me: Is there a plan for getting
> this into trunk without messing with installations that are currently
> using trunk? This is a biggish API change if it overlaps the existing
> django.forms. Were you planning on tagging a 0.96 (or a 0.95.1)
> release to capture bugfixes in the post-0.95 trunk?
Two options come to mind --
1. We override django.forms with this new module and leave the old one
in at django.oldforms, so that immediate breakage can be fixed easily
by changing import statements.
2. We use a new namespace for this new framework, leaving the old one
in for temporary backwards compatibility. Problem is, django.forms is
the name that makes the most sense for this sort of thing, and it's
not worth having an inferior namespace for the newer and better stuff
only for reasons of backward compatibility.
So I guess my preference is for option #1. As for tagging post-0.95
bugfixes, sure, we can do that -- doesn't matter much either way IMHO.
Russell Keith-Magee
>
> On 10/27/06, Russell Keith-Magee <freakb...@gmail.com> wrote:
> Check out my latest version (which I attached to another message to
> this thread). It lets you pass a 'widget' parameter to any Field to
> designate the default widget. It can be either a Widget class or
> instance. Does that do the trick for ya?
I think so. I think I was on the wrong track before - I was thinking
that Widgets would need to be registered with Fields for which they
are appropriate. I see now that you are aiming to allowing _any_
widget to be displayed with _any_ field. +1 to this; sorry for
waffling off in the wrong direction.
The static nature of the as_ calls still troubles me a little.
as_widget can't be called from template, so the only way to access a
custom widget is to make it the default. If we are trying to allow any
field to use any widget, this seems a little artificial.
One of the ideas from the original thread that I didn't really
consider fully until now was to use the template filters for this
purpose. {{ form.name }} renders using the default widget, {{
form.name|as_text }} calls BoundField.as_widget(TextWidget()). A few
short helper functions to register widgets, and you remove the need to
'bless' the built in widgets with as_ methods in the BoundField
implementation.
> This is quite complicated, and I can smell the YAGNI from about a mile
> away, so I'm -1 on this proposal. Let's add it only if there's a
> strong desire/need for it in the future; it wouldn't be a
> backwards-incompatible change.
I'll grant that the full solution I suggested is complex, and probably
YAGNI; if you really have two widgets with different types of input,
defining a different field isn't out of order. I retract my request
for the to_python refactor.
However, the original use case is very real for me - lat/long, color
selection (via multiple widget types), and duration (in days, hours,
and minutes) are all widgets that I have needed in very recent memory.
The 'extract' method provides a fairly simple way to accommodate
to_python implementations that require multiple POST values to compose
a single database value.
If the complexity of the default case is the issue, we could shortcut
the default case by checking for the existence of an extract member
and just using data.get(name,None) if it doesn't exist.
Alternatively, I'll take any suggestions on how to represent multiple
input values->one database value using the code as it stands. The only
option I can see is to do it in javascript; put a hidden input on the
form, with three visible inputs, and write javascript so that whenever
the inputs are modified, the hidden value is modified - but I _really_
don't like that solution.
> 1. We override django.forms with this new module and leave the old one
> in at django.oldforms, so that immediate breakage can be fixed easily
> by changing import statements.
>
> So I guess my preference is for option #1. As for tagging post-0.95
> bugfixes, sure, we can do that -- doesn't matter much either way IMHO.
Works for me. Acutally, we could kill two birds with one stone; check
your prototype in as django.newforms, and when we go live, tag 0.96,
move django.forms to django.oldforms, and django.newforms to
django.forms.
Yours,
Russ Magee %-)
Nesta Campbell
Given the scope of things this may actually be irrelevant at the
moment but I've created a patch that adds an exact_length option to
CharField. I am aware the same result can be had by setting max_length
and min_length to the same value. Patch attached.
Cheers.
--
Nesta Campbell
Adrian Holovaty
> Given the scope of things this may actually be irrelevant at the
> moment but I've created a patch that adds an exact_length option to
> CharField. I am aware the same result can be had by setting max_length
> and min_length to the same value. Patch attached.
Hi Nesta,
In my opinion, that's a bit feature-creepy for inclusion in the main
distro, but the great thing about this new framework is that you can
easily create your own Field subclass that does indeed have an
exact_length option.
Adrian Holovaty
> your prototype in as django.newforms, and when we go live, tag 0.96,
> move django.forms to django.oldforms, and django.newforms to
> django.forms.
I've checked in the prototype as django.newforms, split it into
several modules and moved the unit tests to live with the other Django
unit tests.
Brantley Harris
> I've attached the first draft implementation of the manipulator ...
Yeay! Progress. I'm +1 for progress.
I like the direction towards simplification and separation, but I'm
not sure if the line is drawn at the correct place. Essentially with
the Form-Widget proposal, we've split up the "to_html" (render) and
"to_python" into two different classes. Is that something we really
want to do? I ask sincerely, are there many use-cases for taking the
data in one way but rendering it another?
Why not Widgets as the main class, and pass it a conversion function?
class ContactForm(Form):
subject = TextArea()
date = CharField(to_python=convert_date)
email = CharField(to_python=convert_regex(email_re))
I still don't like the as_text / as_ul stuff. I think that belongs in
template tags. Although I am a bit swayed by Russell's idea of
throwing that stuff into a dictionary.
form_for_fields / form_for_model +1!
Other questions:
1. Default data looks like it will be annoying to put in the view.
2. Validation! I hate validation. Let's just ignore it. If the user
enters wrong information, its their fault, right? Why does Django
have to deal with it!?
3. We still need a way to render the whole form in a one-to-three-liner.
4. Is model validation completely dropped to the way-side? Maybe we
could make a conversion library. Conversion functions could be passed
around easily, and the Form Field "DateTime" could point to the exact
same converter that the Model Field "DateTime" points to.
5. How would sub-forms work? Lately I've been experimenting with form
"trees". I've been working on a very complex form that gathers data
via Javascript into one long JSON tree, or collection of dictionaries
and lists. Each dictionary describes data that is updated on a model
object, and each list describes a set of related objects. I'd like to
see this support a like structure.
Thanks
Rob Hudson
> and came to the same conclusion. I've attached the newest version of
> this file (gee, progressing like this is going to get unwieldy unless
> I check this into SVN), which has changed Widget.render() to take the
> name and value. I've also made some other changes, mostly in
> additional unit tests that show off some of the cool features.
For those of us not using XHTML would it be possible to add a flag to
output valid HTML?
I'm curious if something like this might be a useful way to override
the output...
>>> w = TextInput()
>>> w.template = template.Template("<input {{ attrs }}>")
>>> w.render('email', '')
u'<input type="text" name="email">'
Though this seems like overkill if you can already manipulate all the
attributes of the input tag.
-Rob
thebjorn
data to populate subwidgets? I'm trying to create a Date widget with
dropdown lists for days, months, and years similar to the one I created
here: http://www.eborger.no/hifm/login.html (feel free to play around,
it's written in php but should still be pretty solid ;-). What I did
there was update the selection widgets from post data, and when all of
them had values selected update a hidden field with the text version of
the value.
Here's what I've got so far:
First a quick-n-dirty select widget:
from django.newforms import *
from django.utils.html import escape
from datetime import date
def flatten(d):
return u' '.join('%s="%s"' % (k, escape(v)) for k, v in d.items())
class SelectWidget(Widget):
def __init__(self, options, selected=None):
super(SelectWidget, self).__init__()
self.options = options
self.selected = selected
def render(self, name, value, attrs=None):
if value is not None:
self.selected = value
final_attrs = dict(self.attrs, name=name)
if attrs:
final_attrs.update(attrs)
res = u'<select %s>' % flatten(final_attrs)
for option in self.options:
if option == self.selected:
res += u'<option selected value="%s">%s</option>' %
(option, option)
else:
res += u'<option>%s</option>' % option
res += u'</select>'
return res
then for the date widget. At the point marked HERE I need data that I
don't seem to be able to access...?
class MyDateWidget(Widget):
def render(self, name, value, attrs=None):
if value is None or value == '':
value = ''
d = POST[name+u'_day'] # <=== HERE
m = POST[name+u'_month']
y = POST[name+u'_year']
else:
d,m,y = value.split('/')
days = SelectWidget(range(1,32))
months = SelectWidget(range(1,12))
year = SelectWidget(range(2000, 2007))
slash = u' / '
res = '<input type=hidden value="%s" />' % value
res += days.render(name+u'_day', d)
res += slash
res += months.render(name+u'_month', m)
res += slash
res += year.render(name+u'_year', y)
return res
Did I miss something?
I also wrote up what I got from reading the code and tests at
http://blog.tkbe.org/archive/django-looking-at-newforms/
-- bjorn