From 12c99e4deb1090a5d4080d5f2953ef623544ec80 Mon Sep 17 00:00:00 2001 From: Sok82 Developer Date: Fri, 16 Aug 2024 16:00:17 +0300 Subject: [PATCH 1/3] Added notifyOnComplete paramater for code execution enabling to send model notifications only when all code execution completed --- .../react/src/components/output/Output.tsx | 9 ++++--- .../src/components/output/OutputAdapter.ts | 4 +-- .../src/components/output/OutputExecutor.ts | 4 ++- packages/react/src/jupyter/kernel/Kernel.ts | 3 +++ .../src/jupyter/kernel/KernelExecutor.ts | 25 ++++++++++++++++--- 5 files changed, 35 insertions(+), 10 deletions(-) diff --git a/packages/react/src/components/output/Output.tsx b/packages/react/src/components/output/Output.tsx index 3dc2c8d7..10e88cdc 100644 --- a/packages/react/src/components/output/Output.tsx +++ b/packages/react/src/components/output/Output.tsx @@ -40,6 +40,7 @@ export type IOutputProps = { showEditor: boolean; showKernelProgressBar?: boolean; toolbarPosition: 'up' | 'middle' | 'none'; + notifyOnComplete? : boolean; }; export const Output = (props: IOutputProps) => { @@ -62,6 +63,7 @@ export const Output = (props: IOutputProps) => { showControl, showEditor, showKernelProgressBar = true, + notifyOnComplete = false, id: sourceId, toolbarPosition, } = props; @@ -121,7 +123,7 @@ export const Output = (props: IOutputProps) => { useEffect(() => { if (adapter) { if (autoRun) { - adapter.execute(code); + adapter.execute(code, notifyOnComplete); } } }, [adapter]); @@ -141,12 +143,12 @@ export const Output = (props: IOutputProps) => { const executeRequest = outputStore.getExecuteRequest(sourceId); useEffect(() => { if (adapter && executeRequest && executeRequest === id) { - adapter.execute(code); + adapter.execute(code, notifyOnComplete); } }, [executeRequest, adapter]); useEffect(() => { if (adapter && executeTrigger > 0) { - adapter.execute(code); + adapter.execute(code, notifyOnComplete); } }, [executeTrigger]); useEffect(() => { @@ -244,6 +246,7 @@ Output.defaultProps = { metadata: {}, }, ], + notifyOnComplete : false, toolbarPosition: 'up', } as Partial; diff --git a/packages/react/src/components/output/OutputAdapter.ts b/packages/react/src/components/output/OutputAdapter.ts index 36c50214..f076033b 100755 --- a/packages/react/src/components/output/OutputAdapter.ts +++ b/packages/react/src/components/output/OutputAdapter.ts @@ -79,10 +79,10 @@ export class OutputAdapter { this.initKernel(); } - public async execute(code: string) { + public async execute(code: string, notifyOnComplete? : boolean) { if (this._kernel) { this.clear(); - const done = execute(this._id, code, this._outputArea, this._kernel); + const done = execute(this._id, code, this._outputArea, this._kernel, notifyOnComplete); await done; } } diff --git a/packages/react/src/components/output/OutputExecutor.ts b/packages/react/src/components/output/OutputExecutor.ts index 66d34531..05a4b831 100755 --- a/packages/react/src/components/output/OutputExecutor.ts +++ b/packages/react/src/components/output/OutputExecutor.ts @@ -17,7 +17,8 @@ export async function execute( code: string, output: OutputArea, kernel: Kernel, - metadata?: JSONObject + metadata?: JSONObject, + notifyOnComplete? : boolean ): Promise { // Override the default for `stop_on_error`. let stopOnError = true; @@ -33,6 +34,7 @@ export async function execute( { model: output.model, stopOnError, + notifyOnComplete : notifyOnComplete } ); const future = kernelExecutor!.future; diff --git a/packages/react/src/jupyter/kernel/Kernel.ts b/packages/react/src/jupyter/kernel/Kernel.ts index 62bcff12..d076ab82 100755 --- a/packages/react/src/jupyter/kernel/Kernel.ts +++ b/packages/react/src/jupyter/kernel/Kernel.ts @@ -194,6 +194,7 @@ export class Kernel { stopOnError, storeHistory, allowStdin, + notifyOnComplete = false }: { model?: IOutputAreaModel; iopubMessageHooks?: IOPubMessageHook[]; @@ -202,12 +203,14 @@ export class Kernel { stopOnError?: boolean; storeHistory?: boolean; allowStdin?: boolean; + notifyOnComplete? : boolean } = {} ): KernelExecutor | undefined { if (this._kernelConnection) { const kernelExecutor = new KernelExecutor({ connection: this._kernelConnection, model, + notifyOnComplete, }); kernelExecutor.execute(code, { iopubMessageHooks, diff --git a/packages/react/src/jupyter/kernel/KernelExecutor.ts b/packages/react/src/jupyter/kernel/KernelExecutor.ts index fadd2e74..ed3b2a8e 100644 --- a/packages/react/src/jupyter/kernel/KernelExecutor.ts +++ b/packages/react/src/jupyter/kernel/KernelExecutor.ts @@ -41,6 +41,11 @@ export type IKernelExecutorOptions = { * Outputs model to populate with the execution results. */ model?: IOutputAreaModel; + /** + * Flag defining if notification about model changes + * must only be made when execution complete + */ + notifyOnComplete? : boolean; } export class KernelExecutor { @@ -54,13 +59,15 @@ export class KernelExecutor { private _outputsChanged = new Signal(this); private _future?: JupyterKernel.IFuture; private _shellMessageHooks = new Array(); + private _notifyOnComplete : boolean = false; - public constructor({ connection, model }: IKernelExecutorOptions) { + public constructor({ connection, model, notifyOnComplete }: IKernelExecutorOptions) { this._executed = new PromiseDelegate(); this._kernelConnection = connection; this._model = model ?? new OutputAreaModel(); this._outputs = []; this._kernelState = kernelsStore.getState(); + this._notifyOnComplete = !!notifyOnComplete; } /** @@ -118,6 +125,10 @@ export class KernelExecutor { // Wait for future to be done before resolving the exectud promise. this._future.done.then(() => { kernelsStore.getState().setExecutionPhase(this._kernelConnection.id, ExecutionPhase.completed); + // We emit model changes only when execution completed + if (this._notifyOnComplete) { + this._modelChanged.emit(this._model); + } this._executed.resolve(this._model); }); return this._executed.promise; @@ -204,13 +215,17 @@ export class KernelExecutor { this._outputs.push(message.content as IExecuteResult); this._outputsChanged.emit(this._outputs); this._model.add(output); - this._modelChanged.emit(this._model); + if (!this._notifyOnComplete) { + this._modelChanged.emit(this._model); + } break; case 'display_data': this._outputs.push(message.content as IDisplayData); this._outputsChanged.emit(this._outputs); this._model.add(output); - this._modelChanged.emit(this._model); + if (!this._notifyOnComplete) { + this._modelChanged.emit(this._model); + } break; case 'stream': case 'error': @@ -231,7 +246,9 @@ export class KernelExecutor { this._outputsChanged.emit(this._outputs); // FIXME this needs more advanced analysis see OutputArea this._model.add(output); - this._modelChanged.emit(this._model); + if (!this._notifyOnComplete) { + this._modelChanged.emit(this._model); + } break; case 'status': const executionState = (message.content as any).execution_state as KernelMessage.Status; From 5fb502798b4f55d2c96e7ed416e6bcfe5dd620c0 Mon Sep 17 00:00:00 2001 From: Sok82 Developer Date: Fri, 6 Sep 2024 08:44:36 +0000 Subject: [PATCH 2/3] Fixed missing metadata parameter for OutputExecutor --- .vscode/settings.json | 3 +++ packages/react/src/components/output/OutputAdapter.ts | 4 +++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 450b8248..8cda1d87 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -11,4 +11,7 @@ "[typescriptreact]": { "editor.defaultFormatter": "esbenp.prettier-vscode", }, + "githubPullRequests.ignoredPullRequestBranches": [ + "main" + ], } \ No newline at end of file diff --git a/packages/react/src/components/output/OutputAdapter.ts b/packages/react/src/components/output/OutputAdapter.ts index f076033b..bca2c33f 100755 --- a/packages/react/src/components/output/OutputAdapter.ts +++ b/packages/react/src/components/output/OutputAdapter.ts @@ -5,6 +5,7 @@ */ import { IOutput } from '@jupyterlab/nbformat'; +import { JSONObject } from '@lumino/coreutils'; import { IOutputAreaModel, OutputArea, OutputAreaModel } from '@jupyterlab/outputarea'; import { IRenderMime, RenderMimeRegistry, standardRendererFactories } from '@jupyterlab/rendermime'; import { rendererFactory as jsonRendererFactory } from '@jupyterlab/json-extension'; @@ -82,7 +83,8 @@ export class OutputAdapter { public async execute(code: string, notifyOnComplete? : boolean) { if (this._kernel) { this.clear(); - const done = execute(this._id, code, this._outputArea, this._kernel, notifyOnComplete); + const metadata : JSONObject = new JSONObject(); + const done = execute(this._id, code, this._outputArea, this._kernel, metadata, notifyOnComplete); await done; } } From 8bbf85a08c3cb521e8c41ea8b76d8f93b9295bcb Mon Sep 17 00:00:00 2001 From: Sok82 Developer Date: Fri, 6 Sep 2024 21:08:20 +0300 Subject: [PATCH 3/3] Fixed JSONObject initialization --- packages/react/src/components/output/OutputAdapter.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/components/output/OutputAdapter.ts b/packages/react/src/components/output/OutputAdapter.ts index bca2c33f..af94b8cb 100755 --- a/packages/react/src/components/output/OutputAdapter.ts +++ b/packages/react/src/components/output/OutputAdapter.ts @@ -83,7 +83,7 @@ export class OutputAdapter { public async execute(code: string, notifyOnComplete? : boolean) { if (this._kernel) { this.clear(); - const metadata : JSONObject = new JSONObject(); + const metadata : JSONObject = {}; const done = execute(this._id, code, this._outputArea, this._kernel, metadata, notifyOnComplete); await done; }