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

Drop notebook 6 #4822

Open
wants to merge 17 commits into
base: anywidget
Choose a base branch
from
Open

Drop notebook 6 #4822

wants to merge 17 commits into from

Conversation

marthacryan
Copy link
Collaborator

@marthacryan marthacryan commented Oct 22, 2024

Note: this PR is on top of the changes from #4823 because the plotly.js version used in plotly.py is based on the jupyterlab extension code, and we needed a new place to store the plotly.js version. The widget seemed like a good place because all of the other javascript is removed in this PR.

Fixes #4779.

It appears that the entire packages/javascript/jupyterlab_plotly directory contains code that is either used to support very old versions of jupyter lab/notebook/voila, or is not used at all. This PR removes the entire directory in favor of having a very small packages/python/plotly/js directory that contains only the widget code (see #4663 for adding the widget code into that directory).

The last time that the jupyter extension was significantly changed was 3 years ago in #3142. @fcollonval described the files in the packages/javascript/jupyterlab_plotly directory in this comment. He did add support for federated extensions (new jupyter lab/notebook build), but I wasn't able to find evidence that having an extension installed in jupyterlab >=3 or jupyter notebook >= 7 is even necessary for plotly. @fcollonval if you know of any reason that that isn't true, please let me know!

To test:

  • Checkout branch
  • Create a new python env (conda, mamba, pipenv, etc)
  • git clean -f -x -d
  • cd packages/python/plotly
  • pip install -e .
  • pip install jupyter
  • Try it out in jupyter lab: jupyter lab
  • Try it out in jupyter notebook: jupyter notebook

@fcollonval
Copy link
Contributor

I wasn't able to find evidence that having an extension installed in jupyterlab >=3 or jupyter notebook >= 7 is even necessary for plotly

This is correct because the produced output is using html that will includes plotly.js the first time:

class NotebookRenderer(HtmlRenderer):

Dropping the mimerenderer factory will prevent plotly.json file to be opened in JupyterLab.

@marthacryan
Copy link
Collaborator Author

@fcollonval Thank you for the response! Also hi!

@marthacryan marthacryan marked this pull request as ready for review October 23, 2024 21:59
@gvwilson gvwilson added feature something new P1 needed for current cycle labels Oct 24, 2024
@archmoj
Copy link
Contributor

archmoj commented Oct 24, 2024

This is looking good to me.
@marthacryan Would you please add a changelog entry?

@LiamConnors
Copy link
Member

@marthacryan, testing the FigureWidget in Notebook 7 and I get a JavaScript error:

Code is

import plotly.graph_objects as go

f = go.FigureWidget()
f

image

I have anywidget=0.9.13

@marthacryan
Copy link
Collaborator Author

@LiamConnors Looks like this wasn't happening for me because I hadn't added the npm install && npm run build step into pip install -e ., so the necessary javascript wasn't generated! I'll look into adding that

@LiamConnors
Copy link
Member

LiamConnors commented Oct 24, 2024

In VS Code, I see this:

image

Not really sure what to make of it. I'm using the same environment as in a notebook launched in the browser.

I've commented here as I'm testing off this branch, but I guess this may be more related to #4823

Comment on lines +116 to +118
### Jupyter Widget Support

For use in the Jupyter Notebook, install the `notebook` and `ipywidgets`
For use as a Jupyter widget, install `jupyter` and `anywidget`
Copy link
Member

Choose a reason for hiding this comment

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

The section above this talks about "JupyterLab Support" and should now mention anywidget. Wondering if this section should stay as "Jupyter Notebook Support"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature something new P1 needed for current cycle
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants