diff --git a/news/1 Enhancements/6913.md b/news/1 Enhancements/6913.md new file mode 100644 index 00000000000..335a05fcade --- /dev/null +++ b/news/1 Enhancements/6913.md @@ -0,0 +1 @@ +Default plot output to just PNG, and support showing PNGs or SVGs in the Plot Viewer control. The enablePlotViewer setting still turns on both PNG and SVG plot output, but it's now off by default, not on. \ No newline at end of file diff --git a/package-lock.json b/package-lock.json index 50a11c6a5ca..d73ce9f466c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13548,11 +13548,22 @@ } }, "image-size": { - "version": "0.5.5", - "resolved": "https://registry.npmjs.org/image-size/-/image-size-0.5.5.tgz", - "integrity": "sha1-Cd/Uq50g4p6xw+gLiZA3jfnjy5w=", - "dev": true, - "optional": true + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/image-size/-/image-size-1.0.0.tgz", + "integrity": "sha512-JLJ6OwBfO1KcA+TvJT+v8gbE6iWbj24LyDNFgFEN0lzegn6cC6a/p3NIDaepMsJjQjlUWqIC7wJv8lBFxPNjcw==", + "requires": { + "queue": "6.0.2" + }, + "dependencies": { + "queue": { + "version": "6.0.2", + "resolved": "https://registry.npmjs.org/queue/-/queue-6.0.2.tgz", + "integrity": "sha512-iHZWu+q3IdFZFX36ro/lKBkSvfkztY5Y7HMiPlOUjhupPcG2JMfst2KKEpu5XndviX/3UhFbRngUPNKtgvtZiA==", + "requires": { + "inherits": "~2.0.3" + } + } + } }, "immutable": { "version": "4.0.0-rc.12", @@ -15079,6 +15090,13 @@ "resolved": "https://registry.npmjs.org/clone/-/clone-2.1.2.tgz", "integrity": "sha1-G39Ln1kfHo+DZwQBYANFoCiHQ18=", "dev": true + }, + "image-size": { + "version": "0.5.5", + "resolved": "https://registry.npmjs.org/image-size/-/image-size-0.5.5.tgz", + "integrity": "sha1-Cd/Uq50g4p6xw+gLiZA3jfnjy5w=", + "dev": true, + "optional": true } } }, diff --git a/package.json b/package.json index d37d6b301a6..6b8c20f084c 100644 --- a/package.json +++ b/package.json @@ -1792,8 +1792,8 @@ }, "jupyter.enablePlotViewer": { "type": "boolean", - "default": true, - "description": "Modify plot output so that it can be expanded into a plot viewer window.", + "default": false, + "description": "Modify plot output to include SVG output, for better display in the plot viewer", "scope": "resource" }, "jupyter.codeLenses": { @@ -2078,6 +2078,7 @@ "glob": "^7.1.2", "hash.js": "^1.1.7", "iconv-lite": "^0.4.21", + "image-size": "^1.0.0", "inversify": "^5.0.1", "is-online": "^9.0.0", "jsonc-parser": "^2.0.3", diff --git a/src/client/datascience/constants.ts b/src/client/datascience/constants.ts index eb3bc755613..9c023e03296 100644 --- a/src/client/datascience/constants.ts +++ b/src/client/datascience/constants.ts @@ -658,10 +658,10 @@ export namespace CodeSnippets { ]; export const ChangeDirectoryCommentIdentifier = '# ms-toolsai.jupyter added'; // Not translated so can compare. export const ImportIPython = '{0}\nfrom IPython import get_ipython\n\n{1}'; - export const MatplotLibInitSvg = `import matplotlib\n%matplotlib inline\n${Identifiers.MatplotLibDefaultParams} = dict(matplotlib.rcParams)\n%config InlineBackend.figure_formats = {'svg', 'png'}`; - export const MatplotLibInitPng = `import matplotlib\n%matplotlib inline\n${Identifiers.MatplotLibDefaultParams} = dict(matplotlib.rcParams)\n%config InlineBackend.figure_formats = {'png'}`; - export const ConfigSvg = `%config InlineBackend.figure_formats = {'svg', 'png'}`; - export const ConfigPng = `%config InlineBackend.figure_formats = {'png'}`; + export const MatplotLibInitSvg = `import matplotlib\n%matplotlib inline\n${Identifiers.MatplotLibDefaultParams} = dict(matplotlib.rcParams)\n%config InlineBackend.figure_formats = ['svg', 'png']`; + export const MatplotLibInitPng = `import matplotlib\n%matplotlib inline\n${Identifiers.MatplotLibDefaultParams} = dict(matplotlib.rcParams)\n%config InlineBackend.figure_formats = ['png']`; + export const ConfigSvg = `%config InlineBackend.figure_formats = ['svg', 'png']`; + export const ConfigPng = `%config InlineBackend.figure_formats = ['png']`; export const UpdateCWDAndPath = 'import os\nimport sys\n%cd "{0}"\nif os.getcwd() not in sys.path:\n sys.path.insert(0, os.getcwd())'; export const disableJedi = '%config Completer.use_jedi = False'; diff --git a/src/client/datascience/jupyter/jupyterNotebook.ts b/src/client/datascience/jupyter/jupyterNotebook.ts index 052623f17fe..7beb933b164 100644 --- a/src/client/datascience/jupyter/jupyterNotebook.ts +++ b/src/client/datascience/jupyter/jupyterNotebook.ts @@ -7,13 +7,13 @@ import { Observable } from 'rxjs/Observable'; import { Subscriber } from 'rxjs/Subscriber'; import * as uuid from 'uuid/v4'; import * as path from 'path'; -import { Disposable, Event, EventEmitter, Uri } from 'vscode'; +import { ColorThemeKind, Disposable, Event, EventEmitter, Uri } from 'vscode'; import { CancellationToken } from 'vscode-jsonrpc'; import { ServerStatus } from '../../../datascience-ui/interactive-common/mainState'; import { IApplicationShell, IVSCodeNotebook, IWorkspaceService } from '../../common/application/types'; import { CancellationError, createPromiseFromCancellation } from '../../common/cancellation'; import '../../common/extensions'; -import { traceError, traceInfo, traceInfoIf, traceWarning } from '../../common/logger'; +import { traceError, traceInfo, traceWarning } from '../../common/logger'; import { IConfigurationService, IDisposableRegistry, Resource } from '../../common/types'; import { createDeferred, Deferred, waitForPromise } from '../../common/utils/async'; @@ -41,7 +41,7 @@ import { KernelConnectionMetadata } from './kernels/types'; // eslint-disable-next-line @typescript-eslint/no-require-imports import cloneDeep = require('lodash/cloneDeep'); import { concatMultilineString, formatStreamText, splitMultilineString } from '../../../datascience-ui/common'; -import { isCI, PYTHON_LANGUAGE } from '../../common/constants'; +import { PYTHON_LANGUAGE } from '../../common/constants'; import { IFileSystem } from '../../common/platform/types'; import { RefBool } from '../../common/refBool'; import { PythonEnvironment } from '../../pythonEnvironments/info'; @@ -49,12 +49,10 @@ import { handleTensorBoardDisplayDataOutput } from '../notebook/helpers/executio import { getInterpreterFromKernelConnectionMetadata, getKernelConnectionLanguage, - isPythonKernelConnection, - sendTelemetryForPythonKernelExecutable + isPythonKernelConnection } from './kernels/helpers'; import { isResourceNativeNotebook } from '../notebook/helpers/helpers'; import { sendKernelTelemetryEvent } from '../telemetry/telemetry'; -import { IPythonExecutionFactory } from '../../common/process/types'; class CellSubscriber { public get startTime(): number { @@ -157,7 +155,6 @@ class CellSubscriber { export class JupyterNotebookBase implements INotebook { private sessionStartTime: number; private pendingCellSubscriptions: CellSubscriber[] = []; - private ranInitialSetup = false; private _resource: Resource; private _identity: Uri; private _disposed: boolean = false; @@ -209,8 +206,7 @@ export class JupyterNotebookBase implements INotebook { private workspace: IWorkspaceService, private applicationService: IApplicationShell, private fs: IFileSystem, - private readonly vscNotebook: IVSCodeNotebook, - private readonly executionFactory: IPythonExecutionFactory + private readonly vscNotebook: IVSCodeNotebook ) { this.sessionStartTime = Date.now(); @@ -287,93 +283,6 @@ export class JupyterNotebookBase implements INotebook { return this.session ? this.session.waitForIdle(timeoutMs) : Promise.resolve(); } - // Set up our initial plotting and imports - // eslint-disable-next-line complexity - public async initialize(cancelToken?: CancellationToken): Promise { - if (this.ranInitialSetup) { - return; - } - this.ranInitialSetup = true; - this._workingDirectory = undefined; - - traceInfo(`Initial setup for ${this.identity.toString()} starting ...`); - - try { - // When we start our notebook initial, change to our workspace or user specified root directory - await this.updateWorkingDirectoryAndPath(); - traceInfoIf(isCI, `Initial setup after for updateWorkingDirectoryAndPath ...`); - let isDefinitelyNotAPythonKernel = false; - if ( - this._executionInfo.kernelConnectionMetadata?.kind === 'startUsingKernelSpec' && - this._executionInfo.kernelConnectionMetadata.kernelSpec.language && - this._executionInfo.kernelConnectionMetadata.kernelSpec.language.toLowerCase() !== - PYTHON_LANGUAGE.toLocaleLowerCase() - ) { - isDefinitelyNotAPythonKernel = true; - } - if ( - this._executionInfo.kernelConnectionMetadata?.kind === 'connectToLiveKernel' && - this._executionInfo.kernelConnectionMetadata.kernelModel.language && - this._executionInfo.kernelConnectionMetadata.kernelModel.language.toLowerCase() !== - PYTHON_LANGUAGE.toLocaleLowerCase() - ) { - isDefinitelyNotAPythonKernel = true; - } - - const settings = this.configService.getSettings(this.resource); - if (settings && settings.themeMatplotlibPlots) { - // We're theming matplotlibs, so we have to setup our default state. - traceInfoIf(isCI, `Initialize config for plots for ${this.identity.toString()}`); - if (!isDefinitelyNotAPythonKernel) { - await this.initializeMatplotlib(cancelToken); - } - } else { - this.initializedMatplotlib = false; - const configInit = - (!settings || settings.enablePlotViewer) && - !isResourceNativeNotebook(this._resource, this.vscNotebook, this.fs) - ? CodeSnippets.ConfigSvg - : CodeSnippets.ConfigPng; - traceInfoIf(isCI, `Initialize config for plots for ${this.identity.toString()}`); - if (!isDefinitelyNotAPythonKernel) { - await this.executeSilently(configInit, cancelToken); - } - } - traceInfoIf(isCI, `Initial setup for ${this.identity.toString()} half way ...`); - if ( - !isDefinitelyNotAPythonKernel && - this._executionInfo.connectionInfo.localLaunch && - this._executionInfo.kernelConnectionMetadata - ) { - await sendTelemetryForPythonKernelExecutable( - this, - this._resource?.fsPath || this._identity.fsPath, - this._executionInfo.kernelConnectionMetadata, - this.executionFactory - ); - } - // Run any startup commands that we specified. Support the old form too - let setting = settings.runStartupCommands || settings.runMagicCommands; - - // Convert to string in case we get an array of startup commands. - if (Array.isArray(setting)) { - setting = setting.join(`\n`); - } - - if (setting) { - // Cleanup the line feeds. User may have typed them into the settings UI so they will have an extra \\ on the front. - traceInfoIf(isCI, 'Begin Run startup code for notebook'); - const cleanedUp = setting.replace(/\\n/g, '\n'); - const cells = await this.executeSilently(cleanedUp, cancelToken); - traceInfoIf(isCI, `Run startup code for notebook: ${cleanedUp} - results: ${cells.length}`); - } - - traceInfo(`Initial setup complete for ${this.identity.toString()}`); - } catch (e) { - traceWarning('Failed initialize', e); - } - } - public clear(_id: string): void { // We don't do anything as we don't cache results in this class. noop(); @@ -528,9 +437,6 @@ export class JupyterNotebookBase implements INotebook { // Restart our kernel await this.session.restart(timeoutMs); - // Rerun our initial setup for the notebook - await this.runInitialSetup(); - // Tell our loggers this.loggers.forEach((l) => l.onKernelRestarted(this.getNotebookId())); traceInfo(`Time to restart kernel is ${(Date.now() - this.sessionStartTime) / 1000}s`); @@ -541,13 +447,6 @@ export class JupyterNotebookBase implements INotebook { throw this.getDisposedError(); } - public async runInitialSetup() { - this.ranInitialSetup = false; - traceInfo('restartKernel - initialSetup'); - await this.initialize(); - traceInfo('restartKernel - initialSetup completed'); - } - @captureTelemetry(Telemetry.InterruptJupyterTime) public async interruptKernel(timeoutMs: number): Promise { if (this.session) { @@ -573,10 +472,6 @@ export class JupyterNotebookBase implements INotebook { this.sessionStartTime = Date.now(); traceWarning('Kernel restarting during interrupt'); - // Indicate we have to redo initial setup. We can't wait for starting though - // because sometimes it doesn't happen - this.ranInitialSetup = false; - // Indicate we restarted the race below restarted.resolve([]); @@ -716,18 +611,12 @@ export class JupyterNotebookBase implements INotebook { public async setKernelConnection(connectionMetadata: KernelConnectionMetadata, timeoutMS: number): Promise { // We need to start a new session with the new kernel spec if (this.session) { - // Turn off setup - this.ranInitialSetup = false; - // Change the kernel on the session await this.session.changeKernel(this.resource, connectionMetadata, timeoutMS); // Change our own kernel spec // Only after session was successfully created. this._executionInfo.kernelConnectionMetadata = connectionMetadata; - - // Rerun our initial setup - await this.initialize(); } else { // Change our own kernel spec this._executionInfo.kernelConnectionMetadata = connectionMetadata; @@ -821,6 +710,16 @@ export class JupyterNotebookBase implements INotebook { // get a request to update style await this.executeSilently(matplobInit, cancelToken); + const useDark = this.applicationService.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})` + ); + } + // Use this flag to detemine if we need to rerun this or not. this.initializedMatplotlib = true; } diff --git a/src/client/datascience/jupyter/kernels/kernel.ts b/src/client/datascience/jupyter/kernels/kernel.ts index 88ce0ce1597..16e403e84c2 100644 --- a/src/client/datascience/jupyter/kernels/kernel.ts +++ b/src/client/datascience/jupyter/kernels/kernel.ts @@ -7,9 +7,9 @@ import { KernelMessage } from '@jupyterlab/services'; import { Observable } from 'rxjs/Observable'; import { Subject } from 'rxjs/Subject'; import * as uuid from 'uuid/v4'; +import * as path from 'path'; import { CancellationTokenSource, - ColorThemeKind, Event, EventEmitter, NotebookCell, @@ -19,10 +19,11 @@ import { NotebookDocument, NotebookRange, Uri, - Range + Range, + ColorThemeKind } from 'vscode'; import { ServerStatus } from '../../../../datascience-ui/interactive-common/mainState'; -import { IApplicationShell } from '../../../common/application/types'; +import { IApplicationShell, IWorkspaceService } from '../../../common/application/types'; import { traceError, traceInfo, traceInfoIf, traceWarning } from '../../../common/logger'; import { IFileSystem } from '../../../common/platform/types'; import { IConfigurationService, IDisposableRegistry, Resource } from '../../../common/types'; @@ -52,6 +53,8 @@ import { InteractiveWindowView } from '../../notebook/constants'; import { chainWithPendingUpdates } from '../../notebook/helpers/notebookUpdater'; import { DataScience } from '../../../common/utils/localize'; import { CellOutputDisplayIdTracker } from './cellDisplayIdTracker'; +import { calculateWorkingDirectory } from '../../utils'; +import { expandWorkingDir } from '../jupyterUtils'; export class Kernel implements IKernel { get connection(): INotebookProviderConnection | undefined { @@ -98,6 +101,7 @@ export class Kernel implements IKernel { private restarting?: Deferred; private readonly kernelExecution: KernelExecution; private startCancellation = new CancellationTokenSource(); + private _workingDirectory?: string; constructor( public readonly notebookUri: Uri, public readonly resourceUri: Resource, @@ -114,7 +118,8 @@ export class Kernel implements IKernel { private readonly serverStorage: IJupyterServerUriStorage, controller: NotebookController, private readonly configService: IConfigurationService, - outputTracker: CellOutputDisplayIdTracker + outputTracker: CellOutputDisplayIdTracker, + private readonly workspaceService: IWorkspaceService ) { this.kernelExecution = new KernelExecution( kernelProvider, @@ -344,13 +349,23 @@ export class Kernel implements IKernel { this.disposables ); } + + // Change our initial directory and path + await this.updateWorkingDirectoryAndPath(); + if (isPythonKernelConnection(this.kernelConnectionMetadata)) { await this.disableJedi(); if (this.resourceUri) { await this.notebook.setLaunchingFile(this.resourceUri.fsPath); } - await this.initializeMatplotlib(); + + // For Python notebook initialize matplotlib + await this.initializeMatplotLib(); } + + // Run any startup commands that we have specified + await this.runStartupCommands(); + await this.notebook .requestKernelInfo() .then(async (item) => { @@ -434,18 +449,21 @@ export class Kernel implements IKernel { }); } } - private async initializeMatplotlib(): Promise { - if (!this.notebook) { - return; - } + private async initializeMatplotLib(): Promise { const settings = this.configService.getSettings(this.resourceUri); if (settings && settings.themeMatplotlibPlots) { - const matplobInit = settings.enablePlotViewer - ? CodeSnippets.MatplotLibInitSvg - : CodeSnippets.MatplotLibInitPng; + // We're theming matplotlibs, so we have to setup our default state. + traceInfoIf(isCI, `Initialize config for plots for ${(this.resourceUri || this.notebookUri).toString()}`); + const matplobInit = + !settings || settings.enablePlotViewer + ? CodeSnippets.MatplotLibInitSvg + : CodeSnippets.MatplotLibInitPng; traceInfo(`Initialize matplotlib for ${(this.resourceUri || this.notebookUri).toString()}`); + // Force matplotlib to inline and save the default style. We'll use this later if we + // get a request to update style 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. @@ -456,11 +474,65 @@ export class Kernel implements IKernel { ); } } else { - const configInit = !settings || settings.enablePlotViewer ? CodeSnippets.ConfigSvg : CodeSnippets.ConfigPng; + const configInit = settings && settings.enablePlotViewer ? CodeSnippets.ConfigSvg : CodeSnippets.ConfigPng; traceInfoIf(isCI, `Initialize config for plots for ${(this.resourceUri || this.notebookUri).toString()}`); await this.executeSilently(configInit); } } + private async runStartupCommands() { + const settings = this.configService.getSettings(this.resourceUri); + // Run any startup commands that we specified. Support the old form too + let setting = settings.runStartupCommands || settings.runMagicCommands; + + // Convert to string in case we get an array of startup commands. + if (Array.isArray(setting)) { + setting = setting.join(`\n`); + } + + if (setting) { + // Cleanup the line feeds. User may have typed them into the settings UI so they will have an extra \\ on the front. + traceInfoIf(isCI, 'Begin Run startup code for notebook'); + const cleanedUp = setting.replace(/\\n/g, '\n'); + await this.executeSilently(cleanedUp); + traceInfoIf(isCI, `Run startup code for notebook: ${cleanedUp}`); + } + } + + private async updateWorkingDirectoryAndPath(launchingFile?: string): Promise { + const suggestedDir = await calculateWorkingDirectory(this.configService, this.workspaceService, this.fs); + traceInfo('UpdateWorkingDirectoryAndPath'); + if (this.connection && this.connection.localLaunch) { + if (suggestedDir && (await this.fs.localDirectoryExists(suggestedDir))) { + // We should use the launch info directory. It trumps the possible dir + this._workingDirectory = suggestedDir; + return this.changeDirectoryIfPossible(this._workingDirectory); + } else if ( + launchingFile && + (await this.fs.localFileExists(launchingFile)) && + (await this.fs.localDirectoryExists(path.dirname(launchingFile))) + ) { + // Combine the working directory with this file if possible. + this._workingDirectory = expandWorkingDir(suggestedDir, launchingFile, this.workspaceService); + if (this._workingDirectory) { + return this.changeDirectoryIfPossible(this._workingDirectory); + } + } + } + } + + // Update both current working directory and sys.path with the desired directory + private changeDirectoryIfPossible = async (directory: string): Promise => { + if ( + this.connection && + this.connection.localLaunch && + isPythonKernelConnection(this.kernelConnectionMetadata) && + (await this.fs.localDirectoryExists(directory)) + ) { + traceInfo('changeDirectoryIfPossible'); + await this.executeSilently(CodeSnippets.UpdateCWDAndPath.format(directory)); + } + }; + private async executeSilently(code: string) { if (!this.notebook) { return; diff --git a/src/client/datascience/jupyter/kernels/kernelExecution.ts b/src/client/datascience/jupyter/kernels/kernelExecution.ts index 43a2296b523..1716037db93 100644 --- a/src/client/datascience/jupyter/kernels/kernelExecution.ts +++ b/src/client/datascience/jupyter/kernels/kernelExecution.ts @@ -280,9 +280,6 @@ export class KernelExecution implements IDisposable { traceWarning(`Error during restart: ${exc}`); }); - // Reinitialize the kernel after a session restart - await notebook.runInitialSetup(); - (notebook as JupyterNotebookBase).fireRestart(); } diff --git a/src/client/datascience/jupyter/kernels/kernelProvider.ts b/src/client/datascience/jupyter/kernels/kernelProvider.ts index 1597ca10288..57e3e73561c 100644 --- a/src/client/datascience/jupyter/kernels/kernelProvider.ts +++ b/src/client/datascience/jupyter/kernels/kernelProvider.ts @@ -5,7 +5,7 @@ import { inject, injectable } from 'inversify'; import { Event, EventEmitter, NotebookDocument } from 'vscode'; -import { IApplicationShell } from '../../../common/application/types'; +import { IApplicationShell, IWorkspaceService } from '../../../common/application/types'; import { traceInfo, traceWarning } from '../../../common/logger'; import { IFileSystem } from '../../../common/platform/types'; import { @@ -41,7 +41,8 @@ export class KernelProvider implements IKernelProvider { @inject(IApplicationShell) private readonly appShell: IApplicationShell, @inject(IFileSystem) private readonly fs: IFileSystem, @inject(IJupyterServerUriStorage) private readonly serverStorage: IJupyterServerUriStorage, - @inject(CellOutputDisplayIdTracker) private readonly outputTracker: CellOutputDisplayIdTracker + @inject(CellOutputDisplayIdTracker) private readonly outputTracker: CellOutputDisplayIdTracker, + @inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService ) { this.asyncDisposables.push(this); } @@ -84,7 +85,8 @@ export class KernelProvider implements IKernelProvider { this.serverStorage, options.controller, this.configService, - this.outputTracker + this.outputTracker, + this.workspaceService ); kernel.onRestarted(() => this._onDidRestartKernel.fire(kernel)); this.asyncDisposables.push(kernel); diff --git a/src/client/datascience/jupyter/liveshare/hostJupyterNotebook.ts b/src/client/datascience/jupyter/liveshare/hostJupyterNotebook.ts index 1be0d3a40f3..b9dc7a56938 100644 --- a/src/client/datascience/jupyter/liveshare/hostJupyterNotebook.ts +++ b/src/client/datascience/jupyter/liveshare/hostJupyterNotebook.ts @@ -17,7 +17,6 @@ import { } from '../../types'; import { JupyterNotebookBase } from '../jupyterNotebook'; import { IFileSystem } from '../../../common/platform/types'; -import { IPythonExecutionFactory } from '../../../common/process/types'; /* eslint-disable @typescript-eslint/no-explicit-any */ @@ -36,8 +35,7 @@ export class HostJupyterNotebook extends JupyterNotebookBase implements INoteboo workspace: IWorkspaceService, appService: IApplicationShell, fs: IFileSystem, - vscNotebook: IVSCodeNotebook, - executionFactory: IPythonExecutionFactory + vscNotebook: IVSCodeNotebook ) { super( session, @@ -51,8 +49,7 @@ export class HostJupyterNotebook extends JupyterNotebookBase implements INoteboo workspace, appService, fs, - vscNotebook, - executionFactory + vscNotebook ); } diff --git a/src/client/datascience/jupyter/liveshare/hostJupyterServer.ts b/src/client/datascience/jupyter/liveshare/hostJupyterServer.ts index c47931bb867..7c6ae63703c 100644 --- a/src/client/datascience/jupyter/liveshare/hostJupyterServer.ts +++ b/src/client/datascience/jupyter/liveshare/hostJupyterServer.ts @@ -38,7 +38,6 @@ import { getDisplayNameOrNameOfKernelConnection } from '../kernels/helpers'; import { KernelConnectionMetadata } from '../kernels/types'; import { HostJupyterNotebook } from './hostJupyterNotebook'; import { ILocalKernelFinder, IRemoteKernelFinder } from '../../kernel-launcher/types'; -import { IPythonExecutionFactory } from '../../../common/process/types'; import { isCI, STANDARD_OUTPUT_CHANNEL } from '../../../common/constants'; import { inject, injectable, named } from 'inversify'; /* eslint-disable @typescript-eslint/no-explicit-any */ @@ -178,8 +177,7 @@ export class HostJupyterServer extends JupyterServerBase implements INotebookSer this.workspaceService, this.appService, this.fs, - this.vscodeNotebook, - serviceContainer.get(IPythonExecutionFactory) + this.vscodeNotebook ); // Wait for it to be ready @@ -187,9 +185,6 @@ export class HostJupyterServer extends JupyterServerBase implements INotebookSer const idleTimeout = configService.getSettings().jupyterLaunchTimeout; await notebook.waitForIdle(idleTimeout); - // Run initial setup - await notebook.initialize(cancelToken); - traceInfo(`Finished connecting ${this.id}`); notebookPromise.resolve(notebook); diff --git a/src/client/datascience/notebook/helpers/helpers.ts b/src/client/datascience/notebook/helpers/helpers.ts index 43ff7c28e22..798489d938b 100644 --- a/src/client/datascience/notebook/helpers/helpers.ts +++ b/src/client/datascience/notebook/helpers/helpers.ts @@ -429,8 +429,8 @@ 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) { + // If we have SVG or 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[] = []; diff --git a/src/client/datascience/notebook/outputs/plotViewHandler.ts b/src/client/datascience/notebook/outputs/plotViewHandler.ts index 52c213e034e..cc2dc1abc95 100644 --- a/src/client/datascience/notebook/outputs/plotViewHandler.ts +++ b/src/client/datascience/notebook/outputs/plotViewHandler.ts @@ -1,9 +1,11 @@ +import sizeOf from 'image-size'; import { inject, injectable } from 'inversify'; import { NotebookCellOutputItem, NotebookEditor } from 'vscode'; import { traceError } from '../../../common/logger'; import { IPlotViewerProvider } from '../../types'; const svgMimeType = 'image/svg+xml'; +const pngMimeType = 'image/png'; @injectable() export class PlotViewHandler { @@ -14,11 +16,23 @@ export class PlotViewHandler { return; } const outputItem = getOutputItem(editor, outputId, svgMimeType); + let svgString: string | undefined; if (!outputItem) { - return traceError(`No SVG Plot to open ${editor.document.uri.toString()}, id: ${outputId}`); + // Didn't find svg, see if we have png we can convert + const pngOutput = getOutputItem(editor, outputId, pngMimeType); + + if (!pngOutput) { + return traceError(`No SVG or PNG Plot to open ${editor.document.uri.toString()}, id: ${outputId}`); + } + + // If we did find a PNG wrap it in an SVG element so that we can display it + svgString = convertPngToSvg(pngOutput); + } else { + svgString = new TextDecoder().decode(outputItem.data); + } + if (svgString) { + await this.plotViewProvider.showPlot(svgString); } - const svg = new TextDecoder().decode(outputItem.data); - await this.plotViewProvider.showPlot(svg); } } @@ -32,3 +46,19 @@ function getOutputItem(editor: NotebookEditor, outputId: string, mimeType: strin } } } + +// Wrap our PNG data into an SVG element so what we can display it in the current plot viewer +function convertPngToSvg(pngOutput: NotebookCellOutputItem): string { + const imageBuffer = Buffer.from(pngOutput.data.buffer); // .buffer to not make a copy + const imageData = imageBuffer.toString('base64'); + const dims = sizeOf(imageBuffer); + + // Of note here, we want the dims on the SVG element, and the image at 100% this is due to how the SVG control + // in the plot viewer works. The injected svg is sized down to 100px x 100px on the plot selection list so if + // dims are set on the image then it scales out of bounds + return ` + + + +`; +} diff --git a/src/client/datascience/raw-kernel/liveshare/hostRawNotebookProvider.ts b/src/client/datascience/raw-kernel/liveshare/hostRawNotebookProvider.ts index cc2da43d7f7..18bd87890fb 100644 --- a/src/client/datascience/raw-kernel/liveshare/hostRawNotebookProvider.ts +++ b/src/client/datascience/raw-kernel/liveshare/hostRawNotebookProvider.ts @@ -46,7 +46,6 @@ import { RawJupyterSession } from '../rawJupyterSession'; import { RawNotebookProviderBase } from '../rawNotebookProvider'; import { trackKernelResourceInformation } from '../../telemetry/telemetry'; import { KernelSpecNotFoundError } from './kernelSpecNotFoundError'; -import { IPythonExecutionFactory } from '../../../common/process/types'; import { getResourceType } from '../../common'; import { getTelemetrySafeLanguage } from '../../../telemetry/helpers'; import { inject, injectable, named } from 'inversify'; @@ -179,13 +178,9 @@ export class HostRawNotebookProvider extends RawNotebookProviderBase implements this.workspaceService, this.appShell, this.fs, - this.vscodeNotebook, - this.serviceContainer.get(IPythonExecutionFactory) + this.vscodeNotebook ); - // Run initial setup - await notebook.initialize(cancelToken); - traceInfo(`Finished connecting ${this.id}`); notebookPromise.resolve(notebook); diff --git a/src/client/datascience/types.ts b/src/client/datascience/types.ts index 4ed2d58b833..0b4ce4247f4 100644 --- a/src/client/datascience/types.ts +++ b/src/client/datascience/types.ts @@ -202,7 +202,6 @@ export interface INotebook extends IAsyncDisposable { cancelToken?: CancellationToken ): Promise; restartKernel(timeoutInMs: number): Promise; - runInitialSetup(): Promise; waitForIdle(timeoutInMs: number): Promise; interruptKernel(timeoutInMs: number): Promise; setLaunchingFile(file: string): Promise; diff --git a/src/test/datascience/notebook/helpers.vscode.test.ts b/src/test/datascience/notebook/helpers.vscode.test.ts index 287248e0e4a..6a9bd4a80a6 100644 --- a/src/test/datascience/notebook/helpers.vscode.test.ts +++ b/src/test/datascience/notebook/helpers.vscode.test.ts @@ -247,6 +247,7 @@ suite('DataScience - VSCode Notebook - helpers', () => { new NotebookCellOutputItem(Buffer.from(base64EncodedImage, 'base64'), 'image/jpeg') ], { + __displayOpenPlotIcon: true, executionCount: 1, outputType: output_type, metadata: {} // display_data & execute_result always have metadata. @@ -274,6 +275,7 @@ suite('DataScience - VSCode Notebook - helpers', () => { new NotebookCellOutput( [new NotebookCellOutputItem(Buffer.from(base64EncodedImage, 'base64'), 'image/png')], { + __displayOpenPlotIcon: true, executionCount: 1, metadata: { needs_background: 'light' @@ -303,6 +305,7 @@ suite('DataScience - VSCode Notebook - helpers', () => { new NotebookCellOutput( [new NotebookCellOutputItem(Buffer.from(base64EncodedImage, 'base64'), 'image/png')], { + __displayOpenPlotIcon: true, executionCount: 1, metadata: { needs_background: 'dark' @@ -332,6 +335,7 @@ suite('DataScience - VSCode Notebook - helpers', () => { new NotebookCellOutput( [new NotebookCellOutputItem(Buffer.from(base64EncodedImage, 'base64'), 'image/png')], { + __displayOpenPlotIcon: true, executionCount: 1, metadata: { 'image/png': { height: '111px', width: '999px' } @@ -362,6 +366,7 @@ suite('DataScience - VSCode Notebook - helpers', () => { new NotebookCellOutput( [new NotebookCellOutputItem(Buffer.from(base64EncodedImage, 'base64'), 'image/png')], { + __displayOpenPlotIcon: true, executionCount: 1, metadata: { unconfined: true,