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

Ignore output messages from comm_msg #16169

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/kernels/execution/cellExecutionMessageHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,12 @@ export class CellExecutionMessageHandler implements IDisposable {
CellExecutionMessageHandler.modelIdsOwnedByCells.set(this.cell, modelIds);
}
}
if (originalMessage.parent_header.msg_type === 'comm_msg') {
// Outputs generated as part of a comm message are not displayed in the cell output.
// See https://github.com/microsoft/vscode-jupyter/issues/15996
return;
}

const cellOutput = cellOutputToVSCCellOutput(output);
const displayId =
'transient' in output &&
Expand Down Expand Up @@ -946,6 +952,11 @@ export class CellExecutionMessageHandler implements IDisposable {
// Stream messages will be handled by the Output Widget.
return;
}
if (msg.parent_header.msg_type === 'comm_msg') {
// Outputs generated as part of a comm message are not displayed in the cell output.
// See https://github.com/microsoft/vscode-jupyter/issues/15996
return;
}

// eslint-disable-next-line complexity
traceCellMessage(this.cell, `Update streamed output, new output '${msg.content.text.substring(0, 100)}'`);
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,26 @@
"outputs": [],
"source": [
"%matplotlib widget\n",
"from IPython.display import display\n",
"from ipywidgets import widgets\n",
"import matplotlib.pyplot as plt\n",
"import numpy as np\n",
"\n",
"\n",
"button = widgets.Button(description=\"Click Me!\")\n",
"output = widgets.Output()\n",
"\n",
"def on_button_clicked(b):\n",
"\tX = np.linspace(0, 2*np.pi)\n",
"\tY = np.sin(X)\n",
"\twith output:\n",
"\t\tX = np.linspace(0, 2*np.pi)\n",
"\t\tY = np.sin(X)\n",
"\n",
"\t_, ax = plt.subplots()\n",
"\tax.plot(X, Y)\n",
"\t\tfig, ax = plt.subplots()\n",
"\t\tax.plot(X, Y)\n",
"\t\tplt.show(fig)\n",
"\n",
"display(button)\n",
"button.on_click(on_button_clicked)\n"
"button.on_click(on_button_clicked)\n",
"\n",
"output"
]
},
{
Expand All @@ -32,7 +35,7 @@
"metadata": {},
"outputs": [],
"source": [
"display(button)"
"button"
]
}
],
Expand Down
28 changes: 0 additions & 28 deletions src/test/datascience/widgets/notebooks/interactive_button.ipynb

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,15 @@
"outputs": [],
"source": [
"from ipywidgets import widgets, interact\n",
"output = widgets.Output()\n",
"display(output)\n",
"\n",
"def do_something(filter_text):\n",
" print(f\"Executing do_something with '{filter_text}'\")\n",
" with output:\n",
" print(f\"Executing do_something with '{filter_text}'\")\n",
" return filter_text\n",
" \n",
" \n",
"\n",
"\n",
"interact(do_something, filter_text=widgets.Text(value='Foo'))\n",
"do_something('Hello World')"
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,7 @@ import {
runCell,
selectDefaultController,
waitForCellExecutionToComplete,
waitForExecutionCompletedSuccessfully,
waitForTextOutput
waitForExecutionCompletedSuccessfully
} from '../notebook/helper';
import { initializeWidgetComms, Utils } from './commUtils';
import { WidgetRenderingTimeoutForTests } from './constants';
Expand Down Expand Up @@ -237,24 +236,6 @@ suite('Standard IPyWidget Tests @widgets', function () {
await assertOutputContainsHtml(cell1, comms, ['Button clicked']);
await assertOutputContainsHtml(cell2, comms, ['Button clicked']);
});
test('Button Widget with custom comm message', async () => {
await initializeNotebookForWidgetTest(
disposables,
{
templateFile: 'button_widget_comm_msg.ipynb'
},
editor
);
const [cell0, cell1] = window.activeNotebookEditor!.notebook.getCells();

await executeCellAndWaitForOutput(cell0, comms);
await executeCellAndWaitForOutput(cell1, comms);
await assertOutputContainsHtml(cell0, comms, ['Click Me!', '<button']);

// Click the button and verify we have output in the same cell.
await clickWidget(comms, cell0, 'button');
await waitForTextOutput(cell0, 'Button clicked.', 1, false);
});
test.skip('Widget renders after executing a notebook which was saved after previous execution', async () => {
// // https://github.com/microsoft/vscode-jupyter/issues/8748
// await initializeNotebookForWidgetTest(disposables, { templateFile: 'standard_widgets.ipynb' }, editor);
Expand Down Expand Up @@ -482,33 +463,6 @@ suite('Standard IPyWidget Tests @widgets', function () {
() => `Output doesn't contain text 'Bar' or still contains 'Outside, Inside, Foo', html is ${html}`
);
});
test('Interactive Button', async () => {
await initializeNotebookForWidgetTest(
disposables,
{
templateFile: 'interactive_button.ipynb'
},
editor
);
const cell = window.activeNotebookEditor!.notebook.cellAt(0);

await executeCellAndWaitForOutput(cell, comms);
await assertOutputContainsHtml(cell, comms, ['Click Me!', '<button']);

// Click the button and verify we have output in other cells
await clickWidget(comms, cell, 'button');
await waitForCondition(
() => {
assert.strictEqual(getTextOutputValue(cell.outputs[1]).trim(), 'Button clicked');
return true;
},
5_000,
() =>
`Expected 'Button clicked' to exist in ${
cell.outputs.length > 1 ? getTextOutputValue(cell.outputs[1]) : '<Only one output>'
}`
);
});
test('Interactive Function', async () => {
await initializeNotebookForWidgetTest(
disposables,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,10 @@ import { isWeb } from '../../../platform/common/utils/misc';

await executeCellAndWaitForOutput(cell0, comms);
await executeCellAndWaitForOutput(cell1, comms);
await assertOutputContainsHtml(cell0, comms, ['Click Me!', '<button']);
await assertOutputContainsHtml(cell1, comms, ['Click Me!', '<button']);

// Click the button and verify we have output in the same cell.
await clickWidget(comms, cell0, 'button');
await clickWidget(comms, cell1, 'button');
await assertOutputContainsHtml(cell0, comms, ['>Figure 1<', '<canvas', 'Download plot']);
});
test('Render AnyWidget (test js<-->kernel comms with binary data)', async function () {
Expand Down
Loading