jupyter-widgets / ipywidgets

Interactive Widgets for the Jupyter Notebook
https://ipywidgets.readthedocs.io
BSD 3-Clause "New" or "Revised" License
3.15k stars 950 forks source link

Check logic around changing kernels in JupyterLab manager #3088

Open jasongrout opened 3 years ago

jasongrout commented 3 years ago

In https://github.com/jupyter-widgets/ipywidgets/commit/9d999d7b267ade8037b821ce356868a3ce205bd8 (part of #2532), we introduced changes refactoring how kernels are used in the JupyterLab manager.

Looking over the code in https://github.com/jupyter-widgets/ipywidgets/blob/master/packages/jupyterlab-manager/src/manager.ts, I think it would be good to check a few things:

jtpio commented 3 years ago

When a session context changes a kernel, is this._kernel reset?

Does this question only apply to the KernelWidgetManager?

https://github.com/jupyter-widgets/ipywidgets/blob/7411536c7ccac24d2691a6cc64653e772854857b/jupyterlab_widgets/src/manager.ts#L409

The WidgetManager (used for notebooks) seems to be handling a SessionContext, which should handle kernel restarts.

https://github.com/jupyter-widgets/ipywidgets/blob/7411536c7ccac24d2691a6cc64653e772854857b/jupyterlab_widgets/src/manager.ts#L494-L504

So accessing this.kernel should return the current kernel for the session context:

https://github.com/jupyter-widgets/ipywidgets/blob/7411536c7ccac24d2691a6cc64653e772854857b/jupyterlab_widgets/src/manager.ts#L619-L621

jtpio commented 3 years ago

In ipywidgets 7.x, we waited for the context to be ready in _loadFromKernel - is that still done in the refactor, or is it needed?

Looking at the code, _loadFromKernel seems to be called from the WidgetManager here:

https://github.com/jupyter-widgets/ipywidgets/blob/7411536c7ccac24d2691a6cc64653e772854857b/jupyterlab_widgets/src/manager.ts#L565

And from KernelWidgetManager here:

https://github.com/jupyter-widgets/ipywidgets/blob/7411536c7ccac24d2691a6cc64653e772854857b/jupyterlab_widgets/src/manager.ts#L454

With _loadFromKernel defined in the base LabWidgetManager class:

https://github.com/jupyter-widgets/ipywidgets/blob/7411536c7ccac24d2691a6cc64653e772854857b/jupyterlab_widgets/src/manager.ts#L99