Opened 10 years ago

Closed 10 years ago

#3196 closed defect (fixed)

Dynamically setting choices for a ChoiceField is inconsistent

Reported by: jason@… Owned by: Adrian Holovaty
Component: Forms Version:
Severity: normal Keywords: newforms choicefield
Cc: Triage Stage: Design decision needed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: UI/UX:

Description

To get your list of choices to show up in a ChoiceField before a form is submitted, you must set field.widget.choices = your_iterable. Setting field.choices has no effect. To get the field to validate you must set field.choices = your_iterable, not field.widget.choices. This is inconsistent and confusing. It seems to me that setting field.choices should be the correct thing to do for both.

Attachments (1)

choices_patch.diff (894 bytes) - added by Jason Prado <jason@…> 10 years ago.
Adds a set_choices method to ChoiceField/MultiChoiceField

Download all attachments as: .zip

Change History (7)

Changed 10 years ago by Jason Prado <jason@…>

Attachment: choices_patch.diff added

Adds a set_choices method to ChoiceField/MultiChoiceField

comment:1 Changed 10 years ago by Jason Prado <jason@…>

Summary: Dynamically setting choices for a ChoiceField is inconsistent[patch] Dynamically setting choices for a ChoiceField is inconsistent

The attached patch adds a set_choices method to ChoiceField/MultiChoiceField classes that sets the choices field in the Field and then tells the Widget about it.

comment:2 Changed 10 years ago by Adrian Holovaty

Summary: [patch] Dynamically setting choices for a ChoiceField is inconsistent[design-q] [patch] Dynamically setting choices for a ChoiceField is inconsistent

The current way it's handled is intentional -- a Field doesn't know anything about the internals of its Widget. The Widget.choices completely defines display, whereas the Field.choices completely defines valid options. I agree that this can be confusing, but something rubs me the wrong way about having a Field manipulate the internals of its Widget.

I'm putting [design-q] in the ticket summary to designate "a design decision needs to be made here."

comment:3 Changed 10 years ago by Adrian Holovaty

Summary: [design-q] [patch] Dynamically setting choices for a ChoiceField is inconsistentDynamically setting choices for a ChoiceField is inconsistent
Triage Stage: UnreviewedDesign decision needed

comment:4 Changed 10 years ago by Adrian Holovaty

Patch needs improvement: set

OK, I made the design decision. :-)

In [4378], I changed ChoiceField.__init__() so that a widget uses the choices from its ChoiceField regardless of whether the widget has its own choices.

The next step, to fix this particular ticket, is to accomodate for users who set choices on the field after the fact. Rather than use the behavior in this patch (set_choices() methods), I'd rather we used properties, to be more Pythonic and transparent.

comment:5 Changed 10 years ago by Adrian Holovaty

I'll take care of the patch, BTW.

comment:6 Changed 10 years ago by Adrian Holovaty

Resolution: fixed
Status: newclosed

(In [4379]) Fixed #3196 -- Fixed inconsistency in setting choices on ChoiceFields dynamically

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