search

cytoscape / ipycytoscape

A Cytoscape Jupyter widget
https://ipycytoscape.readthedocs.io/en/master/
BSD 3-Clause "New" or "Revised" License
265 stars 62 forks source link

Only build JupyterLab in postBuild #235

Closed sandertyu closed 3 years ago

sandertyu commented 3 years ago

Problem

With the extension changes to JupyterLab3, most of the contents in the current postBuild of this repository are unnecessary. This can potentially be confusing for users seeking to replicate an environment like this one.

Proposed Solution

I propose that the contents of postBuild are changed to simply jupyter lab build --dev-build=False. This could reduce potential errors in the future as JupyterLab continues to change and it improves clarity by removing redundant and unnecessary code. I have a branch which can be built with binder to test the postBuild changes.

I was originally going to suggest removing postBuild entirely, but the jupyter-offflinenotebook extension got very mad and throws an intrusive error on JupyterLab if you don't do jupyter lab build after installation. This extension comes from repo2docker which is still using JupyterLab2 and they install it with --no-build.

You also see this message when building without postBuild; no-postBuild

I tested the image using the branch linked above with this notebook. Everything works properly as far as I can tell.

Here is the labextension list in binder using the linked image; binder-extension-list

Here is the labextension list in a JupyterLab deployment using the linked image; jlab-extension-list

Additional context

Really this is a decision that is up to the maintainers of the project. It's difficult to keep up with all the changes made to JupyterLab. I can make a PR from my forked branch if you all would like to make this postBuild change.

marimeireles commented 3 years ago

Hey @sandertyu, thank you for opening this issue. I don't see a problem with adding your changes to the postBuild, the code that's there doesn't even work anymore afaic, because it's made to run with jupyterlab2. My only problem is that I couldn't run your local branch for some reason! I used: https://mybinder.org/v2/gh/LibreTexts/ipycytoscape/master?filepath=examples But if you want to open a PR, I'll be happy to merge.

marimeireles commented 3 years ago

Okay :) magically worked now, for me. Are you doing this because the current postbuild file is failing for you somehow or because it's wrong?

sandertyu commented 3 years ago

Sorry for the late response. The code there does not cause any explicit problems so far as I know, but it is at best superfluous, and could potentially cause bugs or version conflicts later on. Additionally, jupyter-offlinenotebook (installed through repo2docker and unavoidable) suggests adding jupyter lab build --dev-build=False anyways, as shown in the first screenshot, so that should be included regardless.

My branch is still available to merge, and I've created a pull request for it if you would like to do so.

marimeireles commented 3 years ago

Is this fixed @sandertyu ? Thanks!

sandertyu commented 3 years ago

Is this fixed @sandertyu ? Thanks!

Yes I believe so. postBuild was removed as it was unnecessary. I'll close the issue now.