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

Add requirements.txt and postBuild for Binder #2701

Merged
merged 4 commits into from
Mar 12, 2021

Conversation

mgeier
Copy link
Contributor

@mgeier mgeier commented Jan 8, 2020

This is an alternative to #2659 which also tries to fix #2301.

Closes #2659

Using jupyterlab==1 in requirements.txt I can avoid the error from #2659 and the Binder image finally finishes building and is actually launched.

The problem, however, is that it still doesn't work. The widgets are not displayed in JupyterLab (but they work fine in the Classic Notebook).

Here's the Binder link: https://mybinder.org/v2/gh/mgeier/ipywidgets/binder-requirements?urlpath=lab/tree/docs/source/examples

@jtpio
Copy link
Member

jtpio commented Jan 9, 2020

The problem, however, is that it still doesn't work. The widgets are not displayed in JupyterLab (but they work fine in the Classic Notebook).

Probably because environment.yml specifies JupyterLab 1.0, and the dev_install.sh script targets JupyterLab 2.0?

@mgeier
Copy link
Contributor Author

mgeier commented Jan 9, 2020

Does that mean that a dev install of ipywidgets is not possible with the latest released version of JupyterLab? Does it also require a dev install of JupyterLab?

@mgeier mgeier mentioned this pull request Jan 9, 2020
@pbugnion
Copy link
Member

pbugnion commented Jan 9, 2020

Does that mean that a dev install of ipywidgets is not possible with the latest released version of JupyterLab? Does it also require a dev install of JupyterLab?

Yes, since a few days ago.

Does it also require a dev install of JupyterLab?

No, it requires installing a JupyterLab 2 beta. See the developer documentation for instance.

@mgeier mgeier force-pushed the binder-requirements branch 2 times, most recently from 7f0051a to a28bf55 Compare January 9, 2020 14:02
@jasongrout
Copy link
Member

jasongrout commented Jan 9, 2020

Does that mean that a dev install of ipywidgets is not possible with the latest released version of JupyterLab? Does it also require a dev install of JupyterLab?

Yes, since a few days ago.

Clarification, since you asked two questions: ipywidgets master requires the jlab 2.0.0 beta 2, not a dev install of JuptyerLab. In other words, the documentation @pbugnion should work.

@mgeier
Copy link
Contributor Author

mgeier commented Jan 9, 2020

@mgeier mgeier marked this pull request as ready for review January 9, 2020 14:45
@jtpio jtpio mentioned this pull request Jan 9, 2020
@pbugnion
Copy link
Member

pbugnion commented Jan 9, 2020

@mgeier Thanks for pushing this through.

I think we should still clarify whether we really want a dev-install, as started in #2659.

I think this depends on what we think the point of the binder link is. Is it:

  1. an easy way for users to test ipywidgets locally without having to install it? (judging by the number of issues, users struggle with installation).
  2. a way to verify current master works with a particular feature.

If it's (1), then I think a dev-install is detrimental, and we should always install the built, stable version. The problem with a dev-install is that it takes 10 minutes to start if the layer containing the node_modules isn't cached. It's also much more likely to break than pulling in pre-built JS dependencies.

I think we very much want to target (1) in the link in the README.

One possibility could be to

  1. merge this as is, doing a dev-install (modulo a proper review etc.)
  2. create a second repository just for binder that installs a stable version of ipywidgets, widgetsnbextension and the jupyterlab manager off pypi or conda-forge and clones the examples gallery. We could then link to this second repository in the README.

This would cater to both use cases.

@mgeier I'd be keen to hear your thoughts as well as @jtpio and other maintainers.

@mgeier
Copy link
Contributor Author

mgeier commented Jan 10, 2020

The problem with a dev-install is that it takes 10 minutes to start if the layer containing the node_modules isn't cached.

How often would that happen?
Is there a way to make this less likely?

It's also much more likely to break than pulling in pre-built JS dependencies.

Do you mean that it could break between two different commits or between to builds of the same commit?

@mgeier
Copy link
Contributor Author

mgeier commented Jan 28, 2020

@pbugnion Do you have answers to my questions?

I think it would be ideal if both use cases you mentioned could be covered by the same setup.

If there are issues with that, it would be great if we could solve them.

If that turns out to be impossible (or at least no feasible), we should consider implementing the two use cases separately.

@jbpauly
Copy link

jbpauly commented Mar 31, 2020

@mgeier thanks for bringing up this issue. I'm trying to understand which solution worked for you. Did you end up using jlab 2.0.0 beta 2, not a dev install of JuptyerLab?

Plotly is new to me, and I brought up the same issue to the Plotly Community: https://community.plotly.com/t/getting-started-with-plotly-widgets-and-jupyterlabs-conflicting-dependencies/36974

