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 widget managers completely in charge of loading widgets #1313

Merged
merged 3 commits into from Apr 27, 2017

Conversation

@jasongrout
Copy link
Member

@jasongrout jasongrout commented Apr 26, 2017

This is an iteration on fixing #1242.

The basic idea is that we now consider the module/version to be semantic namespace (i.e., the spec for the model or view implementation), and the package/version to be a concrete implementation that can be loaded. The original thought was that if the package/version was not specified, it would default to the module/version. However, since package/version will be set on basic widgets classes, they must be overridden in custom widgets to be consistent.

@jasongrout
Copy link
Member Author

@jasongrout jasongrout commented Apr 26, 2017

However, since package/version will be set on basic widgets classes, they must be overridden in custom widgets to be consistent.

Most custom widgets inherit from DOMWidget or just Widget. A hack to get around the above issue would be to just give the package on the core widgets. It's not quite clear where it puts Widget, DOMWidget, and LabeledDOMWidget.

@jasongrout
Copy link
Member Author

@jasongrout jasongrout commented Apr 26, 2017

Okay, new thought: no package name stuff. The widget manager is responsible for loading correct versions of the models/views, full stop. Right now I'm thinking of three widget managers:

  1. The notebook - widget libraries are installed and those are the ones available. It's up to the user to make sure the correct version of the library is installed.
  2. JupyterLab - again, specific widget extensions are registered. It's up to the user to make sure the correct version of the library is installed.
  3. The embedded widget manager - this is the only one that really uses the module_version as an npm package version. For many widgets, this will be correct. Any where it's not, the widget manager is responsible for translating module versions to package versions.

For each of these cases, the version of jupyter-js-widgets is special-cased to be the one we are extending.

So I'll strip this PR down to some of the cleanup I've done, and then the major change will be:

  1. Make each widget manager completely in charge of loading the correct class for views and models. The default implementation will not assume require. I'm still undecided if the manager-base will provide code to load widgets from the jupyter-js-widgets package (leaning against it so that in the future, it's easier to separate out the base manager from our core widget implementation).
  2. Change the documentation of the _module_* attributes to not refer to npm packages, but rather versions of the models/views (which can coincide with npm package names/versions by convention/coincidence).

@maartenbreddels
Copy link
Member

@maartenbreddels maartenbreddels commented Apr 26, 2017

If that means that views will not be loaded via require by default (in the notebook) I think that is a big plus for development, since that makes js 'reloading' via actually work, like what i do here.

@jasongrout
Copy link
Member Author

@jasongrout jasongrout commented Apr 26, 2017

Views would still be loaded using require -that's how things are done in the classic notebook

@jasongrout jasongrout force-pushed the packages branch 3 times, most recently from 5a58662 to ddc34ca Apr 26, 2017
@jasongrout jasongrout force-pushed the packages branch 2 times, most recently from 54126ca to 6ef4903 Apr 26, 2017
@jasongrout jasongrout changed the title WIP Revamp the version number semantics Make widget managers completely in charge of loading widgets Apr 26, 2017
@jasongrout
Copy link
Member Author

@jasongrout jasongrout commented Apr 26, 2017

@SylvainCorlay, @maartenbreddels - Any thoughts on this before merging?

@jasongrout jasongrout merged commit 05ab3ad into jupyter-widgets:master Apr 27, 2017
1 check passed
@jasongrout jasongrout mentioned this pull request Apr 27, 2017
@jasongrout jasongrout added this to the 7.0 milestone May 2, 2017
@jasongrout jasongrout added this to the 7.0 milestone May 2, 2017
@mlucool mlucool mentioned this pull request Aug 3, 2017
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants