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

Update Jupyter extension #498

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

halleysfifthinc
Copy link
Contributor

@halleysfifthinc halleysfifthinc commented Oct 31, 2022

Highlights:

Summary of fixes/changes:

  • Updates to the server and lab extensions for API updates in JupyterLab v3 and v4 (commits 6662f46...6c761da)
  • Updates to the extension build system (commits cdc664b and 615f585...ef21134)
    • JS dependency updates
    • Switch python build system to hatchling, the currently recommended builder for Jupyter extensions
    • Added CI to build the extension
  • Fixes broken interactivity in Jupyter (commits ef21134...0556c09)
    • Julia <=> Javascript interaction uses a comm channel in Jupyter. There were 2 situations where a bad comm channel broke interactivity:
      • Opening a notebook with saved WebIO content. Jupyter tries to render the WebIO content which triggers a premature connection (and opens a comm channel) before WebIO has been loaded in Julia. When WebIO is finally loaded in Julia, the WebIO defined IJulia.CommManager.register_comm method is not called, so WebIO never gets the messages on the webio_comm channel.
        • Solution: delete any existing webio_comm channels in IJulia when WebIO is loaded.
      • Restarting a kernel. The kernel id doesn't change when restarting a kernel, so the labextension doesn't know that the kernel is new and the existing comm channel is no longer connected at one end.
        • Solution: Attach a callback to the kernel status to close comms and force reconnect if the kernel dies/restarts/etc.

@halleysfifthinc halleysfifthinc marked this pull request as draft November 26, 2024 21:52
@halleysfifthinc halleysfifthinc changed the title Modernize AssetServer for new/current Jupyter Lab/Server extensions Update Jupyter extension Nov 26, 2024
- Updates build system to hatchling and deprecates setup.py
- Bumps jupyter package compats in python and javascript
@halleysfifthinc halleysfifthinc force-pushed the webio_jupyter-updates branch 9 times, most recently from 85e5e40 to 2e23b0d Compare November 27, 2024 04:24
- `"importsNotUsedAsValues"` deprecated in favor of `"verbatimModuleSyntax"`
- `"paths"` definition added to silence complaints
    - Recommended in
      https://jupyterlab.readthedocs.io/en/latest/extension/extension_dev.html#developing-a-prebuilt-extension
- Fix imports/type-imports for newer typescript
With the notebook

```julia
using PlotlyJS, Observables
```

```julia
p = plot(scatter(;y=rand(20))
```

```julia
on(p["hover"]) do hover
    @show hover
end
```

Run each cell, hover over several points to see everything working. There should be no errors in the browser console related to hover events.

Restart the kernel, and then hover over points again. There should be errors for every hover event.
Comment on lines +81 to +87
for (k,v) in IJulia.CommManager.comms
if IJulia.CommManager.comm_target(v) == :webio_comm
IJulia.CommManager.close_comm(v)
delete!(IJulia.CommManager.comms, k)
end
end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This doesn't actually work with Requires (not sure why), but it does work in my weakdeps branch at #516 .

Comment on lines +233 to +234
// Stop attempting to handle callbacks if previous kernel is gone
this._webIO.setSendCallback((msg: any) => {});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This fixes a non-user facing issue where trying to update observables after a kernel restart would throw errors in the browser console.

@halleysfifthinc halleysfifthinc marked this pull request as ready for review November 27, 2024 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment