Opened 7 years ago

Closed 4 years ago

#5481 closed (wontfix)

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

Reported by: rcoup 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 rcoup 7 years ago.
choicefield_correct_type.2.diff (5.5 KB) - added by rcoup 7 years ago.
a slightly cleaner implementation
choicefield_correct_type.3.diff (4.6 KB) - added by rcoup 7 years ago.
Forces strings to unicode

Download all attachments as: .zip

Change History (14)

Changed 7 years ago by rcoup

comment:1 Changed 7 years ago by rcoup

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

For the archives, originally kicked off from #3729

Changed 7 years ago by rcoup

a slightly cleaner implementation

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

  • Triage Stage changed from Unreviewed to Ready for checkin

comment:3 Changed 7 years ago by mtredinnick

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 7 years ago by mtredinnick

  • Patch needs improvement set
  • Triage Stage changed from Ready for checkin to Accepted

comment:5 Changed 7 years ago by PhiR

  • Cc philippe.raoult@… added
  • Owner changed from nobody to rcoup

likely related to #5247.

comment:6 Changed 7 years ago by rcoup

#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 7 years ago by rcoup

  • Status changed from new to assigned

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 7 years ago by rcoup

Forces strings to unicode

comment:8 Changed 6 years ago by evenrik

Use the new TypedChoiceField to solve this issue.

comment:9 Changed 6 years ago by klrfin@…

There's no TypedMultipleChoiceField.

comment:10 Changed 5 years ago by rcoup

  • Owner changed from rcoup to nobody
  • Status changed from assigned to new

comment:11 Changed 4 years ago by russellm

  • Resolution set to wontfix
  • Status changed from new to closed

r14829 added a TypedMultipleChoiceField, making this a wontfix.

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