Opened 9 years ago

Closed 6 years ago

#5481 closed (wontfix)

ChoiceField/ModelChoiceField always returns strings, regardless of type in choices

Reported by: Robert Coup Owned by: nobody
Component: Forms Version: master
Severity: Keywords: sprint14sep
Cc: philippe.raoult@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: UI/UX:

Description

f = ChoiceField(choices=((1,'1'), (2,'2')))
f.clean('1')
u'1'
# expected: 1 (integer)

While normally this doesn't cause problems (models are pretty good about accepting strings) it can be extra hassle for custom forms.

The attached patch changes the behaviour of ChoiceField and ModelChoiceField to return exactly what was specified in choices (be it integer, string, unicode, whatever).

Attachments (3)

choicefield_correct_type.diff (5.2 KB) - added by Robert Coup 9 years ago.
choicefield_correct_type.2.diff (5.5 KB) - added by Robert Coup 9 years ago.
a slightly cleaner implementation
choicefield_correct_type.3.diff (4.6 KB) - added by Robert Coup 9 years ago.
Forces strings to unicode

Download all attachments as: .zip

Change History (14)

Changed 9 years ago by Robert Coup

comment:1 Changed 9 years ago by Robert Coup

For the archives, originally kicked off from #3729

Changed 9 years ago by Robert Coup

a slightly cleaner implementation

comment:2 Changed 9 years ago by Simon G. <dev@…>

Triage Stage: UnreviewedReady for checkin

comment:3 Changed 9 years ago by Malcolm Tredinnick

I'm not comfortable with this change. We have a policy that whenever a string is involved, be it a UTF-8 bytestring or a Unicode string, Django will convert it a unicode object for internal use. This is breaking out of that policy by trying to preserve the str type. It's a bit inconsistent.

Is it satisfactory to pass strings_only=True to smart_unicode() instead of removing the smart_unicode() calls? That would make me happier.

comment:4 Changed 9 years ago by Malcolm Tredinnick

Patch needs improvement: set
Triage Stage: Ready for checkinAccepted

comment:5 Changed 9 years ago by Philippe Raoult

Cc: philippe.raoult@… added
Owner: changed from nobody to Robert Coup

likely related to #5247.

comment:6 Changed 9 years ago by Robert Coup

#5247 isn't a duplicate - this ticket is related to how the choices are evaluated in ChoiceField.clean(), the other ticket is related to how the choices get evaluated for display in the widget render method.

comment:7 Changed 9 years ago by Robert Coup

Status: newassigned

I hadn't seen strings_only before, but i have some reservations about it - namely, it should probably be called ignore_none_and_int. That prevents objects, doubles, booleans, Decimals, etc as being keys.

Would doing isinstance(value, str) and unicode'ing only those be acceptable? (Patch .3 does it this way)

Changed 9 years ago by Robert Coup

Forces strings to unicode

comment:8 Changed 8 years ago by evenrik

Use the new TypedChoiceField to solve this issue.

comment:9 Changed 7 years ago by klrfin@…

There's no TypedMultipleChoiceField.

comment:10 Changed 7 years ago by Robert Coup

Owner: changed from Robert Coup to nobody
Status: assignednew

comment:11 Changed 6 years ago by Russell Keith-Magee

Resolution: wontfix
Status: newclosed

r14829 added a TypedMultipleChoiceField, making this a wontfix.

Note: See TracTickets for help on using tickets.
Back to Top