Skip to content

FloatRangeSlider value logic #2435

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

Merged
merged 1 commit into from
Jun 8, 2019
Merged

Conversation

lheagy
Copy link
Contributor

@lheagy lheagy commented Jun 5, 2019

Update logic in setting of default value field of the FloatRangeSlider so that numpy arrays can be passed as inputs (see #2434). Note that this will still currently error in traitlets as a tuple is expected, but at least the error at this point is very clear: A trait error is raised:

TraitError: The 'value' trait of a FloatRangeSlider instance must be a tuple, but a value of class 'numpy.ndarray' (i.e. array([-5,  5])) was specified.

I would appreciate the ability to simply provide a 2-entry long numpy array in the value field. What do folks think of adding something like:

try:
    proposal_tuple = tuple(proposal['value'])
    proposal['value'] = proposal_tuple
except:
    raise TraitError

to the validation of the FloatRange value

@validate('value')
def _validate_value(self, proposal):
lower, upper = proposal['value']
if upper < lower:
raise TraitError('setting lower > upper')
return lower, upper

or would it make more sense to do something at the traitlets level?

Update logic in setting of default `value` field of the FloatRangeSlider so that numpy arrays can be passed as inputs (closes jupyter-widgets#2434)
@lheagy lheagy changed the title Update widget_float.py FloatRangeSlider value logic Jun 5, 2019
@vidartf
Copy link
Member

vidartf commented Jun 6, 2019

Currently, the container traits from traitlets have a whitelist of types they can cast from. I don't think we can use that system for adding support for numpy, as that would add a dependency on numpy. Simply casting to tuple should be ok though, since we are checking the type of the sub-elements (i.e. we will avoid a false positive for tuple('ok')). So unless someone else can think of some other false positives, I say the validate pattern you suggested should be ok.

@vidartf vidartf added this to the 7.5 milestone Jun 8, 2019
@vidartf
Copy link
Member

vidartf commented Jun 8, 2019

@jupyter-widgets/jupyter-widgets We should be able to merge this PR as is for the more helpful error message, and leave the other suggestions as future work for further discussion.

@SylvainCorlay SylvainCorlay merged commit ec3db3e into jupyter-widgets:master Jun 8, 2019
@SylvainCorlay
Copy link
Member

Thanks! This looks good to me.

@lock lock bot added the resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. label May 21, 2020
@lock lock bot locked as resolved and limited conversation to collaborators May 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants