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

Enable plot viewer in Native Notebooks #6640

Merged
merged 3 commits into from
Jul 12, 2021
Merged
Show file tree
Hide file tree
Changes from 2 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
1 change: 0 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -652,7 +652,6 @@ module.exports = {
'src/client/common/startPage/startPage.ts',
'src/client/common/startPage/types.ts',
'src/client/common/startPage/startPageMessageListener.ts',
'src/client/common/application/applicationShell.ts',
'src/client/common/application/languageService.ts',
'src/client/common/application/',
'src/client/common/application/clipboard.ts',
Expand Down
1 change: 1 addition & 0 deletions news/2 Fixes/6315.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Restore plotviewer in Native Notebooks.
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -1959,6 +1959,7 @@
"image/gif",
"image/png",
"image/jpeg",
"image/svg+xml",
"text/latex",
"text/vnd.plotly.v1+html"
]
Expand Down
4 changes: 4 additions & 0 deletions src/client/common/application/applicationShell.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { injectable } from 'inversify';
import {
CancellationToken,
CancellationTokenSource,
ColorTheme,
Disposable,
env,
Event,
Expand Down Expand Up @@ -37,6 +38,9 @@ import { IApplicationShell } from './types';

@injectable()
export class ApplicationShell implements IApplicationShell {
public get activeColorTheme(): ColorTheme {
return window.activeColorTheme;
}
public get onDidChangeWindowState(): Event<WindowState> {
return window.onDidChangeWindowState;
}
Expand Down
4 changes: 3 additions & 1 deletion src/client/common/application/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@ import {
NotebookCell,
NotebookSerializer,
NotebookData,
NotebookDocumentShowOptions
NotebookDocumentShowOptions,
ColorTheme
} from 'vscode';
import * as vsls from 'vsls/vscode';

Expand All @@ -82,6 +83,7 @@ import { ICommandNameArgumentTypeMapping } from './commands';

export const IApplicationShell = Symbol('IApplicationShell');
export interface IApplicationShell {
readonly activeColorTheme: ColorTheme;
/**
* An [event](#Event) which fires when the focus state of the current window
* changes. The value of the event represents whether the window is focused.
Expand Down
73 changes: 60 additions & 13 deletions src/client/datascience/jupyter/kernels/kernel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { Subject } from 'rxjs/Subject';
import * as uuid from 'uuid/v4';
import {
CancellationTokenSource,
ColorThemeKind,
Event,
EventEmitter,
NotebookCell,
Expand All @@ -21,14 +22,14 @@ import {
} from 'vscode';
import { ServerStatus } from '../../../../datascience-ui/interactive-common/mainState';
import { IApplicationShell } from '../../../common/application/types';
import { traceError, traceInfo, traceWarning } from '../../../common/logger';
import { traceError, traceInfo, traceInfoIf, traceWarning } from '../../../common/logger';
import { IFileSystem } from '../../../common/platform/types';
import { IDisposableRegistry, IExtensionContext } from '../../../common/types';
import { IConfigurationService, IDisposableRegistry, IExtensionContext } from '../../../common/types';
import { createDeferred, Deferred } from '../../../common/utils/async';
import { noop } from '../../../common/utils/misc';
import { StopWatch } from '../../../common/utils/stopWatch';
import { sendTelemetryEvent } from '../../../telemetry';
import { CodeSnippets, Telemetry } from '../../constants';
import { CodeSnippets, Identifiers, Telemetry } from '../../constants';
import { sendKernelTelemetryEvent, trackKernelResourceInformation } from '../../telemetry/telemetry';
import { getNotebookMetadata } from '../../notebook/helpers/helpers';
import {
Expand All @@ -45,7 +46,7 @@ import { getSysInfoReasonHeader, isPythonKernelConnection } from './helpers';
import { KernelExecution } from './kernelExecution';
import type { IKernel, IKernelProvider, KernelConnectionMetadata } from './types';
import { SysInfoReason } from '../../interactive-common/interactiveWindowTypes';
import { MARKDOWN_LANGUAGE } from '../../../common/constants';
import { isCI, MARKDOWN_LANGUAGE } from '../../../common/constants';
import { InteractiveWindowView } from '../../notebook/constants';
import { chainWithPendingUpdates } from '../../notebook/helpers/notebookUpdater';

Expand Down Expand Up @@ -96,11 +97,12 @@ export class Kernel implements IKernel {
private readonly errorHandler: IDataScienceErrorHandler,
private readonly editorProvider: INotebookEditorProvider,
kernelProvider: IKernelProvider,
appShell: IApplicationShell,
private readonly appShell: IApplicationShell,
private readonly fs: IFileSystem,
context: IExtensionContext,
private readonly serverStorage: IJupyterServerUriStorage,
controller: NotebookController
controller: NotebookController,
private readonly configService: IConfigurationService
) {
this.kernelExecution = new KernelExecution(
kernelProvider,
Expand Down Expand Up @@ -285,7 +287,6 @@ export class Kernel implements IKernel {
editor.notebook = this.notebook;
}

this.disableJedi();
if (!this.hookedNotebookForEvents.has(this.notebook)) {
this.hookedNotebookForEvents.add(this.notebook);
this.notebook.kernelSocket.subscribe(this._kernelSocket);
Expand All @@ -308,7 +309,9 @@ export class Kernel implements IKernel {
);
}
if (isPythonKernelConnection(this.kernelConnectionMetadata)) {
await this.disableJedi();
await this.notebook.setLaunchingFile(this.uri.fsPath);
await this.initializeMatplotlib();
}
await this.notebook
.requestKernelInfo()
Expand All @@ -320,10 +323,8 @@ export class Kernel implements IKernel {
await this.notebook.waitForIdle(this.launchTimeout);
}

private disableJedi() {
if (isPythonKernelConnection(this.kernelConnectionMetadata) && this.notebook) {
this.notebook.executeObservable(CodeSnippets.disableJedi, this.uri.fsPath, 0, uuid(), true);
}
private async disableJedi() {
await this.executeSilently(CodeSnippets.disableJedi);
}

/**
Expand All @@ -344,7 +345,8 @@ export class Kernel implements IKernel {
}

const message = getSysInfoReasonHeader(reason, this.kernelConnectionMetadata);
const sysInfoMessages = [(info.content as KernelMessage.IInfoReply)?.banner.split('\n').join('\n\n')];
const bannerMessage = (info.content as KernelMessage.IInfoReply)?.banner || '';
const sysInfoMessages = bannerMessage ? bannerMessage.split('\n') : [];
if (sysInfoMessages) {
// Connection string only for our initial start, not restart or interrupt
let connectionString: string = '';
Expand All @@ -362,7 +364,7 @@ export class Kernel implements IKernel {
return chainWithPendingUpdates(notebookDocument, (edit) => {
const markdownCell = new NotebookCellData(
NotebookCellKind.Markup,
sysInfoMessages.join('\n\n'),
sysInfoMessages.join(' \n'),
MARKDOWN_LANGUAGE
);
markdownCell.metadata = { isSysInfoCell: true };
Expand All @@ -374,4 +376,49 @@ export class Kernel implements IKernel {
});
}
}
private async initializeMatplotlib(): Promise<void> {
if (!this.notebook) {
return;
}
const settings = this.configService.getSettings(this.uri);
if (settings && settings.themeMatplotlibPlots) {
const matplobInit = settings.enablePlotViewer
? CodeSnippets.MatplotLibInitSvg
: CodeSnippets.MatplotLibInitPng;

traceInfo(`Initialize matplotlib for ${this.uri.toString()}`);
await this.executeSilently(matplobInit);
const useDark = this.appShell.activeColorTheme.kind === ColorThemeKind.Dark;
if (!settings.ignoreVscodeTheme) {
// Reset the matplotlib style based on if dark or not.
await this.executeSilently(
useDark
? "matplotlib.style.use('dark_background')"
: `matplotlib.rcParams.update(${Identifiers.MatplotLibDefaultParams})`
);
}
} else {
const configInit = !settings || settings.enablePlotViewer ? CodeSnippets.ConfigSvg : CodeSnippets.ConfigPng;
traceInfoIf(isCI, `Initialize config for plots for ${this.uri.toString()}`);
await this.executeSilently(configInit);
}
}
private async executeSilently(code: string) {
if (!this.notebook) {
return;
}
// Force matplotlib to inline and save the default style. We'll use this later if we
DonJayamanne marked this conversation as resolved.
Show resolved Hide resolved
// get a request to update style
const deferred = createDeferred<void>();
const observable = this.notebook.executeObservable(code, this.uri.fsPath, 0, uuid(), true);
const subscription = observable.subscribe(
noop,
(ex) => deferred.reject(ex),
() => deferred.resolve()
);
this.disposables.push({
dispose: () => subscription.unsubscribe()
});
await deferred.promise;
}
}
3 changes: 2 additions & 1 deletion src/client/datascience/jupyter/kernels/kernelProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ export class KernelProvider implements IKernelProvider {
this.fs,
this.context,
this.serverStorage,
options.controller
options.controller,
this.configService
);
this.asyncDisposables.push(kernel);
this.kernelsByUri.set(uri.toString(), { options, kernel });
Expand Down
8 changes: 8 additions & 0 deletions src/client/datascience/notebook/helpers/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,10 @@ function translateDisplayDataOutput(
}
*/
const metadata = getOutputMetadata(output);
// If we have both SVG & PNG, then add special metadata to indicate whether to display `open plot`
if ('image/svg+xml' in output.data && 'image/png' in output.data) {
metadata.__displayOpenPlotIcon = true;
}
const items: NotebookCellOutputItem[] = [];
// eslint-disable-next-line
const data: Record<string, any> = output.data || {};
Expand Down Expand Up @@ -525,6 +529,10 @@ export type CellOutputMetadata = {
* (this is something we have added)
*/
__isJson?: boolean;
/**
* Whether to display the open plot icon.
*/
__displayOpenPlotIcon?: boolean;
};

export function translateCellErrorOutput(output: NotebookCellOutput): nbformat.IError {
Expand Down
34 changes: 34 additions & 0 deletions src/client/datascience/notebook/outputs/plotViewHandler.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { inject, injectable } from 'inversify';
import { NotebookCellOutputItem, NotebookEditor } from 'vscode';
import { traceError } from '../../../common/logger';
import { IPlotViewerProvider } from '../../types';

const svgMimeType = 'image/svg+xml';

@injectable()
export class PlotViewHandler {
constructor(@inject(IPlotViewerProvider) private readonly plotViewProvider: IPlotViewerProvider) {}

public async openPlot(editor: NotebookEditor, outputId: string) {
if (editor.document.isClosed) {
return;
}
const outputItem = getOutputItem(editor, outputId, svgMimeType);
if (!outputItem) {
return traceError(`No SVG Plot to open ${editor.document.uri.toString()}, id: ${outputId}`);
}
const svg = new TextDecoder().decode(outputItem.data);
await this.plotViewProvider.showPlot(svg);
}
}

function getOutputItem(editor: NotebookEditor, outputId: string, mimeType: string): NotebookCellOutputItem | undefined {
for (const cell of editor.document.getCells()) {
for (const output of cell.outputs) {
if (output.id !== outputId) {
continue;
}
return output.items.find((item) => item.mime === mimeType);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { IDisposable } from '../../../common/types';
import { noop } from '../../../common/utils/misc';
import { JupyterNotebookRenderer } from '../constants';
import { PlotSaveHandler } from './plotSaveHandler';
import { PlotViewHandler } from './plotViewHandler';

type RendererMessageTypes = { type: 'saveAs'; outputId: string; mimeType: string };

Expand All @@ -18,7 +19,8 @@ export class RendererCommunication implements IExtensionSyncActivationService, I
private readonly disposables: IDisposable[] = [];
constructor(
@inject(UseVSCodeNotebookEditorApi) private readonly useVSCNotebook: boolean,
@inject(PlotSaveHandler) private readonly plotSaveHandler: PlotSaveHandler
@inject(PlotSaveHandler) private readonly plotSaveHandler: PlotSaveHandler,
@inject(PlotViewHandler) private readonly plotViewHandler: PlotViewHandler
) {}

public dispose() {
Expand All @@ -34,6 +36,8 @@ export class RendererCommunication implements IExtensionSyncActivationService, I
({ editor, message }) => {
if (message.type === 'saveAs') {
this.plotSaveHandler.savePlot(editor, message.outputId, message.mimeType).catch(noop);
} else if (message.type === 'openPlot') {
this.plotViewHandler.openPlot(editor, message.outputId).catch(noop);
}
},
this,
Expand Down
2 changes: 2 additions & 0 deletions src/client/datascience/notebook/serviceRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { RemoteSwitcher } from './remoteSwitcher';
import { INotebookControllerManager } from './types';
import { RendererCommunication } from './outputs/rendererCommunication';
import { PlotSaveHandler } from './outputs/plotSaveHandler';
import { PlotViewHandler } from './outputs/plotViewHandler';

export function registerTypes(serviceManager: IServiceManager) {
serviceManager.addSingleton<NotebookSerializer>(NotebookSerializer, NotebookSerializer);
Expand Down Expand Up @@ -58,6 +59,7 @@ export function registerTypes(serviceManager: IServiceManager) {
serviceManager.addSingleton<NotebookCreator>(NotebookCreator, NotebookCreator);
serviceManager.addSingleton<INotebookControllerManager>(INotebookControllerManager, NotebookControllerManager);
serviceManager.addSingleton<PlotSaveHandler>(PlotSaveHandler, PlotSaveHandler);
serviceManager.addSingleton<PlotViewHandler>(PlotViewHandler, PlotViewHandler);
serviceManager.addSingleton<IExtensionSyncActivationService>(
IExtensionSyncActivationService,
RendererCommunication
Expand Down