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

Draft: Add bi-directional comms and run operations in tasks #135

Open
wants to merge 73 commits into
base: main
Choose a base branch
from

Conversation

fleming79
Copy link

@fleming79 fleming79 commented Jan 4, 2024

This adds supports for asynchronous bi-directional messaging from the Python objects running in the Python kernel with the JavaScript Frontend. If this is adopted, it would be a major release.

  • Bi-directional comms.
  • Panel can add itself to the shell.
  • Add app as a property of Panel.
  • Views & Models close when the Python kernel is stopped / restarted.
  • Panel renders correctly on view.
  • SplitPanel renders correctly on view (need help - it renders but children don't size correctly until it is re-sized in both directions) upstream issue: SplitPanel has 0 width and height jupyterlab/lumino#675.
  • Support autostart via pluggy.
  • Self-contained session (some errors emitted in JS console, but works OK) - need help. Support server-side execution jupyterlab/jupyterlab#15448 (listed in 4.2 Release Plan jupyterlab/jupyterlab#15801) might be useful.
  • Support for execution of nested methods in the frontend.
  • List attributes/methods on the frontend app.
  • Main area widget with own console can hide/show itself.
  • Execute code in a separate python kernel without blocking the kernel.

Copy link
Contributor

github-actions bot commented Jan 4, 2024

Binder 👈 Try it on Binder (branch fleming79/ipylab/main)

Alan Fleming added 3 commits January 16, 2024 16:05
- bi-directional messaging with AsyncWidgetBase
- AsyncWidgetBase supports SINGLETON and corresponds to the typesript model  IpylabModel
- IpylabModel
@fleming79 fleming79 marked this pull request as draft January 16, 2024 23:45
@fleming79 fleming79 changed the title Fix Panel rendering in main area Draft: Add bi-directional comms and run operations in tasks Jan 16, 2024
Alan Fleming added 22 commits January 17, 2024 11:22
- added _customItems
- dropped 'Private' namespace
- added _removeItem
CommandPalette
- _items changed to items
- fix serialization of icon
- fix test for closing on comm_live change
- restore labIcon
SessionManager - restore refresh_running
- added app_session to capture the session in which ipylab is running.
- added open_console
- wait_ready updated to wait for sessions
Added dialog to JupyterFrontEnd
Added get_open_files & get_existing_directory
…o supported.

Added python side plugin support using pluggy (new dependency).
Current hooks: on_frontend_error, get_ipylab_backend_class & register_launcher
register_launcher is not implemented yet.
Alan Fleming added 6 commits February 4, 2024 18:39
- launcher
- kerenel status watcher and onKernelLost to dispose instead of using 'comm_live_update'.
- execute_command on AsyncWidgetBase instead of execute on CommandRegistry.
Renamed
- JupyterFrontEnd.commands to JupyterFrontEnd.command
Removed
- Hookspec: get_ipylab_backend_class & run_once_at_startup
added overload of save_changes to only save when com_live is true
JupyterFrontEndModel added close overload
…other kernel via the model in that kernel.

renamed `execute_method` to `executeMethod`
schedule_operation renamed to scheduleOperation
@fleming79 fleming79 marked this pull request as ready for review February 26, 2024 10:28
@fleming79
Copy link
Author

Regarding creating a new session without a notebook. I'm wondering if the new session should be based around a document...

Currently, the function newSession defined int utils.ts creates a new KernelWidgetManager, however the base instance doesn't provide the models that get defined using the plugin system. Currently only a limited number known are provided with the function registerWidgets defined in utils. This means that models registered using the ipywidgets plugin system don't get automatically added.

It'd be better to use registerWidgetManager from IpyWidgets, but as you can see in the prototype below, the function is expecting a DocumentRegistry.IContext<INotebookModel> context.

export function registerWidgetManager(
  context: DocumentRegistry.IContext<INotebookModel>,
  rendermime: IRenderMimeRegistry,
  renderers: IterableIterator<WidgetRenderer>
): DisposableDelegate {

@fleming79 fleming79 marked this pull request as draft May 6, 2024 21:46
@jtpio
Copy link
Owner

jtpio commented May 17, 2024

Thanks @fleming79 for working on this!

And sorry for the delay, I'll try to have a look soon.

@fleming79
Copy link
Author

@jtpio - no problem. I've been learning as I go and using it for my own purposes.

As an aside, I was looking at your PR for Ipywidgets jupyter-widgets/ipywidgets#3004 from 2020 (which is still open) and saw a bit of a discussion having a widgetManager on a per-kernel basis.

Jason Grout said:
I've been going over this code and thinking about this, and there's something I think this effort is exposing about limitations in how the current lab widget manager is written. This approach (properly, I think) tries to push the notion of a widget manager down to the kernel level (i.e., the widget manager is "owned" by the kernel id, for example). However, the notebook widget manager really lives above that, at the session context level (e.g., a notebook widget manager can be "owned" by several different kernel ids over its lifetime, and right now there really isn't a provision for a notebook widget manager that doesn't happen to be associated with a kernel, as Jeremy points out above when talking about the assertion operator.

This makes me think that perhaps we should restructure the code so that we have one concept of a widget manager that is tied to a kernel, and use composition to interface with the notebook rather than inheritance. In other words, we have a layer on top of the kernel widget manager that manages notebook state and interfaces with the (kernel) widget manager to deal with saving and restoring state from a notebook. In other words, the object at the notebook level HAS a kernel widget manager (not IS a widget manager), and the kernel widget manager can be swapped out or created as needed when the kernel changes. This way consoles and notebooks can cache and share kernel widget managers freely on equal footing. Still thinking through it, but this seems to address this weird disconnect we have between notebook widget managers and kernel widget managers.

I think this concept is worth pursuing and am currently investigating how difficult it may be to implement. It would solve the issue in this PR associated with needing to create a WidgetManager and manually register widgetmodels, and the problem of widget comms shutting down when the notebook is closed.

@fleming79
Copy link
Author

fleming79 commented May 26, 2024

I noticed that jupyter-widgets/ipywidgets#3004 has now been merged - that's great!
Edit:
I'm wondering if the legacy function registerWidgetManager here could be used.

@fleming79 fleming79 marked this pull request as ready for review May 27, 2024 07:04
@fleming79
Copy link
Author

registerWidgetManager appears to work okay with the most recent jupyterlab_widgets mentioned above by creating a dummy context.

  // For the moment we'll use a dummy session.
  // In future it might be better to support a document...
  const session = sessionContext.session;
  const context = {};
  (context as any)['sessionContext'] = sessionContext;
  (context as any)['saveState'] = new Signal({});
  (context as any).saveState.connect(() => {
    null;
  });
  registerWidgetManager(context as any, rendermime, [] as any);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants