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

Inlined build for perspective-jupyterlab, improve PerspectiveWidget #1466

Merged
merged 7 commits into from
Jul 8, 2021

Conversation

sc1f
Copy link
Contributor

@sc1f sc1f commented Jun 30, 2021

This PR changes the perspective-jupyterlab build to inline its external dependencies on the main Perspective modules, and makes some long-overdue improvements to PerspectiveWidget in order to enhance the user experience in Jupyterlab and Voila. Specifically:

  • Fixes Perspective 0.7.0 and above does not work in Voila #1454, which was due to Voila not being able to access perspective-jupyterlab's external dependencies on the main Perspective modules. While the asset produced (10.6MB as loaded into a Jupyterlab production build) is not ideal, it is guaranteed to work in both Jupyterlab and Voila without additional hard-to-maintain/brittle build cruft.
  • Fixes long-running bug where PerspectiveWidget had a height of 0 when rendered inside Voila. The height now defaults to 520px, the same as in Jupyterlab.
  • The <perspective-viewer> config panel now shows by default on all widgets when rendered.
  • worker.terminate() and viewer.delete() is now called whenever the widget output is disposed of, which should make memory usage in Jupyterlab more efficient.
  • Fixes Format height for PerspectiveWidget #1462: PerspectiveWidget now inherits from ipywidgets.DOMWidget instead of ipywidgets.Widget, which means that layout should work out of the box:

widget_layout

  • plugin_config can now be passed in from Python, which should allow for much finer-grained control over styling of the datagrid and chart plugins. However, because plugin_config cannot be read synchronously from the Javascript front-end (as it is not part of the attribute API and there is no async get in Javascript), there is currently no way to read the current plugin_config from the widget unless it has been set by the user already. However, this is still very useful for developers to "pre-set" styles on PerspectiveWidget:

plugin_config

  • Modified WebSocketServer to strip out semver-like ranges from requests. This is helpful when debugging Voila locally, as I can just set the Voila CDN to localhost:8080/node_modules/ and all the CDN requests Voila makes will automatically be resolved against my local build.

@sc1f sc1f added enhancement Feature requests or improvements Jupyter labels Jun 30, 2021
@sc1f sc1f changed the title PerspectiveWidget quality of life improvements Inlined build for perspective-jupyterlab, improve PerspectiveWidget Jul 1, 2021
@sc1f sc1f marked this pull request as ready for review July 1, 2021 13:26
@@ -27,7 +27,7 @@ module.exports = {
maxEntrypointSize: 512000,
maxAssetSize: 512000
},
externals: [/^([a-z0-9]|@(?!finos\/perspective-viewer))/],
externals: [/\@jupyter|\@lumino/],
Copy link
Member

Choose a reason for hiding this comment

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

Should this include jupyter widgets external?

@dhirschfeld
Copy link
Contributor

Super keen to get this released so I can get my hands on the voila fixes! 🚀

@texodus
Copy link
Member

texodus commented Jul 8, 2021

Thanks for the PR! A couple post-review notes:

  • There is a non-zero chance this change does not actually fix Voila, as we can't fully test the interaction with CDNs and Perspective's multiple-asset build has had CDN issues in the past.
  • I still do not fully understand much of JupyterLab/Voila packaging, e.g. why @finos/perspective needs to be inlined, or conversely why @juypter-widgets does not. This combined with minimal testing (though much improved on the JupyterLab side since Add Jupyterlab tests to CI #1460) is a recipe for future regressions here.

@dhirschfeld
Copy link
Contributor

Unfortunately I can't help with the development or packaging but I'm happy to test the JupyterLab/Voila combo on our deployment and report back any issues...

@texodus texodus merged commit 2a31bc3 into master Jul 8, 2021
@texodus texodus deleted the fix-voila branch July 8, 2021 04:25
@sc1f
Copy link
Contributor Author

sc1f commented Jul 8, 2021

@dhirschfeld @texodus

I have just tested the 0.10.0 release in a fresh virtualenv with perspective-python installed from wheel, @finos/perspective-jupyterlab installed from NPM, and everything works. Voila falls back to JsDelivr, and loads the plugin and all externals correctly with or without --enable_nbextensions=True set when running Voila.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature requests or improvements Jupyter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Perspective 0.7.0 and above does not work in Voila
4 participants