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

Exclude browser asset dependencies in emscripten #3834

Closed

Conversation

bollwyvl
Copy link
Contributor

@bollwyvl bollwyvl commented Sep 16, 2023

Elevator Pitch

Don't require jupyterlab_widgets or widgetsnbextension when already in a browser environment, such as JupyteLite.

Changes

  • add ; platform_system != "Emscripten" to browser dependencies in ipywidgets' setup.cfg
  • update .readthedocs.yaml
    • update jupyterlite dependencies
    • use updated mambaforge
    • pre-build assets for more inspectable logs
      • errors will appear in dedicated steps, rather than in the monster sphinx call

Motivation

Both jupyterlite-pyodide-kernel and jupyterlite-xeus-python-kernel report platform.system() as Emscripten. There may be other in-the-wild uses, but these are the two primary ones I know of that implement enough of the Jupyter architecture to support widgets.

These assets are relatively large, and in the end, don't really do anything... in fact, in pyodide's micropip, they don't even go anywhere, as data_files are not respected. Further, when ipywidgets updates its asset dependencies (which is generally very good for users) we have to scramble to deal with stuff downstream.

Particuarly for jupyterlite-pyodide-kernel, this would avoid having to deploy patches for widgetsnbextension, as the notebook dependency can't really be fulfilled (e.g. pyzmq).

Test Procedure

Once the RTD job fires, on opening one of the preview notebooks with the network tab open:

  • run all cells
  • see jupyterlab_widgets and widgetsnbextension wheels are not be loaded
  • import jupyterlab_widgets, widgetsnbextension should fail with ModuleNotFoundError

Future Work

Unbundling **/tests/** from the ipywidgets wheel would take it from 137K down to 73K. Every little bit counts!

While there has been some pushback on this on e.g. ipython, whereas a large number of Jupyter projects (jupyter_server, etc) now only ship tests in sdist packages, which is good for downstreams.

@github-actions
Copy link

Binder 👈 Launch a binder notebook on branch bollwyvl/ipywidgets/no-asset-deps-in-emscripten

@bollwyvl bollwyvl changed the title Exclude browser asset depenedncies in emscripten Exclude browser asset dependencies in emscripten Sep 16, 2023
@maartenbreddels
Copy link
Member

Hi Nick,

I'm not in favor of this because this is only for Jupyter Lite. I know of an emscripten solution that actually does require this, which means this PR will break it.

Regards,

Maarten

@bollwyvl
Copy link
Contributor Author

an emscripten solution

Great! Where is it? Let's find a way to make everyone happy.

The relevant marker env is easiest to pull off packaging:

from packaging.markers import default_environment
default_environment()
  • jupyterlite-pyodide-kernel
{'implementation_name': 'cpython',
 'implementation_version': '3.11.3',
 'os_name': 'posix',
 'platform_machine': 'wasm32',
 'platform_release': '3.1.45',
 'platform_system': 'Emscripten',
 'platform_version': '#1',
 'python_full_version': '3.11.3',
 'platform_python_implementation': 'CPython',
 'python_version': '3.11',
 'sys_platform': 'emscripten'}
  • jupyterlite-xeus-python
{'implementation_name': 'cpython',
 'implementation_version': '3.10.2',
 'os_name': 'posix',
 'platform_machine': 'wasm32',
 'platform_release': '3.1.27',
 'platform_system': 'Emscripten',
 'platform_version': '#1',
 'python_full_version': '3.10.2',
 'platform_python_implementation': 'CPython',
 'python_version': '3.10',
 'sys_platform': 'emscripten'}

There's a lot of siloed work going on which leads to the broader python community not taking the WASM-based work seriously.

@maartenbreddels
Copy link
Member

There is nothing out public yet.

But I think what you want fits into a more general solution, which is to split up ipywidgets into

  • ipywidgets (a meta package with pinned dependencies to the below packages for backwards compatibility)
  • ipywidgets-python/core/.. (just the python code, no assets)
  • jupyterlab_widgets
  • widgetsnbextension

This means that kernels, colab, vscode and jupyter lite install ipywidgets-python (or ipywidgets-core). Jupyter servers environments install the jupyterlab_widgets or widgetsnbextension.

Regular users install ipywidgets, and hope Things Just Work.

What do you think?

@bollwyvl
Copy link
Contributor Author

Yeah, sounds like something to hash out framed in a larger discussion (and a synchronized major version bump). Keeping all the dependency managers happy without intersecting features ([extras] and run_constrained), it's hard to imagine something that wouldn't create more headaches to keep in sync between clients.

@bollwyvl bollwyvl closed this Sep 17, 2023
@maartenbreddels
Copy link
Member

In vaex I've tried keeping all subpackage loosly pinned (semver rules), which was a headache. In solara we decided to keep the subpackages versions completely in sync and pin them to avoid maintenance burden. (e.g. ipywidgets 8.3.4 would require on ipywidgets-core 8.3.4 and they both would be simultaneously released, and also the conda-forge feedstock would be 1 with multi output).

It gives a bit of an overhead in storage for pypi and conda-forge, but with small packages like ipywidgets I think it's ok, especially since we don't release that often.

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.

2 participants