Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make SelectionWidget.values a dict #5012

Merged
merged 4 commits into from Feb 4, 2014
Merged

Conversation

minrk
Copy link
Member

@minrk minrk commented Feb 3, 2014

rename labels to value_names and _value to value_name.

To specify a mapping of value names and values, use a dict. If you specify values=[list], then an OrderedDict will be used, whose keys are str(value).

Assignment to values after construction only supports a dict.

closes #4961

"""

value = Any(help="Selected value")
values = Dict(help="Dictionary of label:value the user can select")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the help string here say name:value?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, fixed.

# preserve list order with an OrderedDict
kwargs['values'] = d = OrderedDict()
for v in values:
d[unicode_type(v)] = v
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's possible to construct an OrderedDict with 2-tuples if we want to make this a bit shorter:

OrderedDict((unicode_type(v), v) for v in values)

@minrk
Copy link
Member Author

minrk commented Feb 4, 2014

Various changes recommended by @takluyver should be done.

The keys of this dictionary are also available as value_names.
""")
value_name = Unicode(help="The name of the selected value", sync=True)
value_names = List(help="""List of names for each value.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe our List traitlet can validate the type of items in the list, i.e. List(Unicode, ...). Is that worth using here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is, and done.

rename 'labels' and '_value' to 'value_names' and 'value_name'.

To specify a mapping of value names and values, use a dict.
If you specify `values=[list]`, then an OrderedDict will be used.

Assignment after construction only supports a dict.
@minrk
Copy link
Member Author

minrk commented Feb 4, 2014

value_names is now read-only, so it is clear that there is only one place to edit this information - the values dictionary.

@takluyver takluyver added this to the 2.0 milestone Feb 4, 2014
@ellisonbg
Copy link
Member

Looks good, merging.

ellisonbg added a commit that referenced this pull request Feb 4, 2014
Make `SelectionWidget.values` a dict
@ellisonbg ellisonbg merged commit a4c9500 into ipython:master Feb 4, 2014
@minrk minrk deleted the selection-dict branch March 31, 2014 23:36
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
Make `SelectionWidget.values` a dict
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug when constructing a selection widget with both values and labels
4 participants