From 0c96d6c7a5ea5d82965a31a05a166b3f8d4a2037 Mon Sep 17 00:00:00 2001 From: Alan Fleming <> Date: Wed, 28 Aug 2024 21:34:12 +1000 Subject: [PATCH] DisposableConnection - use cid instead of id --- examples/autostart.ipynb | 2 +- examples/commands.ipynb | 32 +++++++++++---- ipylab/commands.py | 34 +++++++--------- ipylab/disposable_connection.py | 58 ++++++++++++++-------------- ipylab/shell.py | 5 ++- src/widgets/commands.ts | 12 +----- src/widgets/disposable_connection.ts | 6 +-- src/widgets/ipylab.ts | 33 ++++++---------- 8 files changed, 87 insertions(+), 95 deletions(-) diff --git a/examples/autostart.ipynb b/examples/autostart.ipynb index e756c63..a0be486 100644 --- a/examples/autostart.ipynb +++ b/examples/autostart.ipynb @@ -112,7 +112,7 @@ " start_my_app.callcount = n\n", " path = f\"my app {n}\"\n", " launcher = (\n", - " ipylab.DisposableConnection(id=app.current_widget_id) if app.current_widget_id.startswith(\"launcher\") else None\n", + " ipylab.DisposableConnection(cid=app.current_widget_id) if app.current_widget_id.startswith(\"launcher\") else None\n", " )\n", " await app.exec_eval(execute=create_app, evaluate={\"path\": f\"'{path}'\", \"payload\": \"create_app(path)\"}, path=path)\n", " if launcher:\n", diff --git a/examples/commands.ipynb b/examples/commands.ipynb index 6d0c9db..10c17e0 100644 --- a/examples/commands.ipynb +++ b/examples/commands.ipynb @@ -111,6 +111,24 @@ "w = t.result()" ] }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "t = w.list_attributes()" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "t.result()" + ] + }, { "cell_type": "code", "execution_count": null, @@ -464,19 +482,19 @@ ] }, { - "cell_type": "markdown", + "cell_type": "code", + "execution_count": null, "metadata": {}, + "outputs": [], "source": [ - "Notice the command is not in the pallet." + "show_command_pallet()" ] }, { - "cell_type": "code", - "execution_count": null, + "cell_type": "markdown", "metadata": {}, - "outputs": [], "source": [ - "show_command_pallet()" + "Notice the command is not in the pallet." ] }, { @@ -539,7 +557,7 @@ "metadata": {}, "outputs": [], "source": [ - "assert str(cmd) not in app.commands.all_commands # noqa: S101" + "assert str(cmd) in app.commands.all_commands # noqa: S101" ] }, { diff --git a/ipylab/commands.py b/ipylab/commands.py index 44850e4..d6228b5 100644 --- a/ipylab/commands.py +++ b/ipylab/commands.py @@ -49,7 +49,7 @@ def configure(self, *, emit=True, **kwgs: Unpack[CommandOptions]) -> Task[Comman async def configure_(): config = await self.update_values("config", kwgs) # type: ignore if emit: - await self.app.commands.execute_method("commandChanged.emit", {"id": self.id}) + await self.app.commands.execute_method("commandChanged.emit", {"id": self.cid}) return config return self.to_task(configure_()) @@ -117,10 +117,6 @@ class CommandPalette(AsyncWidgetBase): SINGLETON = True items: Container[tuple[CommandPalletConnection, ...]] = TypedTuple(trait=Instance(CommandPalletConnection)) - def _to_pallet_command_category_id(self, command_id: str | CommandConnection, category: str): - cmd = str(CommandConnection.get_existing_connection(command_id)) - return f"{CommandPalletConnection.to_id(str(cmd))} | {category}" - def add( self, command: str | CommandConnection, @@ -130,8 +126,8 @@ def add( args: dict | None = None, ) -> Task[CommandPalletConnection]: """Add a command to the command pallet (must be registered in this kernel).""" - id_ = self._to_pallet_command_category_id(command, category) - conn = CommandPalletConnection.get_existing_connection(id_, quiet=True) + cid = CommandPalletConnection.to_cid(str(command), category) + conn = CommandPalletConnection.get_existing_connection(cid, quiet=True) if conn: conn.dispose() return self.execute_method( @@ -142,14 +138,12 @@ def add( "command": str(command), "rank": rank, }, - id=id_, + cid=cid, transform=TransformMode.connection, ) - def remove(self, command_id: str | CommandConnection, category: str): - conn = CommandPalletConnection.get_existing_connection( - self._to_pallet_command_category_id(command_id, category), quiet=True - ) + def remove(self, commandID: str | CommandConnection, category: str): + conn = CommandPalletConnection.get_existing_connection(str(commandID), category, quiet=True) if conn: conn.dispose() @@ -159,17 +153,13 @@ class Launcher(AsyncWidgetBase): SINGLETON = True items: Container[tuple[LauncherConnection, ...]] = TypedTuple(trait=Instance(LauncherConnection)) - def _to_launcher_connection_id(self, command_id: str | CommandConnection, category: str): - cmd = str(CommandConnection.get_existing_connection(command_id)) - return f"{LauncherConnection.to_id(str(cmd))} | {category}" - def add(self, command: str | CommandConnection, category: str, *, rank=None, **args) -> Task[LauncherConnection]: """Add a launcher for the command (must be registered in this kerenel). ref: https://jupyterlab.readthedocs.io/en/latest/api/interfaces/launcher.ILauncher.IItemOptions.html """ - id_ = self._to_launcher_connection_id(command, category) - conn = LauncherConnection.get_existing_connection(id_, quiet=True) + cid = LauncherConnection.to_cid(str(command), category) + conn = LauncherConnection.get_existing_connection(cid, quiet=True) if conn: conn.dispose() return self.execute_method( @@ -180,12 +170,12 @@ def add(self, command: str | CommandConnection, category: str, *, rank=None, **a "rank": rank, "args": args, }, - id=id_, + cid=cid, transform=TransformMode.connection, ) def remove(self, command_id: str | CommandConnection, category: str): - conn = LauncherConnection.get_existing_connection(self._to_launcher_connection_id(command_id, category)) + conn = LauncherConnection.get_existing_connection(str(command_id), category) if conn: conn.dispose() @@ -241,9 +231,11 @@ def add( """ tfm = dict(frontend_transform_kwgs) if frontend_transform_kwgs else {} tfm["transform"] = TransformMode(frontend_transform) + cid = CommandConnection.to_cid(name) task = self.schedule_operation( "addCommand", - id=CommandConnection.to_id(name), + id=cid, + cid=cid, caption=caption, label=label, iconClass=icon_class, diff --git a/ipylab/disposable_connection.py b/ipylab/disposable_connection.py index b311a79..06fa156 100644 --- a/ipylab/disposable_connection.py +++ b/ipylab/disposable_connection.py @@ -3,6 +3,7 @@ from __future__ import annotations +import uuid from typing import Generic, TypeVar from ipywidgets import register @@ -20,18 +21,22 @@ class DisposableConnection(AsyncWidgetBase, Generic[T]): This defines the 'base' as the disposable object meaning the frontend attribute methods are associated directly with the object on the frontend. - The 'dispose' method will call the dispose method on the frontend object and close. + The 'dispose' method will call the dispose method on the frontend object and close see: https://lumino.readthedocs.io/en/latest/api/modules/disposable.html - Subclasses that are inherited with and ID_PREFIX + Subclasses that are inherited with and ID_PREFIX. + + In some cases it may be necessary use the keyword argument `cid` to ensure + the subclass instance is returned. The class methods `to_cid` and `new_cid` + will generate an appropriate id. """ ID_PREFIX = "" _CLASS_DEFINITIONS: dict[str, type[T]] = {} # noqa RUF012 _connections: dict[str, T] = {} # noqa RUF012 _model_name = Unicode("DisposableConnectionModel").tag(sync=True) - id = Unicode(read_only=True).tag(sync=True) + cid = Unicode(read_only=True).tag(sync=True) _dispose = Bool(read_only=True).tag(sync=True) def __init_subclass__(cls, **kwargs) -> None: @@ -39,43 +44,40 @@ def __init_subclass__(cls, **kwargs) -> None: cls._CLASS_DEFINITIONS[cls.ID_PREFIX] = cls # type: ignore super().__init_subclass__(**kwargs) - def __new__(cls, *, id: str, **kwgs): # noqa A002 - if id not in cls._connections: - if cls.ID_PREFIX and not id.startswith(cls.ID_PREFIX): - msg = f"Expected prefix '{cls.ID_PREFIX}' not found for {id=}" + def __new__(cls, *, cid: str, **kwgs): + if cid not in cls._connections: + if cls.ID_PREFIX and not cid.startswith(cls.ID_PREFIX): + msg = f"Expected prefix '{cls.ID_PREFIX}' not found for {cid=}" raise ValueError(msg) # Check if a subclass is registered with 'ID_PREFIX' - cls_ = cls._CLASS_DEFINITIONS.get(id.split(":")[0], cls) if ":" in id else cls - cls._connections[id] = super().__new__(cls_, **kwgs) # type: ignore - return cls._connections[id] + cls_ = cls._CLASS_DEFINITIONS.get(cid.split(":")[0], cls) if ":" in cid else cls + cls._connections[cid] = super().__new__(cls_, **kwgs) # type: ignore + return cls._connections[cid] def __str__(self): - return self.id + return self.cid + + @classmethod + def to_cid(cls, *args: str) -> str: + """Generate an id for the args""" + return " | ".join([f"{cls.ID_PREFIX}:{args[0].removeprefix(cls.ID_PREFIX).strip(':')}", *args[1:]]).strip(": ") @classmethod - def to_id(cls, name_or_id: str | T) -> str: - """Generate an id for the given name.""" - if isinstance(name_or_id, DisposableConnection): - if not isinstance(name_or_id, cls): - msg = f"{name_or_id} is not a subclass of {cls}" - raise TypeError(msg) - return name_or_id.id - if not cls.ID_PREFIX: - return name_or_id - return f"{cls.ID_PREFIX}:{name_or_id.removeprefix(cls.ID_PREFIX).strip(':')}" + def new_cid(cls, *args): + return cls.to_cid(str(uuid.uuid4()), *args) @classmethod def get_instances(cls) -> tuple[T]: return tuple(item for item in cls._connections.values() if item.__class__ is cls) # type: ignore - def __init__(self, *, id: str, model_id=None, **kwgs): # noqa: A002 + def __init__(self, *, cid: str, model_id=None, **kwgs): if self._async_widget_base_init_complete: return - self.set_trait("id", id) + self.set_trait("cid", cid) super().__init__(model_id=model_id, **kwgs) def close(self): - self._connections.pop(self.id, None) + self._connections.pop(self.cid, None) super().close() def dispose(self): @@ -84,7 +86,7 @@ def dispose(self): self.close() @classmethod - def get_existing_connection(cls, name_or_id: str | T, *, quiet=False): + def get_existing_connection(cls, *name_or_id: str, quiet=False): """Get an existing connection. quiet: bool @@ -92,9 +94,9 @@ def get_existing_connection(cls, name_or_id: str | T, *, quiet=False): * False -> Will raise an error. * True -> Will return None. """ - id_ = cls.to_id(name_or_id) - conn = cls._connections.get(id_) + cid = cls.to_cid(*name_or_id) + conn = cls._connections.get(cid) if not conn and not quiet: - msg = f"A connection does not exist with id='{id_}'" + msg = f"A connection does not exist with id='{cid}'" raise ValueError(msg) return conn diff --git a/ipylab/shell.py b/ipylab/shell.py index fbbc4b7..b732099 100644 --- a/ipylab/shell.py +++ b/ipylab/shell.py @@ -7,13 +7,14 @@ from ipylab import pack from ipylab._compat.enum import StrEnum from ipylab.asyncwidget import AsyncWidgetBase, TransformMode, Unicode -from ipylab.disposable_connection import DisposableConnection if t.TYPE_CHECKING: from asyncio import Task from ipywidgets import Widget + from ipylab.disposable_connection import DisposableConnection + __all__ = ["Area", "InsertMode", "Shell"] @@ -80,7 +81,7 @@ def addToShell( "activate": activate, "mode": InsertMode(mode), "rank": int(rank) if rank else None, - "ref": ref.id if isinstance(ref, DisposableConnection) else ref or None, + "ref": pack(ref), } return self.app.schedule_operation( "addToShell", diff --git a/src/widgets/commands.ts b/src/widgets/commands.ts index fd57c24..3634f2d 100644 --- a/src/widgets/commands.ts +++ b/src/widgets/commands.ts @@ -3,13 +3,7 @@ import { unpack_models } from '@jupyter-widgets/base'; import { CommandRegistry } from '@lumino/commands'; -import { - IDisposable, - ISerializers, - IpylabModel, - JSONValue, - onKernelLost -} from './ipylab'; +import { IDisposable, ISerializers, IpylabModel, JSONValue } from './ipylab'; /** * The model for a command registry. */ @@ -108,11 +102,7 @@ export class CommandRegistryModel extends IpylabModel { usage: () => config?.usage }; const command = this.commands.addCommand(id, options_ as any); - (command as any).id = id; (command as any).config = config; - - IpylabModel.trackDisposable(command); - onKernelLost(this.kernel, command.dispose, command); return command; } diff --git a/src/widgets/disposable_connection.ts b/src/widgets/disposable_connection.ts index 14e01d7..95ab175 100644 --- a/src/widgets/disposable_connection.ts +++ b/src/widgets/disposable_connection.ts @@ -6,9 +6,7 @@ import { ObjectHash } from 'backbone'; import { IpylabModel } from './ipylab'; /** - * Maintain a connection to a Lumino widget such as a MainAreaWidget, Console, TextEditor, etc... - * The widget must exist in the shell or have already been added to the tracker. - * + * Maintain a connection to any object with a dispose method. */ export class DisposableConnectionModel extends IpylabModel { async initialize( @@ -26,7 +24,7 @@ export class DisposableConnectionModel extends IpylabModel { get base() { try { - return this.getDisposable(this.get('id')); + return this.getDisposable(this.get('cid')); } catch { this.close(); } diff --git a/src/widgets/ipylab.ts b/src/widgets/ipylab.ts index 7413b17..7b8b207 100644 --- a/src/widgets/ipylab.ts +++ b/src/widgets/ipylab.ts @@ -243,7 +243,6 @@ export class IpylabModel extends WidgetModel { } const view = await this.widget_manager.create_view(model, {}); const lw = view.luminoWidget; - IpylabModel.trackDisposable(lw); onKernelLost(this.kernel, lw.dispose, lw); return lw; } @@ -354,7 +353,7 @@ export class IpylabModel extends WidgetModel { /** * Set an attribute at the path with transformation. */ - private async _setAttribute(payload: any): Promise { + private async _setAttribute(payload: any): Promise { const { path, value, valueTransform } = payload; const value_ = await this.transformObject(value, valueTransform); setNestedAttribute(this.base, path, value_); @@ -387,7 +386,7 @@ export class IpylabModel extends WidgetModel { return Private.disposables.get(id); } const disposable = this._getLuminoWidgetFromShell(id); - IpylabModel.trackDisposable(disposable); + IpylabModel.trackDisposable(disposable, id); return disposable; } @@ -405,6 +404,7 @@ export class IpylabModel extends WidgetModel { const transform = typeof args === 'string' ? args : args.transform; let path: string, transform_: string, result, part: string, func; + let cid; switch (transform) { case 'done': @@ -414,16 +414,8 @@ export class IpylabModel extends WidgetModel { case 'null': return null; case 'connection': - if (!obj.id) { - if (args && typeof args.id === 'string') { - obj['id'] = args.id; - } else if (args.kwgs && typeof args.kwgs.id === 'string') { - obj['id'] = args.kwgs.id; - } else { - obj['id'] = `${UUID.uuid4()}`; - } - } - IpylabModel.trackDisposable(obj); + cid = args?.cid || args?.kwgs?.cid || obj.id || DOMUtils.createDomID(); + IpylabModel.trackDisposable(obj, cid); if ( this.kernel && args?.onKernelLost !== false && @@ -431,7 +423,7 @@ export class IpylabModel extends WidgetModel { ) { onKernelLost(this.kernel, obj.dispose, obj, true); } - return { id: obj.id }; + return { cid: cid }; case 'attribute': // expects simple: {parts:['dotted.attribute']} // or advanced: {parts:[{path:'dotted.attribute', transform:'...' }] @@ -463,16 +455,15 @@ export class IpylabModel extends WidgetModel { *Keep a reference to a Disposable so it can be found from the backend. * @param disposable */ - static trackDisposable(disposable: any) { + static trackDisposable(disposable: any, cid: string) { if (typeof disposable.dispose !== 'function') { throw new Error(`Not disposable: ${disposable}`); } - if (!disposable.id) { - disposable.id = DOMUtils.createDomID(); + if (!cid) { + throw new Error('Cannot track without an id'); } - const key = disposable.id; - if (!Private.disposables.has(key)) { - Private.disposables.set(key, disposable); + if (!Private.disposables.has(cid)) { + Private.disposables.set(cid, disposable); if (!disposable.disposed) { // Convert a Disposable into an ObservableDisposable disposable.disposed = new Signal(disposable); @@ -487,7 +478,7 @@ export class IpylabModel extends WidgetModel { }; disposable['dispose'] = dispose.bind(disposable); } - disposable.disposed.connect(() => Private.disposables.delete(key)); + disposable.disposed.connect(() => Private.disposables.delete(cid)); } }