ipywidgets icon indicating copy to clipboard operation
ipywidgets copied to clipboard

Check logic around changing kernels in JupyterLab manager

Open jasongrout opened this issue 4 years ago • 2 comments

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:

  • When a session context changes a kernel, is this._kernel reset?
  • 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?
  • Consider this comment again: https://github.com/jupyter-widgets/ipywidgets/pull/2532#discussion_r367882710
  • Consider this comment again: https://github.com/jupyter-widgets/ipywidgets/pull/2532#discussion_r367884298

jasongrout avatar Feb 05 '21 19:02 jasongrout

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 avatar Mar 10 '21 09:03 jtpio

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

jtpio avatar Mar 10 '21 09:03 jtpio