SingleSelectField should call from_python rather than to_python
Rick
I was working with a SingleSelectField and a complicated validator and
noticed that, when the widget is rendering itself, it calls
_is_option_selected, which in turn calls the validator's "to_python".
It seems that "from_python" should actually be called, since we're
starting with a Python value and ending up with a form value to
compare against. (Plus, it makes my code work when I switch them.)
I'm not sure if this should be created as a ticket/patch/whatever, or
if my understanding is just wrong, so if anyone could point me in the
right direction, it would be greatly appreciated.
Thanks,
Rick
Diez B. Roggisch
No, it shouldn't, as the validator will work on a request-parameter
(always a string in the first place) but should result in something that
is in the options list, e.g. an id from an object.
If you show us some code, we might be able to spot why things don't work
for you.
Diez
Rick
message), but I may not have stated the problem as clearly as
possible. I am concerned with how the SelectionField *displays* its
data, not how it *validates* incoming data. (Validation works just
fine). I provide the SingleSelectField with a Python value to be
*displayed*, and it (should, I believe) either
a) Convert that python value to a string using from_python and compare
it with the <option>s to see which one is selected, OR
b) Convert each <option> value to a corresponding Python value using
to_python to see which one is selected.
What it actually *does*, however, is
c) Try to convert the *python value* to another *python value* using
to_python and compare it with the <option>s to see which one is
selected (obviously never matching anything)
Before I get into my specific code, here's a quick contrived example
of how it currently works and how I think it should work:
@expose(template='.foo')
def test(self):
class TestObj(object):
def __init__(self):
self.testval='three:3'
class TestValidator:
def to_python(self, value, state=None):
log.info('Call to_python on value "%s"', value)
result = {'1':'one:1',
'2':'two:2',
'3':'three:3',
'4':'four:4',
'5':'five:5' }.get(value, None)
if result is None: raise Invalid('Bad Value', value,
state)
return result
def from_python(self, value, state=None):
log.info('Call from_python on value "%s"', value)
name, id = value.split(':')
return id
class TestFields(WidgetsList):
testval=SingleSelectField(
'testval', label='Test Value',
options=[('1','one'), ('2','two'), ('3','three'),
('4','four')],
validator=TestValidator())
test_form = TableForm('test', fields=TestFields())
return dict(form=test_form,
item=TestObj())
The template has the expected ${form.display(item)}. If I use the
widgets code as-is, I get the following log output:
...
2007-03-04 07:00:58,853 tutornet.controllers INFO Call to_python on
value "three:3"
2007-03-04 07:00:58,982 tutornet.controllers INFO Call to_python on
value "three:3"
2007-03-04 07:00:58,982 tutornet.controllers INFO Call to_python on
value "three:3"
2007-03-04 07:00:58,983 tutornet.controllers INFO Call to_python on
value "three:3"
And of course, no option in the select field is ever selected. If I
switch the widgets code to call from_python, however, I get the
following log output:
...
2007-03-04 07:04:34,820 tutornet.controllers INFO Call from_python on
value "three:3"
2007-03-04 07:04:34,822 tutornet.controllers INFO Call from_python on
value "three:3"
2007-03-04 07:04:34,823 tutornet.controllers INFO Call from_python on
value "three:3"
2007-03-04 07:04:34,825 tutornet.controllers INFO Call from_python on
value "three:3"
And the third option is correctly selected.
Now in my *actual* code, I have a validator which converts incoming
ids into objects using the SA .get method:
class SQL(FancyValidator):
not_empty=True
messages = dict(
integer='Please enter an integer value',
notFound='The %(model)s with ID %(value)s was not found')
def __init__(self, model, method='get', base=Int, *l, **kw):
self.model = model
self.method = method
self.base = base
self.l = l
self.kw = kw
self._bound = False
if isinstance(model, str):
##print 'Delay binding of SQL validator on %s' % model
pass
else:
self._bind()
def _bind(self):
if isinstance(self.model, str):
self.model = classregistry.findClass(self.model)
self.method = getattr(self.model, self.method)
if 'if_empty' in self.kw:
self.not_empty=False
self.base = self.base(*self.l, **self.kw)
self._bound = True
def _to_python(self, value, state):
if not self._bound:
self._bind()
value = self.base.to_python(value)
try:
return self.method(value)
except SQLObjectNotFound:
raise Invalid(self.message('notFound', state,
model=self.model.__name__,
value=value),
value, state)
def _from_python(self, value, state):
if isinstance(value, int): keyval = value
else: keyval = value.id
return Int().from_python(keyval, state)
This is used in a form as follows:
class CurriculumItemFields(WidgetsList):
type=SingleSelectField(label='Item Type',
options=lambda: [(t.id, t.name) for t in
model.ItemType.select()],
validator=SQL(model.ItemType))
name=TextField()
description=TextArea(attrs=dict(rows=2))
textile=TextArea(label='Content', attrs=dict(rows=10),
validator=UnicodeString(if_empty=''))
attachment=FileField()
edit_curriculum_item_form=TableForm(
'curriculum_item_form',
fields=CurriculumItemFields(),
action='./save',
submit_text='Save Curriculum Item')
In my controller, I retrieve a CurriculumItem (which has a "type_id"
foreign key into ItemType's table) from the database and then send it
to the template as (simplified):
return dict(item=model.CurriculumItem.get(id),
form=edit_curriculum_item_form)
When I attempt to do form.display(item) inside the Kid template,
however, it does not correctly set the "selected" attr on the correct
option (it does not set *any* "selected" attrs, in fact.) This is
because it is trying to call SQL(model.ItemType).to_python(<ItemType
instance ...>) inside _is_option_selected when rendering the widget.
Replacing the to_python with from_python works beautifully, correctly
coercing the value both in valididating the form and when rendering
it.
On Mar 3, 12:12 pm, "Diez B. Roggisch" <diez.roggi...@artnology.com>
wrote:
Diez B. Roggisch
To me it looks as if you reversed the to- and from-values - no wonder
that you need to switch the calls.
Your option list is this:
[('1','one'), ('2','two'), ('3','three'), ('4','four')]
Which means that '1', '2', ... _are_ the *python* values we're talking
here about.
But in your validator's _to_python-call, you map '1' to 'one:1'.
Now obviously '1' != 'one:1' and so forth, so that the selected item
will never get displayed correctly.
So either your option-list reads
[('one:1','one'), ('two:2','two'), ('three:3','three'), ('four:4','four')]
or your mapping has to work differently.
Diez
Rick
wrote:
Valid form values: '1', '2', '3', '4'
Valid Python values: 'one:1', 'two:2', 'three:3', 'four:4'
The validator correctly converts the "form" value '1' to the "Python"
value 'one:1' and back.
> Your option list is this:
>
> [('1','one'), ('2','two'), ('3','three'), ('4','four')]
>
> Which means that '1', '2', ... _are_ the *python* values we're talking
> here about.
No, it means that '1', '2', ... are the values that will be inserted
*without conversion* into the "value" attribute of the <option> tag in
the template.
> But in your validator's _to_python-call, you map '1' to 'one:1'.
Right. '1' is a form value, 'one:1' is a Python value.
> Now obviously '1' != 'one:1' and so forth, so that the selected item
> will never get displayed correctly.
>
> So either your option-list reads
>
> [('one:1','one'), ('two:2','two'), ('three:3','three'), ('four:4','four')]
Well, I tried that, and didn't have any more success.
> or your mapping has to work differently.
>
> Diez
Looking at the template for the SingleSelectField (and verified
through testing), the '1', '2', etc. are not treated as Python values
at all during the rendering of the template. Instead, whatever is
there is shoved into the "value" attribute of the <option> field
without any conversion done at all:
Quoth turbogears/widgets/forms.py:
<option py:for="value, desc, attrs in options"
value="${value}"
py:attrs="attrs"
py:content="desc"
/>
And, as I confirmed with testing, replacing the '1' with '1:one' also
does *not* select the correct value. I created a new test with an
explicit Python "options" list just to check your idea on the '1'
being a Python value:
@expose(template='tutornet.foo')
def test(self):
class TestValue(object):
def __init__(self, value):
self.value = value
class TestObj(object):
def __init__(self):
self.testval=TestValue(3)
class TestValidator:
def to_python(self, value, state=None):
log.info('Call to_python on value "%s"', value)
result = {'1':TestValue(1),
'2':TestValue(2),
'3':TestValue(3),
'4':TestValue(4),
'5':TestValue(5) }.get(value, None)
if result is None: raise Invalid('Bad Value', value,
state)
return result
def from_python(self, value, state=None):
log.info('Call from_python on value "%s"', value)
return str(value.value)
class TestFields(WidgetsList):
testval=SingleSelectField(
'testval', label='Test Value',
# Here I try putting the Python values into the
options list
options=[(TestValue(1),'one'), (TestValue(2),'two'),
(TestValue(3),'three'), (TestValue(4),
'four')],
validator=TestValidator())
test_form = TableForm('test', fields=TestFields())
return dict(form=test_form,
item=TestObj())
A snippet of the HTML code generated is below:
... <OPTION VALUE="<tutornet.controllers.root.TestValue object at
0x2aaaaad92250>">one</OPTION> ...
So you can see that the values in the options list are not converted
on display.
Diez B. Roggisch
> *without conversion* into the "value" attribute of the <option> tag in
> the template.
This is your misconception. These are the values that are used to be
compared with the _to_python-converted request parameter value. You are
right though that they aren't converted during rendering, except from
being stringified.
See the following example for a working pre-selection of a value in a
SingleSelectField:
---- controller -----
from turbogears import controllers, expose
import turbogears.widgets as w
import turbogears.validators as v
def calc_options():
return [(i, "value: %i" % i) for i in xrange(1000)]
form = w.ListForm(fields=[w.SingleSelectField('test', options=calc_options,
validator=v.Int(not_empty=True)),
w.TextField('foo')])
class Root(controllers.RootController):
@expose(template="testproject.templates.welcome")
def index(self):
import time
# log.debug("Happy TurboGears Controller Responding For Duty")
return dict(now=time.ctime(), form=form, selected=100, foo='bar')
----- template ----
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"
"http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml"
xmlns:py="http://purl.org/kid/ns#"
py:extends="'master.kid'">
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type"
py:replace="''"/>
<title>Welcome to TurboGears</title>
</head>
<body>
${selected}
${form.display(dict(test='20', foo='baz'))}
</body>
</html>
So, again: the values in the option-list are compared to whatever the
validators _to_python-method yields.
From the formencode docs:
"""
The basic metaphor for validation is to_python and from_python. In this
context "Python" is meant to refer to "here" -- the trusted application,
your own Python objects. The "other" may be a web form, an external
database, an XML-RPC request, or any data source that is not completely
trusted or does not map directly to Python's object model. to_python is
the process of taking external data and preparing it for internal use,
from_python generally reverses this process (from_python is usually the
less interesting of the pair, but provides some important features).
"""
So - from that POV, it's the right way TG uses the validation stuff.
OTH, it is sort of an omission that the current implementation doesn't
use to from_python-method to convert e.g. model objects to id to render
them as part of a select.
Diez
Rick
widgets. OK, let me see if I can recap what you are saying. The
current implementation of _is_option_selected assumes that the "value"
in the options list [(value,desc,options)] is a Python value, but the
current implementation of the select field templates do not correctly
render that value into the template using the decorator's from_python
method, making it difficult to impossible to pass complex Python
values in the options list.
In that case, let me suggest that the SelectionField.update_params()
method be updated to actually use the validator's from_python method
to render the values into the template. I have included below a
version of the method which does just that. Unfortunately, this alone
does not fix the problem. Now, when update_params calls
is_option_selected, it is comparing two Python values (one from the
list, one from the value passed into the form). So in this case, it
seems that is_option_selected should skip the conversion step, since
it is comparing two Python values.
I have included monkey-patched versions of these two methods that seem
to give the desired behavior (passing Python values in the options
parameter, correctly rendering python values to the form using
from_python, and correctly determining which one is selected when
displaying/rendering the template). I've also included a fixed
version of my test controller method that works with these patches
fine. Do you see any problem with applying patches (real ones,
obviously, not these monkey-patches) to turbogears/widgets/forms.py?
Thanks,
Rick Copeland
=== Test case begin ===
@expose(template='foo')
def test(self):
class TestValue(object):
def __init__(self, value):
self.value = value
def __eq__(self, other):
# necessary to allow the SelectionField to determine
which value is selected
return self.value == other.value
class TestObj(object):
def __init__(self):
self.testval=TestValue(3)
class TestValidator:
def to_python(self, value, state=None):
log.info('Call to_python on value "%s"', value)
result = {'1':TestValue(1),
'2':TestValue(2),
'3':TestValue(3),
'4':TestValue(4),
'5':TestValue(5) }.get(value, None)
if result is None: raise Invalid('Bad Value', value,
state)
return result
def from_python(self, value, state=None):
return str(value.value)
class TestFields(WidgetsList):
testval=SingleSelectField(
'testval', label='Test Value',
options=[(TestValue(1),'one'), (TestValue(2),'two'),
(TestValue(3),'three'), (TestValue(4),
'four')],
validator=TestValidator())
test_form = TableForm('test', fields=TestFields())
return dict(form=test_form,
item=TestObj())
=== Test case end ===
=== Monkey Patches begin ===
def SF_update_params(self, d):
super(SelectionField, self).update_params(d)
grouped_options = []
options = []
d['options'] = self._extend_options(d['options'])
for optgroup in d["options"]:
if isinstance(optgroup[1], list):
group = True
optlist = optgroup[1]
else:
group = False
optlist = [optgroup]
for i, option in enumerate(optlist):
if len(option) is 2:
option_attrs = {}
elif len(option) is 3:
option_attrs = option[2]
if self._is_option_selected(option[0], d['value']):
option_attrs[self._selected_verb] =
self._selected_verb
optlist[i] = (self.validator.from_python(option[0]), #
This is the only line I changed
option[1], option_attrs)
options.extend(optlist)
if group:
grouped_options.append((optgroup[0], optlist))
# options provides a list of *flat* options leaving out any
eventual
# group, useful for backward compatibility and simpler widgets
d["options"] = options
if grouped_options:
d["grouped_options"] = grouped_options
else:
d["grouped_options"] = [(None, options)]
SelectionField.update_params = SF_update_params
def SF_is_option_selected(self, option_value, value):
# Removed validation logic here
if value is not None:
if self._multiple_selection:
if option_value in value:
return True
else:
if option_value == value:
return True
return False
SelectionField._is_option_selected = SF_is_option_selected
=== Monkey Patches end ===
On Mar 4, 6:06 pm, "Diez B. Roggisch" <diez.roggi...@artnology.com>
wrote:
Diez B. Roggisch
> First off, thanks for the insights into the design philosophy of TG
> widgets. OK, let me see if I can recap what you are saying. The
> current implementation of _is_option_selected assumes that the "value"
> in the options list [(value,desc,options)] is a Python value, but the
> current implementation of the select field templates do not correctly
> render that value into the template using the decorator's from_python
> method, making it difficult to impossible to pass complex Python
> values in the options list.
Yes, that sounds correct to me.
> In that case, let me suggest that the SelectionField.update_params()
> method be updated to actually use the validator's from_python method
> to render the values into the template. I have included below a
> version of the method which does just that. Unfortunately, this alone
> does not fix the problem. Now, when update_params calls
> is_option_selected, it is comparing two Python values (one from the
> list, one from the value passed into the form). So in this case, it
> seems that is_option_selected should skip the conversion step, since
> it is comparing two Python values.
>
> I have included monkey-patched versions of these two methods that seem
> to give the desired behavior (passing Python values in the options
> parameter, correctly rendering python values to the form using
> from_python, and correctly determining which one is selected when
> displaying/rendering the template). I've also included a fixed
> version of my test controller method that works with these patches
> fine. Do you see any problem with applying patches (real ones,
> obviously, not these monkey-patches) to turbogears/widgets/forms.py?
It does sound reasonable, yet I'm pretty sure it will break existing code.
Which would disqualify it for anything but a new release, one or even a few
versions from now.
I suggest you try and get one of the core developers to comment on this -
either explicitly, or via submitting a bug.
Diez
Rick
with an attached patch (which also contains a test case) that does not
break any of the existing tests. (I had to tweak it slightly to
handle MultipleSelectFields, but it seems to work now.)
Thanks again for helping me think through this.
-Rick
On Mar 5, 8:48 am, "Diez B. Roggisch" <diez.roggi...@artnology.com>
wrote: