feat: use jupyterlab-manager output widget #46

msuperina · 2019-01-22T15:19:00Z

Most of the widget manager logic is duplicated from the
@jupyter-widgets/jupyterlab-manager package. This PR aims at using the
package directly.
The main benefit of this approach is support for output of "stream" messages.
It becomes possible to render and interact with interactive widgets.
An example of interactive widget has been added in
notebooks/interactive.ipynb

Fix #19
Fix #23

msuperina · 2019-01-22T15:25:54Z

js/webpack.config.js

@@ -9,7 +9,7 @@ var loaders = [
          'babel-preset-stage-0',
        ].map(require.resolve),
    },
-    exclude: /node_modules/
+    exclude: /node_modules(?!(\/|\\)@jupyter-widgets(\/|\\)jupyterlab-manager)/


If I import the jupyterlab-manager WidgetManager class from node-modules it comes as es5 class and cannot be extended using the es6 extends syntax. If instead I add it to our babel transpilation process it is possible to extend it as babel sees it as es6 class.

wow, gotcha

This puzzled me a bit, but it is the other way around. The output was an ES6 class (https://github.com/jupyter-widgets/ipywidgets/blob/2b91eac459bf4929c3b92794bfb0dc7deef2a353/packages/jupyterlab-manager/src/tsconfig.json#L12) which babel then tried to inherit from, and tried to construct without using new.
I've not configured babel 7 using more modern target browsers such that it output es6 classes for voila as well. The bundle size also seems to have halved.

Apologies for confusing you, I wrote the comment much after writing the code and I realize now how confusing it can be. Also, I should have suggested a babel upgrade as you did in your commit, as at some point I did it as well while implementing this fix. I preferred not to as I am not a maintainer of this repo and such a change could have raise concerns over the the acceptance of this PR.

msuperina · 2019-01-22T15:27:25Z

js/manager.js

+if (typeof window !== "undefined" && typeof window.define !== "undefined") {
+    window.define("@jupyter-widgets/base", base);
+    window.define("@jupyter-widgets/controls", controls);
+    window.define("@jupyter-widgets/output", output);


I would like to find a way to get rid of those lines if possible.
If I remove window.define("@jupyter-widgets/base", base); I cannot load the bqplot example which requires the resource from unpkg and needs an external @jupyter-widgets/base

Yes, unfortunately this is still required for the bundle-based means of loading custom widgets.

Eventually, we will have voila templates that don't load AMD bundles and will not require this.

Maybe a good way to move towards a cleaner solution would be to move this out of this file and include it into a the nbconvert template.

I think we can do that in a different PR ok?

msuperina · 2019-01-22T15:28:48Z

js/manager.js

+            return super.loadClass(className, moduleName, moduleVersion);
+        }
+        else {
+            return htmlManager.loadClass(className, moduleName, moduleVersion);


I am using the HTMLManager loadClass because it fallbacks to unpkg when a class is not resolved, and I did not want to replicate the code. The jupyterlab-manager does not fall back.

I'd say, use the original loader idea. Since you already check those 3 modes, I think you could do:

... else return this.loader(moduleName, moduleVersion)

I am not sure I understand: for the base, controls and output modules I want to use the jupyterlab-manager loadClass, but for the rest of the modules I want to keep using the html-manager loadClass, which internally handles promise rejections and allows me to not replicate such code.

I duplicated the code, I think in this case it's cleaner for the moment, so it's safer in case we want to refactor this.

msuperina · 2019-01-22T15:30:26Z

js/manager.js

+        }
+    }
+
+    _registerWidgets() {


This method exists because the jupyterlab-manager API does not expose the plugin extension directly. I don't know how keen they are to change their API. Given that I see recurrent names of people in all jupyter related repositories I hope this comment can be picked up.

I'm not sure I follow you, this looks fine to me. I guess you see a reason not to have this code, could you explain?

Sorry I realize my comment was not clear. This piece of code exists in the jupyterlab-manager package and a slight modification of their API could allow us to fully consume their module without replicating some of the code.

msuperina · 2019-01-22T15:32:21Z

This is still a WIP and needs testing and cleanup. I thought opening this PR would help people playing with this branch.

SylvainCorlay · 2019-01-22T22:12:26Z

@msuperina thanks for your contribution, this is great, and quite a feat!

I will put some cycles into this tomorrow!

msuperina · 2019-01-24T13:11:30Z

@SylvainCorlay thanks for looking into this. I will wait for your feedback before any refactoring.

maartenbreddels

Great work Marzio. I plan to take a look tomorrow unless Sylvain beats me to it. Some small comments for now.

maartenbreddels · 2019-01-24T15:32:06Z

js/manager.js

+            return super.loadClass(className, moduleName, moduleVersion);
+        }
+        else {
+            return htmlManager.loadClass(className, moduleName, moduleVersion);


I'd say, use the original loader idea. Since you already check those 3 modes, I think you could do:

... else return this.loader(moduleName, moduleVersion)

maartenbreddels · 2019-01-24T15:37:21Z

js/manager.js

+        }
+    }
+
+    _registerWidgets() {


I'm not sure I follow you, this looks fine to me. I guess you see a reason not to have this code, could you explain?

vidartf · 2019-02-05T10:05:25Z

I did not look through this in detail, but in general, I think it would be appropriate to have a look at refactoring the lab manager in order to make it more friendly for library use. If you want to open an issue on the widgets repo outlining some of the current pain-points, that would be an excellent starting point for such a discussion.

SylvainCorlay · 2019-02-12T14:09:31Z

Hey @msuperina quick note on this: your git config email is [email protected] which does not match your GitHub email. You may want to change this so that github credits you for this work.

msuperina · 2019-02-15T14:59:37Z

@SylvainCorlay @maartenbreddels I was very busy with work and could not follow up in the past few days. Is there any work you need me to do on this PR at this stage ?

maartenbreddels · 2019-02-15T15:01:47Z

Thanks for taking this on, I think we're finished, unless Sylvain thinks otherwise. Would be good if you could test it. Also, feel free to change the log so that the email matches the github email address, now you won't get credits.

SylvainCorlay · 2019-02-15T15:15:59Z

@msuperina I am ready to get this in. Do you want to change your committer info so that your github accounts gets the proper credit?

msuperina · 2019-02-15T15:19:47Z

@SylvainCorlay no please go ahead if this is what is stopping you 😃

SylvainCorlay · 2019-02-15T15:24:51Z

If you want me to do it, I am fine as well. I am used to doing this sort of changes. Just give me the email address you want to use.

Most of the widget manager logic is duplicated from the @jupyter-widgets/jupyterlab-manager package. This PR aims at using the package directly. The main benefit of this approach is support for output of "stream" messages. It becomes possible to render and interact with interactive widgets. An example of interactive widget has been added in notebooks/interactive.ipynb Fix #19 Fix #23

SylvainCorlay · 2019-02-18T13:07:43Z

I just rebased the PR attributing Marzio's commits to his GitHub account (msuperina at users.noreply.github.com). Will merge when all is green.

Thanks @maartenbreddels for doing the work on babel7.

msuperina commented Jan 22, 2019

View reviewed changes

msuperina mentioned this pull request Jan 22, 2019

Support for @interact #19

Closed

maartenbreddels reviewed Jan 24, 2019

View reviewed changes

SylvainCorlay mentioned this pull request Feb 12, 2019

Expose JupyterLab Widget Manager jupyter-widgets/ipywidgets#2314

Merged

voila-dashboards deleted a comment from maartenbreddels Feb 13, 2019

voila-dashboards deleted a comment from msuperina Feb 13, 2019

msuperina and others added 6 commits February 18, 2019 14:05

fix: add newline end of files

75687a6

Use latest jupyterlab-manager

22f9ee0

setup babel7, compile to modern ~es6 js, and avoid slow babel-loader

e6d4504

don't use html manager object for functionality

6d7cce7

CRLF -> LF

863b297

SylvainCorlay merged commit a470a4c into voila-dashboards:master Feb 19, 2019

ibayer mentioned this pull request Nov 10, 2019

support voila for creating stand alone app ideonate/jupyter-innotater#7

Closed

sandertyu mentioned this pull request Jul 9, 2021

interact() from ipywidgets not working with Thebe LibreTexts/metalc#239

Closed

TimStewartJ mentioned this pull request Jul 13, 2021

[REFACTOR] Implement jupyterlab manager jupyter-book/thebe#418

Merged

sandertyu mentioned this pull request Jul 15, 2021

Use jupyterlab manager LibreTexts/thebe#1

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: use jupyterlab-manager output widget #46

feat: use jupyterlab-manager output widget #46

msuperina commented Jan 22, 2019

msuperina Jan 22, 2019

SylvainCorlay Feb 12, 2019

maartenbreddels Feb 15, 2019

msuperina Feb 15, 2019 •

edited

Loading

msuperina Jan 22, 2019

SylvainCorlay Feb 12, 2019

maartenbreddels Feb 15, 2019

SylvainCorlay Feb 15, 2019

msuperina Jan 22, 2019

maartenbreddels Jan 24, 2019

msuperina Feb 4, 2019

maartenbreddels Feb 15, 2019

msuperina Jan 22, 2019

maartenbreddels Jan 24, 2019

msuperina Feb 4, 2019

msuperina commented Jan 22, 2019

SylvainCorlay commented Jan 22, 2019

msuperina commented Jan 24, 2019

maartenbreddels left a comment

maartenbreddels Jan 24, 2019

maartenbreddels Jan 24, 2019

vidartf commented Feb 5, 2019

SylvainCorlay commented Feb 12, 2019

msuperina commented Feb 15, 2019

maartenbreddels commented Feb 15, 2019

SylvainCorlay commented Feb 15, 2019

msuperina commented Feb 15, 2019

SylvainCorlay commented Feb 15, 2019

SylvainCorlay commented Feb 18, 2019

feat: use jupyterlab-manager output widget #46

feat: use jupyterlab-manager output widget #46

Conversation

msuperina commented Jan 22, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

msuperina Feb 15, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

msuperina commented Jan 22, 2019

SylvainCorlay commented Jan 22, 2019

msuperina commented Jan 24, 2019

maartenbreddels left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vidartf commented Feb 5, 2019

SylvainCorlay commented Feb 12, 2019

msuperina commented Feb 15, 2019

maartenbreddels commented Feb 15, 2019

SylvainCorlay commented Feb 15, 2019

msuperina commented Feb 15, 2019

SylvainCorlay commented Feb 15, 2019

SylvainCorlay commented Feb 18, 2019

msuperina Feb 15, 2019 •

edited

Loading