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

Fix WebGL plotting in Jupyter Lab #859

Closed
wants to merge 2 commits into from
Closed

Conversation

astrofrog
Copy link
Contributor

This declares raw-loader as a runtime dependency since it is needed to load some of the shader code (specifically for the scales).

If we don't specify this, we are dependent on the default versions of raw-loader in notebook and lab which might be different - in particular, recent versions of raw-loader used by Jupyter Lab don't return a string when used with require() but instead return an object with a 'default' property, which causes issues in three.js.

Fixes #858

@jasongrout
Copy link
Member

What issues are caused? Can we check if the import has a default attribute?

@astrofrog
Copy link
Contributor Author

astrofrog commented May 31, 2019

@jasongrout in this statement:

THREE.ShaderChunk['scales'] = require('raw-loader!../shaders/scales.glsl')

old versions of raw-loader return a string, new versions return an object with a default property (this then caused threejs to fail because it expects values in ShaderChunk to be strings). I could just check here which is the case and adjust automatically, which would mean the raw-loader version doesn't matter. Would you prefer that fix? In any case, raw-loader should be listed as a runtime dependency though.

@maartenbreddels
Copy link
Member

In any case, raw-loader should be listed as a runtime dependency though.

I don't think so, the loader is only needed when building the bundles right, or did something change with raw-loader 2? I don't think the raw-loader should end up in the bundle, or am I getting the concepts of runtime/dev dependency wrong?

@astrofrog
Copy link
Contributor Author

astrofrog commented May 31, 2019

I may have used the wrong term, but I mean that raw-loader definitely needs to be mentioned in the dependencies section (not just devDependencies) otherwise we are at the mercy of whatever version Jupyter Lab wants to use (which is what was causing the issue here).

@astrofrog
Copy link
Contributor Author

I've updated the approach here, but I'm seeing the following error today when trying to install the Jupyter Lab extension:

> bqplot@0.5.0-alpha.0 build /home/tom/Code/bqplot/js
> tsc && webpack && lessc less/bqplot.less css/bqplot.css

src/Mark.ts:103:58 - error TS2345: Argument of type 'unknown' is not assignable to parameter of type 'WidgetModel'.

103             scale_promises[key] = that.create_child_view(model);
                                                             ~~~~~

src/MarketMap.ts:333:58 - error TS2345: Argument of type 'unknown' is not assignable to parameter of type 'WidgetModel'.
  Type '{}' is missing the following properties from type 'WidgetModel': defaults, isNew, initialize, comm_live, and 78 more.

333             scale_promises[key] = that.create_child_view(model);
                                                             ~~~~~


Found 2 errors.

This seems to be unrelated to the changes I made as it happens even on master. Any idea what that could be due to?

@jasongrout
Copy link
Member

The code changes in this PR look good to me (caveat: I haven't tested).

FYI, you could do scales.default instead of scales['default'], which is visually complex.

@jasongrout
Copy link
Member

This seems to be unrelated to the changes I made as it happens even on master. Any idea what that could be due to?

Possibly due to the recent release of the jlab widget manager and associated packages. I'll check.

@jasongrout
Copy link
Member

I'm seeing the following error today when trying to install the Jupyter Lab extension:

What commands are you using to do this? And is it on bqplot master? What is your jlab version?

@astrofrog
Copy link
Contributor Author

I have Jupyter Lab 1.0.0a5 and I am running jupyter labextension install . inside the js directory. I see this on bqplot master (as well as this branch).

@astrofrog
Copy link
Contributor Author

Weirdly it worked yesterday, so something happened in the last 24h...

@jasongrout
Copy link
Member

I published a new version of the jlab manager (0.41) yesterday that supports jlab 1a5. If you're using jlab 1a3, you'll need to use jlab manager 0.40.

@astrofrog
Copy link
Contributor Author

I just installed jupyterlab-manager 0.41 since I have Jupyter Lab 1.0.0a5 but it's still crashing during the build:

$ jupyter labextension list
JupyterLab v1.0.0a5
Known labextensions:
   app dir: /home/tom/python/debug-bqplot/share/jupyter/lab
        @jupyter-widgets/jupyterlab-manager v0.41.0  enabled  OK

@jasongrout
Copy link
Member

This works great for me:

conda create -y -n tmp-bqplot859 notebook 'tornado<6' ipywidgets
conda activate tmp-bqplot859
pip install --pre jupyterlab
pip install -ve /path/to/bqplot/repo
jupyter labextension install @jupyter-widgets/jupyterlab-manager
jupyter labextension install /path/to/bqplot/repo/js

Given the alpha was just released, and also a new version of the jlab widget manager to support the alpha, perhaps there are some old dependencies somewhere in your chain? I'd try cleaning things out and reinstalling.

@astrofrog
Copy link
Contributor Author

I ran into the same issue on another computer but indeed the failure is gone after setting up a new environment, so false alert.

@astrofrog
Copy link
Contributor Author

Just confirming that this PR works and fixes bqplot in Jupyter Lab

@stonebig
Copy link

it would be nice to have a bqplot update with this for Jupterlab-1.0.0 (release today ?)

@agoose77
Copy link
Contributor

agoose77 commented Jul 8, 2019

Any updates on this? Can I do anything to help move it along?

… some of the shader code.

If we don't specify this, we are dependent on the default versions of 
raw-loader in notebook and lab which might be different - in particular, 
recent versions of raw-loader don't return a string when used with 
require() but instead return an object with a 'default' property.
…w-loader and can deal with it returning an object or a string
@astrofrog
Copy link
Contributor Author

I've rebased this and it looks like the CI is passing now. This is ready for a final review/merging.

@maartenbreddels
Copy link
Member

The breaking change is described here: webpack-contrib/raw-loader#69

Basically, the advice is to use (for raw-loader >=2):

import str from '!!raw-loader!./string.txt'

If bqplot moves to raw-loader 2, we'll use version >= 2 during the normal bundle building and Jupyter lab's bundle building. I'll test that out now and see if that works.

maartenbreddels added a commit to maartenbreddels/bqplot that referenced this pull request Jul 18, 2019
…he shaders the same

This will allow bqplot to use WebGL within Jupyter lab

This replaces bqplot#859
@maartenbreddels
Copy link
Member

I think #881 is a cleaner solution, I'll also use that for ipyvolume.

@martinRenou
Copy link
Member

This PR can be closed, it has been fixed by #881

@astrofrog astrofrog closed this Sep 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issue with bqplot 0.12.0a1 in Jupyter Lab
6 participants