@jasongrout
Copy link
Member

jasongrout commented Mar 31, 2020

The answer to your question at https://community.plotly.com/t/getting-started-with-plotly-widgets-and-jupyterlabs-conflicting-dependencies/36974 is to just install the extension with:

jupyter labextension install @jupyter-widgets/jupyterlab-manager

It will figure out the right version to get and install.

You saw a problem since you said specifically to install version 1.1 (that's the @1.1 you had in your command), but that's the wrong version for JupyterLab 2.0, which is what you have installed. Instead, use the above command and JupyterLab will find, fetch, and install the correct version.

@jbpauly
Copy link

jbpauly commented Mar 31, 2020

@jasongrout thanks for the quick reply! I realized the mistake after reading your response and reading the README https://github.com/jupyter-widgets/ipywidgets/tree/master/packages/jupyterlab-manager

I tried to reinstall using:

# Avoid "JavaScript heap out of memory" errors during extension installation
# (OS X/Linux)
export NODE_OPTIONS=--max-old-space-size=4096

# Jupyter widgets extension
jupyter labextension install @jupyter-widgets/jupyterlab-manager@2 --no-build

# FigureWidget support
jupyter labextension install [email protected] --no-build

# and jupyterlab renderer support
jupyter labextension install [email protected] --no-build

This worked all the way up until the jupyter lab build call

# Build extensions (must be done to activate extensions since --no-build is used above)
jupyter lab build

I also tried your recommendation

jupyter labextension install @jupyter-widgets/jupyterlab-manager
Both attempts results in an error. Looking at the log, it looks like I need to update Node. I will attempt to update Node and reinstall the proper extensions.

Thanks!

@mgeier
Copy link
Contributor Author

mgeier commented Mar 31, 2020

@jbpauly

I'm trying to understand which solution worked for you.

The current version of this PR (a28bf55) works on binder (at least it worked at that time), see the links in #2701 (comment).

Did you end up using jlab 2.0.0 beta 2, not a dev install of JuptyerLab?

Yes, exactly.
I used jupyterlab>=2.0.0b2 in requirements.txt.

@jasongrout
Copy link
Member

Likely you can use the actual jlab 2.0 final release at this point.

@jbpauly
Copy link

jbpauly commented Apr 3, 2020

@jasongrout and @mgeier, thanks for the responses.

If either are curious, installing manager@2
jupyter labextension install @jupyter-widgets/jupyterlab-manager@2 --no-build
does work.

@mgeier
Copy link
Contributor Author

mgeier commented May 5, 2020

I've re-based this PR and I've changed the JupyterLab dependency to >=2 as suggested by @jasongrout.

It still works: https://mybinder.org/v2/gh/mgeier/ipywidgets/binder-requirements?urlpath=lab/tree/docs/source/examples

@ianhi ianhi linked an issue Feb 16, 2021 that may be closed by this pull request
@jtpio
Copy link
Member

jtpio commented Mar 12, 2021

Just rebased onto the latest master.

@jtpio
Copy link
Member

jtpio commented Mar 12, 2021

Now that we have a dev Binder bot to easily test new PRs (#3148), it looks like it would make sense to setup Binder to install from source with the ./dev-install.sh script (what this PR does).

For example to test this branch:

Binder

Based on the discussion above, it sounds like it would still make sense to also provide a "stable" Binder that would install ipywidgets from conda forge or PyPI. As an example the jupyterlab repo Binder badge points to https://github.com/jupyterlab/jupyterlab-demo which uses released packages, and also has the Binder link setup on PRs to test changes in dev mode.

cc @ianhi

@jtpio
Copy link
Member

jtpio commented Mar 12, 2021

So as a conclusion I would be in favor or merging this PR as is, at least as a first step.

This would make it very convenient to try out changes for new PRs without having to manually check out the code locally. And fix #3152

@jtpio jtpio linked an issue Mar 12, 2021 that may be closed by this pull request
.binder/requirements.txt Outdated Show resolved Hide resolved
@ianhi
Copy link
Contributor

ianhi commented Mar 12, 2021

So as a conclusion I would be in favor or merging this PR as is, at least as a first step.

Makes sense to me!

Copy link
Contributor

@ianhi ianhi left a comment

Choose a reason for hiding this comment

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

Thanks @mgeier and @jtpio

@ianhi ianhi merged commit f085739 into jupyter-widgets:master Mar 12, 2021
@ianhi ianhi mentioned this pull request Mar 12, 2021
@mgeier mgeier deleted the binder-requirements branch March 13, 2021 09:09
@ianhi ianhi mentioned this pull request Apr 1, 2021
@jasongrout jasongrout added this to the 8.0 milestone Dec 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants