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

Multiple widgets have the same model/view information #1256

Closed
jasongrout opened this issue Apr 5, 2017 · 12 comments · Fixed by #1290
Closed

Multiple widgets have the same model/view information #1256

jasongrout opened this issue Apr 5, 2017 · 12 comments · Fixed by #1290
Labels
resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion.
Milestone

Comments

@jasongrout
Copy link
Member

jasongrout commented Apr 5, 2017

A ramification of #1228 is that the FloatSlider and FloatRangeSlider are confused in the kernel, since both have exactly the same model and view implementation. So we basically have different presentations of the same model to the user, and have to disambiguate between them.

I think this is a problem with the models, not with the idea of #1228. I think we should probably have separate models for float slider and float range slider, for example.

@jasongrout jasongrout added this to the 7.0 milestone Apr 5, 2017
@jasongrout
Copy link
Member Author

Here are the current duplicates (i.e., python-side classes that implement the same model and view):

  • ('jupyter-js-widgets', '3', 'IntTextModel', 'jupyter-js-widgets', '3', 'IntTextView'): [<class 'ipywidgets.widgets.widget_int.IntText'>, <class 'ipywidgets.widgets.widget_int.BoundedIntText'>]
  • ('jupyter-js-widgets', '3', 'IntSliderModel', 'jupyter-js-widgets', '3', 'IntSliderView'): [<class 'ipywidgets.widgets.widget_int.IntSlider'>, <class 'ipywidgets.widgets.widget_int.IntRangeSlider'>]
  • ('jupyter-js-widgets', '3', 'FloatTextModel', 'jupyter-js-widgets', '3', 'FloatTextView'): [<class 'ipywidgets.widgets.widget_float.FloatText'>, <class 'ipywidgets.widgets.widget_float.BoundedFloatText'>]
  • ('jupyter-js-widgets', '3', 'ProgressModel', 'jupyter-js-widgets', '3', 'ProgressView'): [<class 'ipywidgets.widgets.widget_int.IntProgress'>, <class 'ipywidgets.widgets.widget_float.FloatProgress'>]
  • ('jupyter-js-widgets', '3', 'FloatSliderModel', 'jupyter-js-widgets', '3', 'FloatSliderView'): [<class 'ipywidgets.widgets.widget_float.FloatSlider'>, <class 'ipywidgets.widgets.widget_float.FloatRangeSlider'>]

@SylvainCorlay
Copy link
Member

They used to be different, but now they only differ for the default formatting of the readout.

@jasongrout
Copy link
Member Author

Since the default is part of the model spec, that points to them needing to be different models.

@SylvainCorlay
Copy link
Member

I know, but we have not released the spec, which may say "any value" which satisfies the formatting mini-language.

@jasongrout
Copy link
Member Author

which may say "any value" which satisfies the formatting mini-language.

Are you saying that the spec may not give a specific default value, and it would be up to the implementation? I think that would be a mistake.

@SylvainCorlay
Copy link
Member

which may say "any value" which satisfies the formatting mini-language.

Are you saying that the spec may not give a specific default value, and it would be up to the implementation? I think that would be a mistake.

Nope, I mean that there would be only one numerical slider widget with a default value for the format that is meaningful for all numerical types

The Python float slider class would inherit from this numerical slider and merely define a new default value for the trait type. Up on serialization, this would typically result in this being saved in the serialized state since different from the default.

@SylvainCorlay
Copy link
Member

The only complication in the code is that the default comparison retrieves the default value from the trait type, so we would need to do something different there.

@jasongrout
Copy link
Member Author

So the issue at hand is instantiating a python version of the model from the javascript side. If we have multiple python implementations for the same model and view, this would be ambiguous.

Before, we hardcoded information about not just the model to instantiate, but about the specific object on the other side implementing the model. That's what #1228 takes away, leaving the model and view information as the final word about what to instantiate on the other side.

Is it all right with you if, when we instantiate a slider from javascript, the corresponding python object is just a generic slider, not a FloatSlider or IntSlider?

@SylvainCorlay
Copy link
Member

SylvainCorlay commented Apr 5, 2017

Is it all right with you if, when we instantiate a slider from javascript, the corresponding python object is just a generic slider, not a FloatSlider or IntSlider?

I am still not sold on giving up the @register decorator, which had other benefits than just the instantiation from the js side.

@jasongrout
Copy link
Member Author

to be clear, it's not giving up the registration, it's giving up assigning specific names to kernel-side variations of the same model. The focus here is making the model spec itself self-contained, and the sole reference if you want to implement the models in a widget package.

@SylvainCorlay
Copy link
Member

SylvainCorlay commented Apr 5, 2017

Advantages of the @register

  • a simple API on the python side to list all available widget types (as of what was imported)
  • which also helps with building tools such drag-&-drop GUI builders which should know this list.

@jasongrout
Copy link
Member Author

Again, we still have registration, and we still have the @register decorator. So we still have a simple API on the python side to list all available widgets. The change was that the registration key is the model and view information in the model itself, not some special string. This means that the model spec is the final word on the behavior of models, which means it is much easier to implement other backends with the same models.

@jasongrout jasongrout changed the title Errors in instantiating models Multiple widgets have the same model/view information Apr 8, 2017
jasongrout added a commit to jasongrout/ipywidgets that referenced this issue Apr 17, 2017
@github-actions github-actions bot added the resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. label Feb 13, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 13, 2021
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 a pull request may close this issue.

2 participants