-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
[pydeck] Create a prebuilt extension for JupyterLab #7030
base: master
Are you sure you want to change the base?
Conversation
Note that this ties it to 8.8.0-alpha.6; see visgl#7025 for comments
bump_version.py generates these files so could be updated
The main thing is modifying package.json and webpack.config.js to work with the jupyterlab builder. Since the previous build was just using webpack, we let the builder integrate the webpack config from that build with the labextension webpack. Required some changes to remove the original dist directory so everything just goes to a pydeck/labextension directory now.
iirc this is only valid for specific jupyter versions. Only for |
@@ -1 +1 @@ | |||
__version__ = "0.7.1" | |||
__version__ = "0.8.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: I'd bump the version in a follow-up PR
"setuptools", | ||
"wheel", | ||
] | ||
requires = ["jupyter_packaging~=0.10,<2", "jupyterlab~=3.1"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you include jupyterlab
as a build dependency? The jupyter-packaging docs don't seem to require it: https://github.com/jupyter/jupyter-packaging/#as-a-build-backend
|
||
[tool.black] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why delete the black config section?
|
||
```bash | ||
npm login | ||
npm publish --access public |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: we should be able to put --access public
in the package.json I think so we don't need to specify it manually. Ref
https://docs.npmjs.com/cli/v8/configuring-npm/package-json#publishconfig and https://docs.npmjs.com/cli/v8/using-npm/config#access
python -m build | ||
``` | ||
|
||
> `python setup.py sdist bdist_wheel` is deprecated and will not work for this package. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you update the Makefile
with the new commands as well?
|
||
[tool.jupyter-packaging.build-args] | ||
build_cmd = "build:prod" | ||
source_dir = "../../modules/jupyter-widget" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This path doesn't make sense here or at least is unnecessary
build_dir = "pydeck/labextension" | ||
npm = ["jlpm"] | ||
|
||
[tool.check-manifest] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The check-manifest
config is defined here but it seems there's instructions on how to use it.
factory = "jupyter_packaging.npm_builder" | ||
|
||
[tool.jupyter-packaging.build-args] | ||
build_cmd = "build:labextension" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This only builds the labextension?
|
||
logging.basicConfig(format="%(levelname)s: %(message)s") | ||
logging.warning( | ||
"Build tool `jupyter-packaging` is missing. Install it with pip or conda." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not clear to me why this should be a warning not an error. It seems like it could lead to hard-to-debug issues where the extension isn't updating as expected. Additionally, I would expect that jupyter-packaging
is always installed given that it's defined in build-requirements
in the pyproject.toml
.
author_email=pkg_json["author"]["email"], | ||
description=pkg_json["description"], | ||
license=pkg_json["license"], | ||
license_file="LICENSE.txt", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that in this PR you're now defining more of the build process in pyproject.toml
, move the non-dynamic parts of the setuptools config there too? https://setuptools.pypa.io/en/latest/userguide/pyproject_config.html
@@ -20,7 +20,6 @@ | |||
"prettier: 2.3.2 breaks on `import type`, remove when ocular bumps" | |||
], | |||
"resolutions": { | |||
"acorn": "7.4.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @Pessimistress on this. I don't know why it was previously pinned
@@ -0,0 +1 @@ | |||
import './base.css'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this folder used? Seems like it just imports an empty CSS file.
Correct, although even notebook will now be built on jupyterlab infrastructure (ref). I think you could keep the other method (current install process) working for the other modes, but these would have to be tested. JupyterLab developers are aware of some difficulties for users finding and installing the new prebuilt extensions: jupyterlab/jupyterlab#10554 Perhaps a better approach in structuring the code would be to structure the jupyter-widget components as currently done, and create a separate build process for the JupyterLab 3 plugin so that different dependency versions (specific to JupyterLab's build/plugin process) could be integrated without impacting the deck.gl's current dependencies. I didn't end up going that route because I couldn't figure out what was going on with the original approach that just included |
@dakoop do you plan on continuing that work? It would be awesome. |
My impression (not a regular Jupyter user!) is that nbextensions for Jupyter are dying a slow death at this point, and with Jupyter Notebooks v7+ (released July 2023), you'll get errors like ...
... for which old solutions will no longer work. Likely a prebuilt extension for JupyterLab is becoming necessary. Are there other alternatives we should consider, or anything I can do to help land this PR? Otherwise, I think pydeck may be stuck in Jupyter Notebook 6.4 or lower, or nbclassic. |
I think the main difference is that Jupyter Notebook v7 is built upon JupyterLab architecture, but in a way that looks like the classic notebook. So I think you need to make JupyterLab extensions to support the latest notebook |
As @kylebarron noted, Notebook 7 introduces extension changes as it moved to lab architecture. This PR was a push to update the pydeck extension a while back, but I haven't had time to reexamine this. There is other work on deck.gl Jupyter integration: lonboard and ipydeck. |
0a083b6
to
eb85da3
Compare
At this point, maybe it could make sense to consider building the pydeck widget on top of anywidget? This could help reduce maintenance, and would make |
Oh, I hadn't noticed, thanks @kylebarron! I was curious if But it's good to see that |
I also have an example with Lonboard in JupyterLite rendering 3.2 million points: https://jupyterlite.ds.io/lab/index.html?path=lonboard_example%2Fdata-filter-extension.ipynb |
pydeck does work via JupyterLite!
https://duberste.in/pydeck-in-pyodide/lab/index.html
…On Fri, 13 Dec 2024 at 9:40 am, Kyle Barron ***@***.***> wrote:
I also have an example with Lonboard in JupyterLite rendering 3.2 million
points:
https://jupyterlite.ds.io/lab/index.html?path=lonboard_example%2Fdata-filter-extension.ipynb
image.png (view on web)
<https://github.com/user-attachments/assets/4d53b398-f29f-4ea7-bd51-0581a20519c6>
—
Reply to this email directly, view it on GitHub
<#7030 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAQ2IVNCOVOAWWMRYBNGUF32FMLXLAVCNFSM6AAAAABTOMWJZWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKNBRHE2DAMJQGM>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
Just some context: I can confirm that the deck.gl TSC (technical steering committee) is talking about the value of a refresh of pydeck - and is strongly in favor of adopting anywidget in pydeck. It is not a lack of faith in the value of pydeck, it is mainly an issue of (Python capable) maintainers not being available.
All of this leaves pydeck with very interesting options for the future, but unfortunately without a very clear immediate path forward (though happy to see original author @adjuberstein chiming in!) |
For #6989
The general idea is to create a prebuilt extension that can be distributed as part of the pydeck python package. There is no need for a separate
@deck.gl/jupyter-widget
install as it is bundled in the pydeck python package, and the extension does not require jupyterlab to be rebuilt as it uses webpack 5's module federation. Just runpip install pydeck
. This is WIP as it doesn't currently check for compatibility with classic notebook support, etc. To build,yarn bootstrap
python -m build
inbindings/pydeck
a. If this doesn't work, try
jlpm build
inmodules/jupyter-widget
b. This should create a wheel in
dist
pip install dist/*.whl
Change List
@jupyterlab/builder
and webpack 5 into the jupyter-widget build process. This is a one-step build now as thejupyter labextension build
uses webpack to create a federated module (no separatedist/index.js
)package.json
. Some part of the new build process requires a newer version of acorn, as the build just hangs otherwise. I am not sure what the effect is on other parts of deck.gl.pydeck/labextension
inbindings/pydeck
so it can be distributed as part of the python wheel.python -m build
method for building pydeck and the labextensionTodo
jupyter labextension develop .
(something with pyproject.toml?)