diff --git a/news/2 Fixes/9503.md b/news/2 Fixes/9503.md new file mode 100644 index 00000000000..f75ef677f70 --- /dev/null +++ b/news/2 Fixes/9503.md @@ -0,0 +1 @@ +Support displaying of complex outputs (such as Plots) in the Output Widget. diff --git a/src/kernels/execution/cellExecutionMessageHandler.ts b/src/kernels/execution/cellExecutionMessageHandler.ts index a7cacf06094..35e34f56492 100644 --- a/src/kernels/execution/cellExecutionMessageHandler.ts +++ b/src/kernels/execution/cellExecutionMessageHandler.ts @@ -5,7 +5,7 @@ import * as fastDeepEqual from 'fast-deep-equal'; import type * as nbformat from '@jupyterlab/nbformat'; -import type * as KernelMessage from '@jupyterlab/services/lib/kernel/messages'; +import * as KernelMessage from '@jupyterlab/services/lib/kernel/messages'; import { NotebookCell, NotebookCellExecution, @@ -30,15 +30,20 @@ import { CellExecutionCreator } from './cellExecutionCreator'; import { IApplicationShell } from '../../platform/common/application/types'; import { disposeAllDisposables } from '../../platform/common/helpers'; import { traceError, traceWarning } from '../../platform/logging'; -import { RefBool } from '../../platform/common/refBool'; import { IDisposable, IExtensionContext } from '../../platform/common/types'; import { concatMultilineString, formatStreamText, isJupyterNotebook } from '../../platform/common/utils'; -import { traceCellMessage, cellOutputToVSCCellOutput, translateCellDisplayOutput } from './helpers'; +import { + traceCellMessage, + cellOutputToVSCCellOutput, + translateCellDisplayOutput, + CellOutputMimeTypes +} from './helpers'; import { swallowExceptions } from '../../platform/common/utils/decorators'; import { noop } from '../../platform/common/utils/misc'; import { ITracebackFormatter } from '../../kernels/types'; import { handleTensorBoardDisplayDataOutput } from './executionHelpers'; import { WIDGET_MIMETYPE } from '../../kernels/ipywidgets-message-coordination/constants'; +import isObject = require('lodash/isObject'); // Helper interface for the set_next_input execute reply payload interface ISetNextInputPayload { @@ -60,10 +65,43 @@ type DisplayData = nbformat.IDisplayData & { */ export const activeNotebookCellExecution = new WeakMap(); +function getParentHeaderMsgId(msg: KernelMessage.IMessage): string | undefined { + if (msg.parent_header && 'msg_id' in msg.parent_header) { + return msg.parent_header.msg_id; + } + return undefined; +} + +/** + * The Output Widget in Jupyter can render multiple outputs. However some of them + * like vendored mimetypes cannot be handled by it. + */ +function canMimeTypeBeRenderedByWidgetManager(mime: string) { + if (mime == CellOutputMimeTypes.stderr || mime == CellOutputMimeTypes.stdout || mime == CellOutputMimeTypes.error) { + // These are plain text mimetypes that can be rendered by the Jupyter Lab widget manager. + return true; + } + if (mime === WIDGET_MIMETYPE) { + return true; + } + if (mime.startsWith('application/vnd')) { + // Custom vendored mimetypes cannot be rendered by the widget manager, it relies on the output renderers. + return false; + } + // Everything else can be rendered by the Jupyter Lab widget manager. + return true; +} + /** * Responsible for handling of jupyter messages as a result of execution of individual cells. */ export class CellExecutionMessageHandler implements IDisposable { + /** + * Keeps track of the Models Ids (model_id) that is displayed in the output of a specific cell. + * model_id's maps to widget outputs. + * Hence here we're keeping track of the widgets displayed in outputs of specific cells. + */ + private static modelIdsOwnedByCells = new WeakMap>(); /** * The msg_id of the original request execute (when executing the cell). */ @@ -73,10 +111,12 @@ export class CellExecutionMessageHandler implements IDisposable { */ private completedExecution?: boolean; /** - * Listen to messages and update our cell execution state appropriately - * Keep track of our clear state + * Jupyter can sent a `clear_output` message which indicates the output of a cell should be cleared. + * If the flag `wait` is set to `true`, then we should wait for the next output before clearing the output. + * I.e. if the value for `wait` is false (default) then clear the cell output immediately. + * https://ipywidgets.readthedocs.io/en/latest/examples/Output%20Widget.html#Output-widgets:-leveraging-Jupyter's-display-system */ - private readonly clearState = new RefBool(false); + private clearOutputOnNextUpdateToOutput?: boolean; private execution?: NotebookCellExecution; private readonly _onErrorHandlingIOPubMessage = new EventEmitter<{ @@ -98,6 +138,25 @@ export class CellExecutionMessageHandler implements IDisposable { * Because if after the stream we have an image, then the stream is not the last output item, hence its cleared. */ private lastUsedStreamOutput?: { stream: 'stdout' | 'stderr'; text: string; output: NotebookCellOutput }; + private outputsAreSpecificToAWidget?: { + /** + * Comm Id (or model_id) of widget that will handle all messages and render them via the widget manager. + * This could be a widget in another cell. + */ + handlingCommId: string; + /** + * All messages that have a parent_header.msg_id = msg_id will be swallowed and handled by the widget with model_id = this.handlingCommId. + * These requests could be from another cell, ie messages can original from one cell and end up getting displayed in another. + * E.g. widget is in cell 1 and output will be redirected from cell 2 into widget 1. + */ + msgIdsToSwallow: string; + /** + * If true, then we should clear all of the output owned by the widget defined by the commId. + * By owned, we mean the output added after the widget widget output and not the widget itself. + */ + clearOutputOnNextUpdateToOutput?: boolean; + }; + private commIdsMappedToParentWidgetModel = new Map(); private readonly disposables: IDisposable[] = []; private readonly prompts = new Set(); /** @@ -105,11 +164,12 @@ export class CellExecutionMessageHandler implements IDisposable { * or for any subsequent requests as a result of outputs sending custom messages. */ private readonly ownedCommIds = new Set(); + private static readonly outputsOwnedByWidgetModel = new Map>(); /** * List of msg_ids of requests sent either as part of request_execute * or for any subsequent requests as a result of outputs sending custom messages. */ - private readonly ownedRequestIds = new Set(); + private readonly ownedRequestMsgIds = new Set(); constructor( public readonly cell: NotebookCell, private readonly applicationService: IApplicationShell, @@ -122,7 +182,7 @@ export class CellExecutionMessageHandler implements IDisposable { cellExecution: NotebookCellExecution ) { this.executeRequestMessageId = request.msg.header.msg_id; - this.ownedRequestIds.add(request.msg.header.msg_id); + this.ownedRequestMsgIds.add(request.msg.header.msg_id); workspace.onDidChangeNotebookDocument( (e) => { if (!isJupyterNotebook(e.notebook)) { @@ -142,16 +202,16 @@ export class CellExecutionMessageHandler implements IDisposable { this.disposables ); this.execution = cellExecution; - request.onIOPub = (msg) => { + // We're in all messages. + // When using the `interact` function in Python, we can get outputs from comm messages even before execution has completed. + // See https://github.com/microsoft/vscode-jupyter/issues/9503 for more information on why we need to monitor anyMessage and iopubMessage signals. + this.kernel.anyMessage.connect(this.onKernelAnyMessage, this); + this.kernel.iopubMessage.connect(this.onKernelIOPubMessage, this); + + request.onIOPub = () => { // Cell has been deleted or the like. - if (this.cell.document.isClosed) { + if (this.cell.document.isClosed && !this.completedExecution) { request.dispose(); - return; - } - try { - this.handleIOPub(msg); - } catch (ex) { - this._onErrorHandlingIOPubMessage.fire(ex); } }; request.onReply = (msg) => { @@ -166,11 +226,6 @@ export class CellExecutionMessageHandler implements IDisposable { request.done .finally(() => { this.completedExecution = true; - // We're only interested in messages after execution has completed. - // See https://github.com/microsoft/vscode-jupyter/issues/9503 for more information. - // this.kernel.anyMessage.connect(this.onKernelAnyMessage, this); - // this.kernel.iopubMessage.connect(this.onKernelIOPubMessage, this); - this.endCellExecution(); }) .catch(noop); @@ -201,7 +256,7 @@ export class CellExecutionMessageHandler implements IDisposable { this.clearLastUsedStreamOutput(); this.execution = undefined; - if (this.ownedCommIds.size === 0 && this.completedExecution) { + if (this.cell.document.isClosed || (this.ownedCommIds.size === 0 && this.completedExecution)) { // If no comms channels were opened as a result of any outputs of this cell, // this means we don't have any widgets that can send comm message back to the kernel. // Hence no point listening to any of the iopub messages & the like, i.e. we can stop listening to everything in this class. @@ -209,6 +264,10 @@ export class CellExecutionMessageHandler implements IDisposable { } } private onKernelAnyMessage(_: unknown, { direction, msg }: Kernel.IAnyMessageArgs) { + if (this.cell.document.isClosed) { + return this.endCellExecution(); + } + // We're only interested in messages after execution has completed. // See https://github.com/microsoft/vscode-jupyter/issues/9503 for more information. if (direction !== 'send' || !this.completedExecution) { @@ -219,10 +278,13 @@ export class CellExecutionMessageHandler implements IDisposable { if (jupyterLab.KernelMessage.isCommMsgMsg(msg) && this.ownedCommIds.has(msg.content.comm_id)) { // Looks like we have a comm msg request sent by some output or the like. // See https://github.com/microsoft/vscode-jupyter/issues/9503 for more information. - this.ownedRequestIds.add(msg.header.msg_id); + this.ownedRequestMsgIds.add(msg.header.msg_id); } } private onKernelIOPubMessage(_: unknown, msg: KernelMessage.IIOPubMessage) { + if (this.cell.document.isClosed) { + return this.endCellExecution(); + } // We're only interested in messages after execution has completed. // See https://github.com/microsoft/vscode-jupyter/issues/9503 for more information. @@ -230,11 +292,9 @@ export class CellExecutionMessageHandler implements IDisposable { // comm message (requests) sent by an output widget. // See https://github.com/microsoft/vscode-jupyter/issues/9503 for more information. if ( - !this.completedExecution || !msg.parent_header || !('msg_id' in msg.parent_header) || - msg.parent_header.msg_id === this.executeRequestMessageId || - !this.ownedRequestIds.has(msg.parent_header.msg_id) || + !this.ownedRequestMsgIds.has(msg.parent_header.msg_id) || msg.channel !== 'iopub' ) { return; @@ -243,6 +303,11 @@ export class CellExecutionMessageHandler implements IDisposable { this.handleIOPub(msg); } catch (ex) { traceError(`Failed to handle iopub message as a result of some comm message`, msg, ex); + if (!this.completedExecution && !this.cell.document.isClosed) { + // If there are problems handling the execution, then bubble those to the calling code. + // Else just log the errors. + this._onErrorHandlingIOPubMessage.fire(ex); + } } } private clearLastUsedStreamOutput() { @@ -321,7 +386,7 @@ export class CellExecutionMessageHandler implements IDisposable { } else if (jupyterLab.KernelMessage.isCommOpenMsg(msg)) { // Noop. } else if (jupyterLab.KernelMessage.isCommMsgMsg(msg)) { - // Noop. + this.handleCommMsg(msg); } else if (jupyterLab.KernelMessage.isCommCloseMsg(msg)) { // Noop. } else { @@ -333,16 +398,91 @@ export class CellExecutionMessageHandler implements IDisposable { this.execution.executionOrder = msg.content.execution_count; } } + private handleCommMsg(msg: KernelMessage.ICommMsgMsg) { + const data = msg.content.data as Partial<{ + method: 'update'; + state: { msg_id: string } | { children: string[] }; + }>; + if (!isObject(data) || data.method !== 'update' || !isObject(data.state)) { + return; + } - private addToCellData(output: ExecuteResult | DisplayData | nbformat.IStream | nbformat.IError | nbformat.IOutput) { - if ( - this.context.extensionMode === ExtensionMode.Test && - output.data && - typeof output.data === 'object' && - WIDGET_MIMETYPE in output.data + if ('msg_id' in data.state && typeof data.state.msg_id === 'string') { + // When such a comm message is received, then + // the kernel is instructing the front end (UI widget manager) + // to handle all of the messages that would have other wise been handled as regular execution messages for msg_id. + const parentHeader = 'msg_id' in msg.parent_header ? msg.parent_header : undefined; + if ( + this.ownedRequestMsgIds.has(msg.content.comm_id) || + (parentHeader && this.ownedRequestMsgIds.has(parentHeader.msg_id)) + ) { + if (data.state.msg_id) { + // Any future messages sent from `parent_header.msg_id = msg_id` must be handled by the widget with the `mode_id = msg.content.comm_id`. + this.outputsAreSpecificToAWidget = { + handlingCommId: msg.content.comm_id, + msgIdsToSwallow: data.state.msg_id + }; + } else if (this.outputsAreSpecificToAWidget?.handlingCommId === msg.content.comm_id) { + // Handle all messages the normal way. + this.outputsAreSpecificToAWidget = undefined; + } + } + } else if ( + 'children' in data.state && + Array.isArray(data.state.children) && + this.ownedCommIds.has(msg.content.comm_id) ) { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (output.data[WIDGET_MIMETYPE] as any)['_vsc_test_cellIndex'] = this.cell.index; + // This is the kernel instructing the widget manager that some outputs (comm_ids) + // are in fact children of another output (comm). + // We need to keep track of this so that we know who the common parent is. + const IPY_MODEL_PREFIX = 'IPY_MODEL_'; + data.state.children.forEach((item) => { + if (typeof item !== 'string') { + return traceWarning(`Came across a comm update message a child that isn't a string`, item); + } + if (!item.startsWith(IPY_MODEL_PREFIX)) { + return traceWarning( + `Came across a comm update message a child that start start with ${IPY_MODEL_PREFIX}`, + item + ); + } + const commId = item.substring(IPY_MODEL_PREFIX.length); + this.ownedCommIds.add(commId); + this.commIdsMappedToParentWidgetModel.set(commId, msg.content.comm_id); + }); + } + } + private clearOutputIfNecessary(execution: NotebookCellExecution | undefined): { + previousValueOfClearOutputOnNextUpdateToOutput: boolean; + } { + if (this.clearOutputOnNextUpdateToOutput) { + traceCellMessage(this.cell, 'Clear cell output'); + this.clearLastUsedStreamOutput(); + execution?.clearOutput().then(noop, noop); + this.clearOutputOnNextUpdateToOutput = false; + return { previousValueOfClearOutputOnNextUpdateToOutput: true }; + } + return { previousValueOfClearOutputOnNextUpdateToOutput: false }; + } + private addToCellData( + output: ExecuteResult | DisplayData | nbformat.IStream | nbformat.IError | nbformat.IOutput, + originalMessage: KernelMessage.IMessage + ) { + if (output.data && typeof output.data === 'object' && WIDGET_MIMETYPE in output.data) { + const widgetData = output.data[WIDGET_MIMETYPE] as { + version_major: number; + version_minor: number; + model_id: string; + }; + if (widgetData && this.context.extensionMode === ExtensionMode.Test) { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (widgetData as any)['_vsc_test_cellIndex'] = this.cell.index; + } + if (widgetData && 'model_id' in widgetData) { + const modelIds = CellExecutionMessageHandler.modelIdsOwnedByCells.get(this.cell) || new Set(); + modelIds.add(widgetData.model_id); + CellExecutionMessageHandler.modelIdsOwnedByCells.set(this.cell, modelIds); + } } const cellOutput = cellOutputToVSCCellOutput(output); const displayId = @@ -358,12 +498,8 @@ export class CellExecutionMessageHandler implements IDisposable { } traceCellMessage(this.cell, 'Update output'); // Clear if necessary - if (this.clearState.value) { - this.clearLastUsedStreamOutput(); - this.execution?.clearOutput().then(noop, noop); - this.clearState.update(false); - } - // Keep track of the displa_id against the output item, we might need this to update this later. + this.clearOutputIfNecessary(this.execution); + // Keep track of the display_id against the output item, we might need this to update this later. if (displayId) { this.outputDisplayIdTracker.trackOutputByDisplayId(this.cell, displayId, cellOutput); } @@ -375,10 +511,127 @@ export class CellExecutionMessageHandler implements IDisposable { const task = this.execution || this.createTemporaryTask(); this.clearLastUsedStreamOutput(); traceCellMessage(this.cell, 'Append output in addToCellData'); - task?.appendOutput([cellOutput]).then(noop, noop); + // If the output belongs to a widget, then add the output to that specific widget (i.e. just below the widget). + let outputShouldBeAppended = true; + const parentHeaderMsgId = getParentHeaderMsgId(originalMessage); + if ( + this.outputsAreSpecificToAWidget && + this.outputsAreSpecificToAWidget?.msgIdsToSwallow === parentHeaderMsgId && + cellOutput.items.every((item) => canMimeTypeBeRenderedByWidgetManager(item.mime)) + ) { + // Plain text outputs will be displayed by the widget. + outputShouldBeAppended = false; + } else if ( + this.outputsAreSpecificToAWidget && + this.outputsAreSpecificToAWidget?.msgIdsToSwallow === parentHeaderMsgId + ) { + const result = this.updateJupyterOutputWidgetWithOutput( + { commId: this.outputsAreSpecificToAWidget.handlingCommId, outputToAppend: cellOutput }, + task + ); + + if (result?.outputAdded) { + outputShouldBeAppended = false; + } + } + if (outputShouldBeAppended) { + task?.appendOutput([cellOutput]).then(noop, noop); + } this.endTemporaryTask(); } + /** + * Assume we have a Jupyter Output Widget, and we render that in Cell1. + * Now we add some output to that output from Cell2, general outputs will be automatically handled by the widget class. + * More complex outputs like plotly or the like, need to be handed by us (as Jupyter Output Widget expects Jupyter Notebooks/Jupyter Lab UI to handle that). + * To make this possible, we first need to find out the cell that owns the Output Widget, and then add that output to that particular cell. + * What we do in such a case is append this complex output just after the Output widget. + * + * In other cases, when appending such output, we need to clear the previous output that we added. + * + * This method is responsible for: + * - Finding the cell that owns the output widget + * - Clearing the custom output that was appended after the output widget. + * - Appending custom output after the output widget + */ + private updateJupyterOutputWidgetWithOutput( + options: { outputToAppend: NotebookCellOutput; commId: string } | { clearOutput: true }, + task?: NotebookCellExecution + ): { outputAdded: true } | undefined { + const commId = 'commId' in options ? options.commId : this.outputsAreSpecificToAWidget?.handlingCommId; + if (!commId) { + return; + } + const outputToAppend = 'outputToAppend' in options ? options.outputToAppend : undefined; + + const expectedModelId = this.commIdsMappedToParentWidgetModel.get(commId) || commId; + // Find the cell that owns the widget (where its model_id = commId). + const cell = this.cell.notebook + .getCells() + .find((cell) => CellExecutionMessageHandler.modelIdsOwnedByCells.get(cell)?.has(expectedModelId)); + if (!cell) { + traceWarning(`Unable to find a cell that owns the model ${expectedModelId}`); + return; + } + const widgetOutput = cell.outputs.find((output) => { + return output.items.find((outputItem) => { + if (outputItem.mime !== WIDGET_MIMETYPE) { + return false; + } + try { + const value = JSON.parse(Buffer.from(outputItem.data).toString()) as { model_id?: string }; + return value.model_id === expectedModelId; + } catch (ex) { + traceWarning(`Failed to deserialize the widget data`, ex); + } + return false; + }); + }); + if (!widgetOutput) { + return; + } + const outputsOwnedByWidgetModel = + CellExecutionMessageHandler.outputsOwnedByWidgetModel.get(expectedModelId) || new Set(); + + // We have some new outputs, that need to be placed immediately after the widget and before any other output + // that doesn't belong to the widget. + const clearWidgetOutput = this.outputsAreSpecificToAWidget?.clearOutputOnNextUpdateToOutput === true; + if (this.outputsAreSpecificToAWidget) { + this.outputsAreSpecificToAWidget.clearOutputOnNextUpdateToOutput = false; + } + const newOutputs = cell.outputs.slice().filter((item) => { + if (clearWidgetOutput) { + // If we're supposed to clear the output, then clear all of the output that's + // specific to this widget. + // These are tracked further below. + return !outputsOwnedByWidgetModel.has(item.id); + } else { + return true; + } + }); + + const outputsUptoWidget = newOutputs.slice(0, newOutputs.indexOf(widgetOutput) + 1); + const outputsAfterWidget = newOutputs.slice(newOutputs.indexOf(widgetOutput) + 1); + + CellExecutionMessageHandler.outputsOwnedByWidgetModel.set(expectedModelId, outputsOwnedByWidgetModel); + if (outputToAppend) { + // Keep track of the output added that belongs to the widget. + // Next time when we need to clear the output belonging to this widget, all we need to do is + // filter out (exclude) these outputs. + outputsOwnedByWidgetModel.add(outputToAppend.id); + } + // Ensure the new output is added just after the widget. + const newOutput = outputToAppend + ? outputsUptoWidget.concat(outputToAppend).concat(outputsAfterWidget) + : outputsUptoWidget.concat(outputsAfterWidget); + if (outputsAfterWidget.length === 0 && outputToAppend) { + // No need to replace everything, just append what we need. + task?.appendOutput(outputToAppend, cell).then(noop, noop); + } else { + task?.replaceOutput(newOutput, cell).then(noop, noop); + } + return { outputAdded: true }; + } private async handleInputRequest(msg: KernelMessage.IStdinMessage) { // Ask the user for input if (msg.content && 'prompt' in msg.content) { @@ -405,14 +658,17 @@ export class CellExecutionMessageHandler implements IDisposable { // See this for docs on the messages: // https://jupyter-client.readthedocs.io/en/latest/messaging.html#messaging-in-jupyter private handleExecuteResult(msg: KernelMessage.IExecuteResultMsg) { - this.addToCellData({ - output_type: 'execute_result', - data: msg.content.data, - metadata: msg.content.metadata, - // eslint-disable-next-line @typescript-eslint/no-explicit-any - transient: msg.content.transient as any, // NOSONAR - execution_count: msg.content.execution_count - }); + this.addToCellData( + { + output_type: 'execute_result', + data: msg.content.data, + metadata: msg.content.metadata, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + transient: msg.content.transient as any, // NOSONAR + execution_count: msg.content.execution_count + }, + msg + ); } private handleExecuteReply(msg: KernelMessage.IExecuteReplyMsg) { @@ -428,15 +684,18 @@ export class CellExecutionMessageHandler implements IDisposable { this.handleSetNextInput(payload as unknown as ISetNextInputPayload); } if (payload.data && payload.data.hasOwnProperty('text/plain')) { - this.addToCellData({ - // Mark as stream output so the text is formatted because it likely has ansi codes in it. - output_type: 'stream', - // eslint-disable-next-line @typescript-eslint/no-explicit-any - text: (payload.data as any)['text/plain'].toString(), - name: 'stdout', - metadata: {}, - execution_count: reply.execution_count - }); + this.addToCellData( + { + // Mark as stream output so the text is formatted because it likely has ansi codes in it. + output_type: 'stream', + // eslint-disable-next-line @typescript-eslint/no-explicit-any + text: (payload.data as any)['text/plain'].toString(), + name: 'stdout', + metadata: {}, + execution_count: reply.execution_count + }, + msg + ); } }); } @@ -477,6 +736,14 @@ export class CellExecutionMessageHandler implements IDisposable { traceCellMessage(this.cell, `Kernel switching to ${msg.content.execution_state}`); } private handleStreamMessage(msg: KernelMessage.IStreamMsg) { + if ( + getParentHeaderMsgId(msg) && + this.outputsAreSpecificToAWidget?.msgIdsToSwallow == getParentHeaderMsgId(msg) + ) { + // Stream messages will be handled by the widget output. + return; + } + // eslint-disable-next-line complexity traceCellMessage(this.cell, 'Update streamed output'); // Possible execution of cell has completed (the task would have been disposed). @@ -485,13 +752,7 @@ export class CellExecutionMessageHandler implements IDisposable { const task = this.execution || this.createTemporaryTask(); // Clear output if waiting for a clear - const clearOutput = this.clearState.value; - if (clearOutput) { - traceCellMessage(this.cell, 'Clear cell output'); - this.clearLastUsedStreamOutput(); - task?.clearOutput().then(noop, noop); - this.clearState.update(false); - } + const { previousValueOfClearOutputOnNextUpdateToOutput } = this.clearOutputIfNecessary(task); // Ensure we append to previous output, only if the streams as the same & // If the last output is the desired stream type. if (this.lastUsedStreamOutput?.stream === msg.content.name) { @@ -524,7 +785,7 @@ export class CellExecutionMessageHandler implements IDisposable { }); traceCellMessage(this.cell, `Replace output items ${this.lastUsedStreamOutput.text.substring(0, 100)}`); task?.replaceOutputItems(output.items, this.lastUsedStreamOutput.output).then(noop, noop); - } else if (clearOutput) { + } else if (previousValueOfClearOutputOnNextUpdateToOutput) { // Replace the current outputs with a single new output. const text = formatStreamText(concatMultilineString(msg.content.text)); const output = cellOutputToVSCCellOutput({ @@ -558,14 +819,28 @@ export class CellExecutionMessageHandler implements IDisposable { // eslint-disable-next-line @typescript-eslint/no-explicit-any transient: msg.content.transient as any // NOSONAR }; - this.addToCellData(output); + this.addToCellData(output, msg); } private handleClearOutput(msg: KernelMessage.IClearOutputMsg) { - // If the message says wait, add every message type to our clear state. This will - // make us wait for this type of output before we clear it. - if (msg && msg.content.wait) { - this.clearState.update(true); + // Check if this message should be handled by a specific Widget output. + if ( + this.outputsAreSpecificToAWidget && + this.outputsAreSpecificToAWidget.msgIdsToSwallow === getParentHeaderMsgId(msg) + ) { + if (msg.content.wait) { + this.outputsAreSpecificToAWidget.clearOutputOnNextUpdateToOutput = true; + } else { + const task = this.execution || this.createTemporaryTask(); + this.updateJupyterOutputWidgetWithOutput({ clearOutput: true }, task); + this.endTemporaryTask(); + } + return; + } + + // Regular output. + if (msg.content.wait) { + this.clearOutputOnNextUpdateToOutput = true; } else { // Possible execution of cell has completed (the task would have been disposed). // This message could have come from a background thread. @@ -590,7 +865,7 @@ export class CellExecutionMessageHandler implements IDisposable { traceback }; - this.addToCellData(output); + this.addToCellData(output, msg); this.cellHasErrorsInOutput = true; } diff --git a/src/platform/common/refBool.ts b/src/platform/common/refBool.ts deleted file mode 100644 index 22b0421b5e9..00000000000 --- a/src/platform/common/refBool.ts +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -export class RefBool { - constructor(private val: boolean) {} - - public get value(): boolean { - return this.val; - } - - public update(newVal: boolean) { - this.val = newVal; - } -} diff --git a/src/test/datascience/notebook/executionService.vscode.test.ts b/src/test/datascience/notebook/executionService.vscode.test.ts index a549cd4ef38..4713da541dd 100644 --- a/src/test/datascience/notebook/executionService.vscode.test.ts +++ b/src/test/datascience/notebook/executionService.vscode.test.ts @@ -99,6 +99,8 @@ suite('DataScience - VSCode Notebook - (Execution) (slow)', function () { await startJupyterServer(); await createEmptyPythonNotebook(disposables); assert.isOk(vscodeNotebook.activeNotebookEditor, 'No active notebook'); + // With less realestate, the outputs might not get rendered (VS Code optimization to avoid rendering if not in viewport). + await commands.executeCommand('workbench.action.closePanel'); traceInfo(`Start Test (completed) ${this.currentTest?.title}`); } catch (e) { await captureScreenShot(this.currentTest?.title || 'unknown'); @@ -371,6 +373,66 @@ suite('DataScience - VSCode Notebook - (Execution) (slow)', function () { await waitForExecutionCompletedSuccessfully(cell); }); + test('Clearing output immediately via code', async function () { + // Assume you are executing a cell that prints numbers 1-100. + // When printing number 50, you click clear. + // Cell output should now start printing output from 51 onwards, & not 1. + await insertCodeCell( + dedent` + from ipywidgets import widgets + from IPython.display import display, clear_output + import time + + display(widgets.Button(description="First Button")) + + time.sleep(2) + clear_output() + + display(widgets.Button(description="Second Button"))`, + { index: 0 } + ); + const cell = vscodeNotebook.activeNotebookEditor?.notebook.cellAt(0)!; + + await runAllCellsInActiveNotebook(); + + await Promise.all([ + waitForExecutionCompletedSuccessfully(cell), + waitForTextOutput(cell, 'Second Button', 0, false) + ]); + }); + test('Clearing output via code only when receiving new output', async function () { + // Assume you are executing a cell that prints numbers 1-100. + // When printing number 50, you click clear. + // Cell output should now start printing output from 51 onwards, & not 1. + await insertCodeCell( + dedent` + from ipywidgets import widgets + from IPython.display import display, clear_output + import time + + display(widgets.Button(description="First Button")) + + time.sleep(2) + clear_output(True) + + display(widgets.Button(description="Second Button"))`, + { index: 0 } + ); + const cell = vscodeNotebook.activeNotebookEditor?.notebook.cellAt(0)!; + + await runAllCellsInActiveNotebook(); + + // Wait for first button to appear then second. + await Promise.all([ + waitForExecutionCompletedSuccessfully(cell), + waitForTextOutput(cell, 'First Button', 0, false), + waitForTextOutput(cell, 'Second Button', 0, false) + ]); + + // Verify first is no longer visible and second is visible. + assert.notInclude(getCellOutputs(cell), 'First Button'); + assert.include(getCellOutputs(cell), 'Second Button'); + }); test('Shell commands should give preference to executables in Python Environment', async function () { if (IS_REMOTE_NATIVE_TEST()) { return this.skip(); diff --git a/src/test/datascience/widgets/notebooks/interactive_button.ipynb b/src/test/datascience/widgets/notebooks/interactive_button.ipynb new file mode 100644 index 00000000000..742dd5b15ab --- /dev/null +++ b/src/test/datascience/widgets/notebooks/interactive_button.ipynb @@ -0,0 +1,28 @@ +{ + "cells": [ + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "from IPython.display import display\n", + "from ipywidgets import widgets, interactive\n", + "button = widgets.Button(description=\"Click Me!\")\n", + "def on_button_clicked(b):\n", + " print(\"Button clicked\")\n", + "\n", + "display(button)\n", + "button.on_click(on_button_clicked)\n" + ] + } + ], + "metadata": { + "language_info": { + "name": "python" + }, + "orig_nbformat": 4 + }, + "nbformat": 4, + "nbformat_minor": 2 +} diff --git a/src/test/datascience/widgets/notebooks/interactive_function.ipynb b/src/test/datascience/widgets/notebooks/interactive_function.ipynb new file mode 100644 index 00000000000..468744abf36 --- /dev/null +++ b/src/test/datascience/widgets/notebooks/interactive_function.ipynb @@ -0,0 +1,28 @@ +{ + "cells": [ + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "from ipywidgets import widgets, interact\n", + "def do_something(filter_text):\n", + " print(f\"Executing do_something with '{filter_text}'\")\n", + " return filter_text\n", + " \n", + " \n", + "interact(do_something, filter_text=widgets.Text(value='Foo'))\n", + "do_something('Hello World')" + ] + } + ], + "metadata": { + "language_info": { + "name": "python" + }, + "orig_nbformat": 4 + }, + "nbformat": 4, + "nbformat_minor": 2 +} diff --git a/src/test/datascience/widgets/notebooks/interactive_plot.ipynb b/src/test/datascience/widgets/notebooks/interactive_plot.ipynb new file mode 100644 index 00000000000..0401774694f --- /dev/null +++ b/src/test/datascience/widgets/notebooks/interactive_plot.ipynb @@ -0,0 +1,60 @@ +{ + "cells": [ + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "# Since we don't want to install plotly on the CI, we can test with a custom mime type.\n", + "# The mime type application/vnd.custom should end up getting rendered as an individual output in the cell (instead of letting the Output Widget handle it - which it cannot).\n", + "\n", + "import ipywidgets as widgets\n", + "from ipywidgets import interact\n", + "import IPython.display\n", + "\n", + "\n", + "def update_graph(txt):\n", + " print(f\"Text Value is {txt}\")\n", + " display({\"application/vnd.custom\": f\"Text Value is {txt}\"}, raw=True)\n", + "\n", + "\n", + "text = widgets.Text(value='Foo')\n", + "interact(update_graph, txt=text)\n", + "update_graph(\"Hello World\")\n" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "# This is a real world example.\n", + "\n", + "import ipywidgets as widgets\n", + "from ipywidgets import interact\n", + "import pandas as pd\n", + "import plotly.express as px\n", + "pd.options.plotting.backend = \"plotly\"\n", + "\n", + "def update_graph(rng):\n", + " df = pd.DataFrame({\"x\": list(range(*rng)), \"y\": list(range(*rng))})\n", + " fig = df.plot.scatter(x=\"x\", y=\"y\")\n", + " fig.show()\n", + "\n", + "slider=widgets.IntRangeSlider(value=(0, 10), min=0, max=20, description='Hello')\n", + "interact(update_graph, rng=slider)\n", + "update_graph([0, 10])\n" + ] + } + ], + "metadata": { + "language_info": { + "name": "python" + }, + "orig_nbformat": 4 + }, + "nbformat": 4, + "nbformat_minor": 2 +} diff --git a/src/test/datascience/widgets/notebooks/interactive_plot_another_cell.ipynb b/src/test/datascience/widgets/notebooks/interactive_plot_another_cell.ipynb new file mode 100644 index 00000000000..9ec539dc577 --- /dev/null +++ b/src/test/datascience/widgets/notebooks/interactive_plot_another_cell.ipynb @@ -0,0 +1,84 @@ +{ + "cells": [ + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "import ipywidgets as widgets\n", + "from ipywidgets import interact\n", + "# import pandas as pd\n", + "# import plotly.express as px\n", + "# pd.options.plotting.backend = \"plotly\"\n", + "\n", + "from ipywidgets import interact, interactive, fixed, interact_manual\n", + "import ipywidgets as widgets\n", + "from IPython.display import display\n", + "\n", + "out = widgets.Output()\n", + "out" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "@out.capture()\n", + "def function_with_captured_output():\n", + " print('First output widget')\n", + "\n", + "function_with_captured_output()" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "out2 = widgets.Output()\n", + "caption = widgets.Label(value='Label Widget')\n", + "textbox = widgets.Text()\n", + "widgets.link((caption, 'value'), (textbox, 'value'))\n", + "\n", + "@out.capture()\n", + "def function_with_captured_output():\n", + " display(out2)\n", + " display(caption)\n", + " display(textbox)\n", + " # Output a custom mime type instead of a plotly plot so that we don't have to install plotly on CI.\n", + " display({\"application/vnd.custom\": f\"Custom Mime Output under the `output widget`\"}, raw=True)\n", + " # df = pd.DataFrame({\"x\": list(range(10)), \"y\": list(range(10))})\n", + " # fig = df.plot.scatter(x=\"x\", y=\"y\")\n", + " # fig.show()\n", + "\n", + "\n", + "function_with_captured_output()" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "@out2.capture()\n", + "def function_with_captured_output2():\n", + " print('Second output widget')\n", + "\n", + "function_with_captured_output2()" + ] + } + ], + "metadata": { + "language_info": { + "name": "python" + }, + "orig_nbformat": 4 + }, + "nbformat": 4, + "nbformat_minor": 2 +} diff --git a/src/test/datascience/widgets/notebooks/nested_output_widget.ipynb b/src/test/datascience/widgets/notebooks/nested_output_widget.ipynb new file mode 100644 index 00000000000..820adbbc1ba --- /dev/null +++ b/src/test/datascience/widgets/notebooks/nested_output_widget.ipynb @@ -0,0 +1,72 @@ +{ + "cells": [ + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "from ipywidgets import interact, interactive, fixed, interact_manual\n", + "import ipywidgets as widgets\n", + "from IPython.display import display\n", + "\n", + "out = widgets.Output()\n", + "out" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "@out.capture()\n", + "def function_with_captured_output():\n", + " print('First output widget')\n", + "\n", + "function_with_captured_output()" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "out2 = widgets.Output()\n", + "caption = widgets.Label(value='Label Widget')\n", + "textbox = widgets.Text()\n", + "widgets.link((caption, 'value'), (textbox, 'value'))\n", + "\n", + "@out.capture()\n", + "def function_with_captured_output():\n", + " display(out2)\n", + " display(caption)\n", + " display(textbox)\n", + "\n", + "function_with_captured_output()" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "@out2.capture()\n", + "def function_with_captured_output2():\n", + " print('Second output widget')\n", + "\n", + "function_with_captured_output2()" + ] + } + ], + "metadata": { + "language_info": { + "name": "python" + }, + "orig_nbformat": 4 + }, + "nbformat": 4, + "nbformat_minor": 2 +} diff --git a/src/test/datascience/widgets/standard.vscode.common.test.ts b/src/test/datascience/widgets/standard.vscode.common.test.ts index cc3dec9a71b..1931933f60a 100644 --- a/src/test/datascience/widgets/standard.vscode.common.test.ts +++ b/src/test/datascience/widgets/standard.vscode.common.test.ts @@ -22,6 +22,7 @@ import { defaultNotebookTestTimeout, prewarmNotebooks, runCell, + waitForCellExecutionToComplete, waitForExecutionCompletedSuccessfully, waitForKernelToGetAutoSelected, waitForTextOutput, @@ -29,6 +30,7 @@ import { } from '../notebook/helper'; import { initializeWidgetComms, Utils } from './commUtils'; import { WidgetRenderingTimeoutForTests } from './constants'; +import { getTextOutputValue } from '../../../kernels/execution/helpers'; /* eslint-disable @typescript-eslint/no-explicit-any, no-invalid-this */ suite('IPyWisdget Tests', function () { @@ -73,6 +75,7 @@ suite('IPyWisdget Tests', function () { await startJupyterServer(); await closeNotebooks(); traceInfo(`Start Test (completed) ${this.currentTest?.title}`); + // With less realestate, the outputs might not get rendered (VS Code optimization to avoid rendering if not in viewport). await commands.executeCommand('workbench.action.closePanel'); }); teardown(async function () { @@ -395,4 +398,123 @@ suite('IPyWisdget Tests', function () { await executeCellAndWaitForOutput(cell, comms); await assertOutputContainsHtml(cell, comms, ['66'], '.widget-readout'); }); + test('Nested Output Widgets', async () => { + const comms = await initializeNotebook({ templateFile: 'nested_output_widget.ipynb' }); + const [cell1, cell2, cell3, cell4] = vscodeNotebook.activeNotebookEditor!.notebook.getCells(); + await executeCellAndWaitForOutput(cell1, comms); + + // Run the second cell & verify we have output in the first cell. + await Promise.all([runCell(cell2), waitForCellExecutionToComplete(cell1)]); + await assertOutputContainsHtml(cell1, comms, ['First output widget'], '.widget-output'); + + // Run the 3rd cell to add a nested output. + // Also display the same nested output and the widget in the 3rd cell. + await Promise.all([runCell(cell3), waitForCellExecutionToComplete(cell3)]); + await assertOutputContainsHtml(cell1, comms, ['Executing do_something with 'Foo'", + ">'Foo'" + ]); + await waitForCondition(() => cell.outputs.length >= 3, 5_000, 'Cell must have 3 outputs'); + assert.strictEqual(getTextOutputValue(cell.outputs[1]).trim(), `Executing do_something with 'Hello World'`); + assert.strictEqual(getTextOutputValue(cell.outputs[2]).trim(), `'Hello World'`); + + // Update the textbox and confirm the output is updated accordingly. + await comms.setValue(cell, '.widget-text input', 'Bar'); + await assertOutputContainsHtml(cell, comms, [ + 'Executing do_something with 'Bar'", + ">'Bar'" + ]); + assert.strictEqual(getTextOutputValue(cell.outputs[1]).trim(), `Executing do_something with 'Hello World'`); + assert.strictEqual(getTextOutputValue(cell.outputs[2]).trim(), `'Hello World'`); + }); + test('Interactive Plot', async () => { + const comms = await initializeNotebook({ templateFile: 'interactive_plot.ipynb' }); + const cell = vscodeNotebook.activeNotebookEditor!.notebook.cellAt(0); + + await executeCellAndWaitForOutput(cell, comms); + await assertOutputContainsHtml(cell, comms, ['Text Value is Foo']); + assert.strictEqual(cell.outputs.length, 4, 'Cell should have 4 outputs'); + + // This cannot be displayed by output widget, hence we need to handle this. + assert.strictEqual(cell.outputs[1].items[0].mime, 'application/vnd.custom'); + assert.strictEqual(Buffer.from(cell.outputs[1].items[0].data).toString(), 'Text Value is Foo'); + + assert.strictEqual(getTextOutputValue(cell.outputs[2]).trim(), 'Text Value is Hello World'); + + // This cannot be displayed by output widget, hence we need to handle this. + assert.strictEqual(cell.outputs[3].items[0].mime, 'application/vnd.custom'); + assert.strictEqual(Buffer.from(cell.outputs[3].items[0].data).toString().trim(), 'Text Value is Hello World'); + + // Wait for the second output to get updated. + const outputUpdated = new Promise((resolve) => { + workspace.onDidChangeNotebookDocument( + (e) => { + const currentCellChange = e.cellChanges.find((item) => item.cell === cell); + if (!currentCellChange || !currentCellChange.outputs || currentCellChange.outputs.length < 4) { + return; + } + const secondOutput = currentCellChange.outputs[1]; + if (Buffer.from(secondOutput.items[0].data).toString() === 'Text Value is Bar') { + resolve(true); + } + }, + undefined, + disposables + ); + }); + // Update the textbox and confirm the output is updated accordingly. + await comms.setValue(cell, '.widget-text input', 'Bar'); + + // Wait for the output to get updated. + await waitForCondition(() => outputUpdated, 5_000, 'Second output not updated'); + + // The first & second outputs should have been updated + await assertOutputContainsHtml(cell, comms, ['Text Value is Bar']); + assert.strictEqual(cell.outputs[1].items[0].mime, 'application/vnd.custom'); + assert.strictEqual(Buffer.from(cell.outputs[1].items[0].data).toString().trim(), 'Text Value is Bar'); + + // The last two should not have changed. + assert.strictEqual(getTextOutputValue(cell.outputs[2]).trim(), 'Text Value is Hello World'); + assert.strictEqual(cell.outputs[3].items[0].mime, 'application/vnd.custom'); + assert.strictEqual(Buffer.from(cell.outputs[3].items[0].data).toString().trim(), 'Text Value is Hello World'); + }); }); diff --git a/src/webviews/webview-side/ipywidgets/kernel/index.ts b/src/webviews/webview-side/ipywidgets/kernel/index.ts index 26c11a533af..b83310d6df7 100644 --- a/src/webviews/webview-side/ipywidgets/kernel/index.ts +++ b/src/webviews/webview-side/ipywidgets/kernel/index.ts @@ -81,7 +81,7 @@ class WidgetManagerComponent { const outputDisposables = new Map(); const htmlDisposables = new WeakMap(); -const renderedWidgets = new Set(); +const renderedWidgets = new Map(); /** * Called from renderer to render output. * This will be exposed as a public method on window for renderer to render output. @@ -109,9 +109,14 @@ export function renderOutput(outputItem: OutputItem, element: HTMLElement, logge } export function disposeOutput(outputId?: string) { if (outputId) { + const widget = renderedWidgets.get(outputId)?.widget; + renderedWidgets.delete(outputId); stackOfWidgetsRenderStatusByOutputId = stackOfWidgetsRenderStatusByOutputId.filter( (item) => !(outputId in item) ); + if (widget) { + widget.dispose(); + } } } function renderIPyWidget( @@ -140,9 +145,12 @@ function renderIPyWidget( ele.className = 'cell-output-ipywidget-background'; container.appendChild(ele); ele.appendChild(output); - renderedWidgets.add(outputId); + renderedWidgets.set(outputId, {}); createWidgetView(model, ele) .then((w) => { + if (renderedWidgets.has(outputId)) { + renderedWidgets.get(outputId)!.widget = w; + } const disposable = { dispose: () => { // What if we render the same model in two cells.