From 388f87d8a3b26d62c0d79cf34493db4b5d62025a Mon Sep 17 00:00:00 2001 From: Sergio Villalobos Date: Wed, 27 May 2020 23:27:10 -0700 Subject: [PATCH 1/5] Issue#12033 Expose API to show Data Viewer component from an external extension --- src/client/api.ts | 11 +++ .../datascience/data-viewing/dataViewer.ts | 79 ++++++++------- ...ViewerProvider.ts => dataViewerFactory.ts} | 17 ++-- .../jupyterVariableDataProvider.ts | 97 +++++++++++++++++++ src/client/datascience/data-viewing/types.ts | 24 ++++- .../interactive-common/interactiveBase.ts | 13 ++- .../interactive-ipynb/nativeEditor.ts | 10 +- .../nativeEditorOldWebView.ts | 10 +- .../interactive-window/interactiveWindow.ts | 10 +- src/client/datascience/serviceRegistry.ts | 18 +++- src/client/datascience/types.ts | 20 +++- .../data-explorer/cellFormatter.tsx | 9 +- .../data-explorer/mainPanel.tsx | 10 +- .../data-explorer/reactSlickGrid.tsx | 18 ++-- .../data-viewing/dataViewer.unit.test.ts | 75 ++++++++++++++ .../dataViewerProvider.unit.test.ts | 80 --------------- .../datascience/dataScienceIocContainer.ts | 27 +++++- .../dataviewer.functional.test.tsx | 75 +++++++++----- 18 files changed, 404 insertions(+), 199 deletions(-) rename src/client/datascience/data-viewing/{dataViewerProvider.ts => dataViewerFactory.ts} (64%) create mode 100644 src/client/datascience/data-viewing/jupyterVariableDataProvider.ts create mode 100644 src/test/datascience/data-viewing/dataViewer.unit.test.ts delete mode 100644 src/test/datascience/data-viewing/dataViewerProvider.unit.test.ts diff --git a/src/client/api.ts b/src/client/api.ts index 0b37a5da101c..c6f88172c351 100644 --- a/src/client/api.ts +++ b/src/client/api.ts @@ -7,6 +7,8 @@ import { isTestExecution } from './common/constants'; import { DebugAdapterNewPtvsd } from './common/experiments/groups'; import { traceError } from './common/logger'; import { IConfigurationService, IExperimentsManager, Resource } from './common/types'; +import { IDataViewerDataProvider } from './datascience/data-viewing/types'; +import { IDataViewer, IDataViewerFactory } from './datascience/types'; import { getDebugpyLauncherArgs, getDebugpyPackagePath, @@ -64,6 +66,9 @@ export interface IExtensionApi { */ getExecutionCommand(resource?: Resource): string[] | undefined; }; + datascience: { + showDataViewer(dataProvider: IDataViewerDataProvider, title: string): Promise; + }; } export function buildApi( @@ -118,6 +123,12 @@ export function buildApi( // If pythonPath equals an empty string, no interpreter is set. return pythonPath === '' ? undefined : [pythonPath]; } + }, + datascience: { + async showDataViewer(dataProvider: IDataViewerDataProvider, title: string) { + const dataViewerProviderService = serviceContainer.get(IDataViewerFactory); + return dataViewerProviderService.create(dataProvider, title); + } } }; diff --git a/src/client/datascience/data-viewing/dataViewer.ts b/src/client/datascience/data-viewing/dataViewer.ts index 9c5985fd30c7..a88a2036df68 100644 --- a/src/client/datascience/data-viewing/dataViewer.ts +++ b/src/client/datascience/data-viewing/dataViewer.ts @@ -3,7 +3,7 @@ 'use strict'; import '../../common/extensions'; -import { inject, injectable, named } from 'inversify'; +import { inject, injectable } from 'inversify'; import * as path from 'path'; import { ViewColumn } from 'vscode'; @@ -16,18 +16,23 @@ import * as localize from '../../common/utils/localize'; import { noop } from '../../common/utils/misc'; import { StopWatch } from '../../common/utils/stopWatch'; import { sendTelemetryEvent } from '../../telemetry'; -import { HelpLinks, Identifiers, Telemetry } from '../constants'; +import { HelpLinks, Telemetry } from '../constants'; import { JupyterDataRateLimitError } from '../jupyter/jupyterDataRateLimitError'; -import { ICodeCssGenerator, IDataViewer, IJupyterVariable, IJupyterVariables, INotebook, IThemeFinder } from '../types'; +import { ICodeCssGenerator, IDataViewer, IThemeFinder } from '../types'; import { WebViewHost } from '../webViewHost'; import { DataViewerMessageListener } from './dataViewerMessageListener'; -import { DataViewerMessages, IDataViewerMapping, IGetRowsRequest } from './types'; +import { + DataViewerMessages, + IDataFrameInfo, + IDataViewerDataProvider, + IDataViewerMapping, + IGetRowsRequest +} from './types'; const dataExplorereDir = path.join(EXTENSION_ROOT_DIR, 'out', 'datascience-ui', 'viewers'); @injectable() export class DataViewer extends WebViewHost implements IDataViewer, IDisposable { - private notebook: INotebook | undefined; - private variable: IJupyterVariable | undefined; + private dataProvider: IDataViewerDataProvider | undefined; private rowsTimer: StopWatch | undefined; private pendingRowsCount: number = 0; @@ -37,7 +42,6 @@ export class DataViewer extends WebViewHost implements IData @inject(ICodeCssGenerator) cssGenerator: ICodeCssGenerator, @inject(IThemeFinder) themeFinder: IThemeFinder, @inject(IWorkspaceService) workspaceService: IWorkspaceService, - @inject(IJupyterVariables) @named(Identifiers.ALL_VARIABLES) private variableManager: IJupyterVariables, @inject(IApplicationShell) private applicationShell: IApplicationShell, @inject(IExperimentsManager) experimentsManager: IExperimentsManager, @inject(UseCustomEditorApi) useCustomEditorApi: boolean @@ -57,33 +61,41 @@ export class DataViewer extends WebViewHost implements IData useCustomEditorApi, false ); + } - // Load the web panel using our current directory as we don't expect to load any other files - super.loadWebPanel(process.cwd()).catch(traceError); + private static trimTitle(title: string): string { + const TRIM_LENGTH = 40; + if (title && title.length > TRIM_LENGTH) { + title = `${title.substr(0, TRIM_LENGTH)}...`; + } + return title; } - public async showVariable(variable: IJupyterVariable, notebook: INotebook): Promise { + public async showData(dataProvider: IDataViewerDataProvider, title: string): Promise { if (!this.isDisposed) { - // Save notebook this is tied to - this.notebook = notebook; + this.dataProvider = dataProvider; - // Fill in our variable's beginning data - this.variable = await this.prepVariable(variable, notebook); + // Load the web panel using our current directory as we don't expect to load any other files + await super.loadWebPanel(process.cwd()).catch(traceError); - // Create our new title with the variable name - let newTitle = `${localize.DataScience.dataExplorerTitle()} - ${variable.name}`; - const TRIM_LENGTH = 40; - if (newTitle.length > TRIM_LENGTH) { - newTitle = `${newTitle.substr(0, TRIM_LENGTH)}...`; - } - - super.setTitle(newTitle); + super.setTitle(DataViewer.trimTitle(title)); // Then show our web panel. Eventually we need to consume the data await super.show(true); + const dataFrameInfo = await this.prepDataFrameInfo(dataProvider); + // Send a message with our data - this.postMessage(DataViewerMessages.InitializeData, this.variable).ignoreErrors(); + this.postMessage(DataViewerMessages.InitializeData, dataFrameInfo).ignoreErrors(); + } + } + + public dispose(): void { + super.dispose(); + + if (this.dataProvider) { + this.dataProvider.dispose(); + this.dataProvider = undefined; } } @@ -109,9 +121,9 @@ export class DataViewer extends WebViewHost implements IData super.onMessage(message, payload); } - private async prepVariable(variable: IJupyterVariable, notebook: INotebook): Promise { + private async prepDataFrameInfo(dataProvider: IDataViewerDataProvider): Promise { this.rowsTimer = new StopWatch(); - const output = await this.variableManager.getDataFrameInfo(variable, notebook); + const output = await dataProvider.getDataFrameInfo(); // Log telemetry about number of rows try { @@ -131,13 +143,8 @@ export class DataViewer extends WebViewHost implements IData private async getAllRows() { return this.wrapRequest(async () => { - if (this.variable && this.variable.rowCount && this.notebook) { - const allRows = await this.variableManager.getDataFrameRows( - this.variable, - this.notebook, - 0, - this.variable.rowCount - ); + if (this.dataProvider) { + const allRows = await this.dataProvider.getAllRows(); this.pendingRowsCount = 0; return this.postMessage(DataViewerMessages.GetAllRowsResponse, allRows); } @@ -146,12 +153,10 @@ export class DataViewer extends WebViewHost implements IData private getRowChunk(request: IGetRowsRequest) { return this.wrapRequest(async () => { - if (this.variable && this.variable.rowCount && this.notebook) { - const rows = await this.variableManager.getDataFrameRows( - this.variable, - this.notebook, + if (this.dataProvider) { + const rows = await this.dataProvider.getRows( request.start, - Math.min(request.end, this.variable.rowCount) + Math.min(request.end, (await this.dataProvider.getDataFrameInfo()).rowCount) ); return this.postMessage(DataViewerMessages.GetRowsResponse, { rows, diff --git a/src/client/datascience/data-viewing/dataViewerProvider.ts b/src/client/datascience/data-viewing/dataViewerFactory.ts similarity index 64% rename from src/client/datascience/data-viewing/dataViewerProvider.ts rename to src/client/datascience/data-viewing/dataViewerFactory.ts index 7a41344b01fa..a4b102ffba77 100644 --- a/src/client/datascience/data-viewing/dataViewerProvider.ts +++ b/src/client/datascience/data-viewing/dataViewerFactory.ts @@ -7,16 +7,15 @@ import { inject, injectable } from 'inversify'; import { IAsyncDisposable, IAsyncDisposableRegistry } from '../../common/types'; import { IServiceContainer } from '../../ioc/types'; -import { IDataViewer, IDataViewerProvider, IJupyterVariable, INotebook } from '../types'; -import { DataViewerDependencyService } from './dataViewerDependencyService'; +import { IDataViewer, IDataViewerFactory } from '../types'; +import { IDataViewerDataProvider } from './types'; @injectable() -export class DataViewerProvider implements IDataViewerProvider, IAsyncDisposable { +export class DataViewerFactory implements IDataViewerFactory, IAsyncDisposable { private activeExplorers: IDataViewer[] = []; constructor( @inject(IServiceContainer) private serviceContainer: IServiceContainer, - @inject(IAsyncDisposableRegistry) asyncRegistry: IAsyncDisposableRegistry, - @inject(DataViewerDependencyService) private dependencyService: DataViewerDependencyService + @inject(IAsyncDisposableRegistry) asyncRegistry: IAsyncDisposableRegistry ) { asyncRegistry.push(this); } @@ -25,18 +24,16 @@ export class DataViewerProvider implements IDataViewerProvider, IAsyncDisposable await Promise.all(this.activeExplorers.map((d) => d.dispose())); } - public async create(variable: IJupyterVariable, notebook: INotebook): Promise { + public async create(dataProvider: IDataViewerDataProvider, title: string): Promise { let result: IDataViewer | undefined; // Create the data explorer (this should show the window) const dataExplorer = this.serviceContainer.get(IDataViewer); try { - // Verify this is allowed. - await this.dependencyService.checkAndInstallMissingDependencies(notebook.getMatchingInterpreter()); - // Then load the data. this.activeExplorers.push(dataExplorer); - await dataExplorer.showVariable(variable, notebook); + + await dataExplorer.showData(dataProvider, title); result = dataExplorer; } finally { if (!result) { diff --git a/src/client/datascience/data-viewing/jupyterVariableDataProvider.ts b/src/client/datascience/data-viewing/jupyterVariableDataProvider.ts new file mode 100644 index 000000000000..1e163576939d --- /dev/null +++ b/src/client/datascience/data-viewing/jupyterVariableDataProvider.ts @@ -0,0 +1,97 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +'use strict'; +import '../../common/extensions'; + +import { inject, injectable, named } from 'inversify'; + +import { JSONObject } from '@phosphor/coreutils'; + +import { Identifiers } from '../constants'; +import { IJupyterVariable, IJupyterVariableDataProvider, IJupyterVariables, INotebook } from '../types'; +import { DataViewerDependencyService } from './dataViewerDependencyService'; +import { ColumnType, IDataFrameInfo } from './types'; + +@injectable() +export class JupyterVariableDataProvider implements IJupyterVariableDataProvider { + private initialized: boolean = false; + + constructor( + @inject(IJupyterVariables) @named(Identifiers.ALL_VARIABLES) private variableManager: IJupyterVariables, + @inject(DataViewerDependencyService) private dependencyService: DataViewerDependencyService, + private variable: IJupyterVariable, + private notebook: INotebook + ) {} + + private static getNormalizedColumns(columns: { key: string; type: string }[]): { key: string; type: ColumnType }[] { + return columns.map((column: { key: string; type: string }) => { + let normalizedType: ColumnType; + switch (column.type) { + case 'bool': + normalizedType = ColumnType.Bool; + break; + case 'integer': + case 'int32': + case 'int64': + case 'float': + case 'float32': + case 'float64': + case 'number': + normalizedType = ColumnType.Number; + break; + default: + normalizedType = ColumnType.String; + } + return { + key: column.key, + type: normalizedType + }; + }); + } + + public dispose(): void { + return; + } + + public async getDataFrameInfo(): Promise { + await this.ensureInitialized(); + return { + columns: this.variable.columns + ? JupyterVariableDataProvider.getNormalizedColumns(this.variable.columns) + : this.variable.columns, + indexColumn: this.variable.indexColumn, + rowCount: this.variable.rowCount || 0 + }; + } + + public async getAllRows() { + let allRows: JSONObject = {}; + await this.ensureInitialized(); + if (this.variable.rowCount) { + allRows = await this.variableManager.getDataFrameRows( + this.variable, + this.notebook, + 0, + this.variable.rowCount + ); + } + return allRows; + } + + public async getRows(start: number, end: number) { + let rows: JSONObject = {}; + await this.ensureInitialized(); + if (this.variable.rowCount) { + rows = await this.variableManager.getDataFrameRows(this.variable, this.notebook, start, end); + } + return rows; + } + + private async ensureInitialized(): Promise { + if (!this.initialized) { + await this.dependencyService.checkAndInstallMissingDependencies(this.notebook.getMatchingInterpreter()); + this.variable = await this.variableManager.getDataFrameInfo(this.variable, this.notebook); + this.initialized = true; + } + } +} diff --git a/src/client/datascience/data-viewing/types.ts b/src/client/datascience/data-viewing/types.ts index 35372910d149..e20421dcb1bf 100644 --- a/src/client/datascience/data-viewing/types.ts +++ b/src/client/datascience/data-viewing/types.ts @@ -1,10 +1,9 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. 'use strict'; -import type { JSONObject } from '@phosphor/coreutils'; +import { JSONObject } from '@phosphor/coreutils'; import { SharedMessages } from '../messages'; -import { IJupyterVariable } from '../types'; export const CellFetchAllLimit = 100000; export const CellFetchSizeFirst = 100000; @@ -43,10 +42,29 @@ export interface IGetRowsResponse { export type IDataViewerMapping = { [DataViewerMessages.Started]: never | undefined; [DataViewerMessages.UpdateSettings]: string; - [DataViewerMessages.InitializeData]: IJupyterVariable; + [DataViewerMessages.InitializeData]: IDataFrameInfo; [DataViewerMessages.GetAllRowsRequest]: never | undefined; [DataViewerMessages.GetAllRowsResponse]: JSONObject; [DataViewerMessages.GetRowsRequest]: IGetRowsRequest; [DataViewerMessages.GetRowsResponse]: IGetRowsResponse; [DataViewerMessages.CompletedData]: never | undefined; }; + +export interface IDataFrameInfo { + columns?: { key: string; type: ColumnType }[]; + indexColumn?: string; + rowCount: number; +} + +export interface IDataViewerDataProvider { + dispose(): void; + getDataFrameInfo(): Promise; + getAllRows(): Promise; + getRows(start: number, end: number): Promise; +} + +export enum ColumnType { + String = 'string', + Number = 'number', + Bool = 'bool' +} diff --git a/src/client/datascience/interactive-common/interactiveBase.ts b/src/client/datascience/interactive-common/interactiveBase.ts index 9487a38439ca..cfc8aa3dc4a9 100644 --- a/src/client/datascience/interactive-common/interactiveBase.ts +++ b/src/client/datascience/interactive-common/interactiveBase.ts @@ -74,13 +74,14 @@ import { ICell, ICodeCssGenerator, IDataScienceErrorHandler, - IDataViewerProvider, + IDataViewerFactory, IInteractiveBase, IInteractiveWindowInfo, IInteractiveWindowListener, IJupyterDebugger, IJupyterExecution, IJupyterKernelSpec, + IJupyterVariableDataProviderFactory, IJupyterVariables, IJupyterVariablesRequest, IJupyterVariablesResponse, @@ -137,7 +138,8 @@ export abstract class InteractiveBase extends WebViewHost { try { if (await this.checkColumnSize(request.columnSize)) { - await this.dataExplorerProvider.create(request.variable, this._notebook!); + const jupyterVariableDataProvider = this.jupyterVariableDataProviderFactory( + request.variable, + this._notebook! + ); + const title: string = `${localize.DataScience.dataExplorerTitle()} - ${request.variable.name}`; + await this.dataExplorerFactory.create(jupyterVariableDataProvider, title); } } catch (e) { this.applicationShell.showErrorMessage(e.toString()); diff --git a/src/client/datascience/interactive-ipynb/nativeEditor.ts b/src/client/datascience/interactive-ipynb/nativeEditor.ts index 27e656e6dc2f..bdf0265e196f 100644 --- a/src/client/datascience/interactive-ipynb/nativeEditor.ts +++ b/src/client/datascience/interactive-ipynb/nativeEditor.ts @@ -61,12 +61,13 @@ import { ICell, ICodeCssGenerator, IDataScienceErrorHandler, - IDataViewerProvider, + IDataViewerFactory, IInteractiveWindowInfo, IInteractiveWindowListener, IJupyterDebugger, IJupyterExecution, IJupyterKernelSpec, + IJupyterVariableDataProviderFactory, IJupyterVariables, INotebookEditor, INotebookEditorProvider, @@ -164,7 +165,9 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { @inject(IWorkspaceService) workspaceService: IWorkspaceService, @inject(NativeEditorSynchronizer) private readonly synchronizer: NativeEditorSynchronizer, @inject(INotebookEditorProvider) private editorProvider: INotebookEditorProvider, - @inject(IDataViewerProvider) dataExplorerProvider: IDataViewerProvider, + @inject(IDataViewerFactory) dataExplorerFactory: IDataViewerFactory, + @inject(IJupyterVariableDataProviderFactory) + jupyterVariableDataProviderFactory: IJupyterVariableDataProviderFactory, @inject(IJupyterVariables) @named(Identifiers.ALL_VARIABLES) jupyterVariables: IJupyterVariables, @inject(IJupyterDebugger) jupyterDebugger: IJupyterDebugger, @inject(INotebookImporter) protected readonly importer: INotebookImporter, @@ -191,7 +194,8 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { configuration, jupyterExporter, workspaceService, - dataExplorerProvider, + dataExplorerFactory, + jupyterVariableDataProviderFactory, jupyterVariables, jupyterDebugger, errorHandler, diff --git a/src/client/datascience/interactive-ipynb/nativeEditorOldWebView.ts b/src/client/datascience/interactive-ipynb/nativeEditorOldWebView.ts index 1d89b995ec88..bc8f6d7560df 100644 --- a/src/client/datascience/interactive-ipynb/nativeEditorOldWebView.ts +++ b/src/client/datascience/interactive-ipynb/nativeEditorOldWebView.ts @@ -35,10 +35,11 @@ import { KernelSwitcher } from '../jupyter/kernels/kernelSwitcher'; import { ICodeCssGenerator, IDataScienceErrorHandler, - IDataViewerProvider, + IDataViewerFactory, IInteractiveWindowListener, IJupyterDebugger, IJupyterExecution, + IJupyterVariableDataProviderFactory, IJupyterVariables, INotebookEditorProvider, INotebookExporter, @@ -87,7 +88,9 @@ export class NativeEditorOldWebView extends NativeEditor { @inject(IWorkspaceService) workspaceService: IWorkspaceService, @inject(NativeEditorSynchronizer) synchronizer: NativeEditorSynchronizer, @inject(INotebookEditorProvider) editorProvider: INotebookEditorProvider, - @inject(IDataViewerProvider) dataExplorerProvider: IDataViewerProvider, + @inject(IDataViewerFactory) dataExplorerFactory: IDataViewerFactory, + @inject(IJupyterVariableDataProviderFactory) + jupyterVariableDataProviderFactory: IJupyterVariableDataProviderFactory, @inject(IJupyterVariables) @named(Identifiers.ALL_VARIABLES) jupyterVariables: IJupyterVariables, @inject(IJupyterDebugger) jupyterDebugger: IJupyterDebugger, @inject(INotebookImporter) importer: INotebookImporter, @@ -118,7 +121,8 @@ export class NativeEditorOldWebView extends NativeEditor { workspaceService, synchronizer, editorProvider, - dataExplorerProvider, + dataExplorerFactory, + jupyterVariableDataProviderFactory, jupyterVariables, jupyterDebugger, importer, diff --git a/src/client/datascience/interactive-window/interactiveWindow.ts b/src/client/datascience/interactive-window/interactiveWindow.ts index 9951a1f6ee64..0a33612b3faa 100644 --- a/src/client/datascience/interactive-window/interactiveWindow.ts +++ b/src/client/datascience/interactive-window/interactiveWindow.ts @@ -45,7 +45,7 @@ import { ICell, ICodeCssGenerator, IDataScienceErrorHandler, - IDataViewerProvider, + IDataViewerFactory, IInteractiveWindow, IInteractiveWindowInfo, IInteractiveWindowListener, @@ -53,6 +53,7 @@ import { IJupyterDebugger, IJupyterExecution, IJupyterKernelSpec, + IJupyterVariableDataProviderFactory, IJupyterVariables, INotebookExporter, INotebookProvider, @@ -101,7 +102,9 @@ export class InteractiveWindow extends InteractiveBase implements IInteractiveWi @inject(INotebookExporter) jupyterExporter: INotebookExporter, @inject(IWorkspaceService) workspaceService: IWorkspaceService, @inject(IInteractiveWindowProvider) private interactiveWindowProvider: IInteractiveWindowProvider, - @inject(IDataViewerProvider) dataExplorerProvider: IDataViewerProvider, + @inject(IDataViewerFactory) dataExplorerFactory: IDataViewerFactory, + @inject(IJupyterVariableDataProviderFactory) + jupyterVariableDataProviderFactory: IJupyterVariableDataProviderFactory, @inject(IJupyterVariables) @named(Identifiers.ALL_VARIABLES) jupyterVariables: IJupyterVariables, @inject(IJupyterDebugger) jupyterDebugger: IJupyterDebugger, @inject(IDataScienceErrorHandler) errorHandler: IDataScienceErrorHandler, @@ -127,7 +130,8 @@ export class InteractiveWindow extends InteractiveBase implements IInteractiveWi configuration, jupyterExporter, workspaceService, - dataExplorerProvider, + dataExplorerFactory, + jupyterVariableDataProviderFactory, jupyterVariables, jupyterDebugger, errorHandler, diff --git a/src/client/datascience/serviceRegistry.ts b/src/client/datascience/serviceRegistry.ts index 3b5ebd4ad77a..3ea4ae867935 100644 --- a/src/client/datascience/serviceRegistry.ts +++ b/src/client/datascience/serviceRegistry.ts @@ -19,7 +19,8 @@ import { Identifiers } from './constants'; import { ActiveEditorContextService } from './context/activeEditorContext'; import { DataViewer } from './data-viewing/dataViewer'; import { DataViewerDependencyService } from './data-viewing/dataViewerDependencyService'; -import { DataViewerProvider } from './data-viewing/dataViewerProvider'; +import { DataViewerFactory } from './data-viewing/dataViewerFactory'; +import { JupyterVariableDataProvider } from './data-viewing/jupyterVariableDataProvider'; import { DataScience } from './datascience'; import { DataScienceSurveyBannerLogger } from './dataScienceSurveyBanner'; import { DebugLocationTrackerFactory } from './debugLocationTrackerFactory'; @@ -115,7 +116,7 @@ import { IDataScienceCommandListener, IDataScienceErrorHandler, IDataViewer, - IDataViewerProvider, + IDataViewerFactory, IDebugLocationTracker, IGatherLogger, IGatherProvider, @@ -132,8 +133,12 @@ import { IJupyterServerProvider, IJupyterSessionManagerFactory, IJupyterSubCommandExecutionService, + IJupyterVariable, + IJupyterVariableDataProvider, + IJupyterVariableDataProviderFactory, IJupyterVariables, IKernelDependencyService, + INotebook, INotebookAndInteractiveWindowUsageTracker, INotebookEditor, INotebookEditorProvider, @@ -206,7 +211,7 @@ export function registerTypes(serviceManager: IServiceManager) { serviceManager.addSingleton(IDataScienceCodeLensProvider, DataScienceCodeLensProvider); serviceManager.addSingleton(IDataScienceCommandListener, InteractiveWindowCommandListener); serviceManager.addSingleton(IDataScienceCommandListener, NativeEditorCommandListener); - serviceManager.addSingleton(IDataViewerProvider, DataViewerProvider); + serviceManager.addSingleton(IDataViewerFactory, DataViewerFactory); serviceManager.addSingleton(IDebugLocationTracker, DebugLocationTrackerFactory); serviceManager.addSingleton(IExtensionSingleActivationService, Activation); serviceManager.addSingleton(IExtensionSingleActivationService, Decorator); @@ -256,6 +261,13 @@ export function registerTypes(serviceManager: IServiceManager) { serviceManager.add(IProtocolParser, ProtocolParser); serviceManager.addSingleton(IJupyterDebugService, MultiplexingDebugService, Identifiers.MULTIPLEXING_DEBUGSERVICE); serviceManager.addSingleton(IJupyterDebugService, JupyterDebugService, Identifiers.RUN_BY_LINE_DEBUGSERVICE); + serviceManager.addFactory(IJupyterVariableDataProviderFactory, (context) => { + return (variable: IJupyterVariable, notebook: INotebook) => { + const variableManager: IJupyterVariables = context.container.getNamed(IJupyterVariables, Identifiers.ALL_VARIABLES); + const dependencyService: DataViewerDependencyService = context.container.get(DataViewerDependencyService); + return new JupyterVariableDataProvider(variableManager, dependencyService, variable, notebook); + }; + }); registerGatherTypes(serviceManager); registerNotebookTypes(serviceManager); diff --git a/src/client/datascience/types.ts b/src/client/datascience/types.ts index b80d78395dcf..556d80363b7c 100644 --- a/src/client/datascience/types.ts +++ b/src/client/datascience/types.ts @@ -29,6 +29,7 @@ import { IAsyncDisposable, IDataScienceSettings, IDisposable, Resource } from '. import { StopWatch } from '../common/utils/stopWatch'; import { PythonInterpreter } from '../interpreter/contracts'; import { JupyterCommands } from './constants'; +import { IDataViewerDataProvider } from './data-viewing/types'; import { NotebookModelChange } from './interactive-common/interactiveWindowTypes'; import { JupyterServerInfo } from './jupyter/jupyterConnection'; import { JupyterInstallError } from './jupyter/jupyterInstallError'; @@ -780,6 +781,15 @@ export interface IJupyterVariable { indexColumn?: string; } +export const IJupyterVariableDataProvider = Symbol('IJupyterVariableDataProvider'); +export interface IJupyterVariableDataProvider extends IDataViewerDataProvider {} + +export const IJupyterVariableDataProviderFactory = Symbol('IJupyterVariableDataProviderFactory'); +export interface IJupyterVariableDataProviderFactory extends Function { + // tslint:disable-next-line: callable-types + (variable: IJupyterVariable, notebook: INotebook): IJupyterVariableDataProvider; +} + export const IJupyterVariables = Symbol('IJupyterVariables'); export interface IJupyterVariables { readonly refreshRequired: Event; @@ -819,14 +829,14 @@ export interface IJupyterVariablesResponse { pageResponse: IJupyterVariable[]; } -export const IDataViewerProvider = Symbol('IDataViewerProvider'); -export interface IDataViewerProvider { - create(variable: IJupyterVariable, notebook: INotebook): Promise; +export const IDataViewerFactory = Symbol('IDataViewerFactory'); +export interface IDataViewerFactory { + create(dataProvider: IDataViewerDataProvider, title: string): Promise; } -export const IDataViewer = Symbol('IDataViewer'); +export const IDataViewer = Symbol('IDataViewer'); export interface IDataViewer extends IDisposable { - showVariable(variable: IJupyterVariable, notebook: INotebook): Promise; + showData(dataProvider: IDataViewerDataProvider, title: string): Promise; } export const IPlotViewerProvider = Symbol('IPlotViewerProvider'); diff --git a/src/datascience-ui/data-explorer/cellFormatter.tsx b/src/datascience-ui/data-explorer/cellFormatter.tsx index 7ac5674fe51e..7c29d00a1265 100644 --- a/src/datascience-ui/data-explorer/cellFormatter.tsx +++ b/src/datascience-ui/data-explorer/cellFormatter.tsx @@ -5,6 +5,7 @@ import './cellFormatter.css'; import * as React from 'react'; import * as ReactDOMServer from 'react-dom/server'; +import { ColumnType } from '../../client/datascience/data-viewing/types'; import { ISlickRow } from './reactSlickGrid'; interface ICellFormatterProps { @@ -23,15 +24,11 @@ class CellFormatter extends React.Component { // tslint:disable-next-line: no-any const columnType = (this.props.columnDef as any).type; switch (columnType) { - case 'bool': + case ColumnType.Bool: return this.renderBool(this.props.value as boolean); break; - case 'integer': - case 'float': - case 'int64': - case 'float64': - case 'number': + case ColumnType.Number: return this.renderNumber(this.props.value as number); break; diff --git a/src/datascience-ui/data-explorer/mainPanel.tsx b/src/datascience-ui/data-explorer/mainPanel.tsx index 1f1a9ececb2c..25db35975fae 100644 --- a/src/datascience-ui/data-explorer/mainPanel.tsx +++ b/src/datascience-ui/data-explorer/mainPanel.tsx @@ -11,12 +11,14 @@ import { CellFetchAllLimit, CellFetchSizeFirst, CellFetchSizeSubsequent, + ColumnType, DataViewerMessages, + IDataFrameInfo, IDataViewerMapping, IGetRowsResponse } from '../../client/datascience/data-viewing/types'; import { SharedMessages } from '../../client/datascience/messages'; -import { IDataScienceExtraSettings, IJupyterVariable } from '../../client/datascience/types'; +import { IDataScienceExtraSettings } from '../../client/datascience/types'; import { getLocString, storeLocStrings } from '../react-common/locReactSide'; import { IMessageHandler, PostOffice } from '../react-common/postOffice'; import { Progress } from '../react-common/progress'; @@ -198,7 +200,7 @@ export class MainPanel extends React.Component private initializeData(payload: any) { // Payload should be an IJupyterVariable with the first 100 rows filled out if (payload) { - const variable = payload as IJupyterVariable; + const variable = payload as IDataFrameInfo; if (variable) { const columns = this.generateColumns(variable); const totalRowCount = variable.rowCount ? variable.rowCount : 0; @@ -285,9 +287,9 @@ export class MainPanel extends React.Component } } - private generateColumns(variable: IJupyterVariable): Slick.Column[] { + private generateColumns(variable: IDataFrameInfo): Slick.Column[] { if (variable.columns) { - return variable.columns.map((c: { key: string; type: string }, i: number) => { + return variable.columns.map((c: { key: string; type: ColumnType }, i: number) => { return { type: c.type, field: c.key.toString(), diff --git a/src/datascience-ui/data-explorer/reactSlickGrid.tsx b/src/datascience-ui/data-explorer/reactSlickGrid.tsx index ae11d3c69c10..516d36d2860b 100644 --- a/src/datascience-ui/data-explorer/reactSlickGrid.tsx +++ b/src/datascience-ui/data-explorer/reactSlickGrid.tsx @@ -4,7 +4,7 @@ import * as React from 'react'; import * as ReactDOM from 'react-dom'; -import { MaxStringCompare } from '../../client/datascience/data-viewing/types'; +import { ColumnType, MaxStringCompare } from '../../client/datascience/data-viewing/types'; import { KeyCodes } from '../react-common/constants'; import { measureText } from '../react-common/textMeasure'; import './globalJQueryImports'; @@ -19,6 +19,9 @@ const slickgridJQ = require('slickgrid/lib/jquery-1.11.2.min'); // Adding comments to ensure order of imports does not change due to auto formatters. // tslint:disable-next-line: ordered-imports +// Adding comments to ensure order of imports does not change due to auto formatters. +// tslint:disable-next-line: ordered-imports +import 'slickgrid/plugins/slick.autotooltips'; import 'slickgrid/slick.core'; // Adding comments to ensure order of imports does not change due to auto formatters. // tslint:disable-next-line: ordered-imports @@ -28,9 +31,6 @@ import 'slickgrid/slick.dataview'; import 'slickgrid/slick.grid'; // Adding comments to ensure order of imports does not change due to auto formatters. // tslint:disable-next-line: ordered-imports -import 'slickgrid/plugins/slick.autotooltips'; -// Adding comments to ensure order of imports does not change due to auto formatters. -// tslint:disable-next-line: ordered-imports import 'slickgrid/slick.grid.css'; // Make sure our css comes after the slick grid css. We override some of its styles. // tslint:disable-next-line: ordered-imports @@ -79,18 +79,12 @@ class ColumnFilter { if (text && text.length > 0) { const columnType = (column as any).type; switch (columnType) { - case 'string': + case ColumnType.String: default: this.matchFunc = (v: any) => !v || v.toString().includes(text); break; - case 'integer': - case 'float': - case 'int64': - case 'int32': - case 'float32': - case 'float64': - case 'number': + case ColumnType.Number: this.matchFunc = this.generateNumericOperation(text); break; } diff --git a/src/test/datascience/data-viewing/dataViewer.unit.test.ts b/src/test/datascience/data-viewing/dataViewer.unit.test.ts new file mode 100644 index 000000000000..a5c6ae8b723a --- /dev/null +++ b/src/test/datascience/data-viewing/dataViewer.unit.test.ts @@ -0,0 +1,75 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import { anything, instance, mock, verify, when } from 'ts-mockito'; +import { ConfigurationChangeEvent, EventEmitter } from 'vscode'; +import { ApplicationShell } from '../../../client/common/application/applicationShell'; +import { IApplicationShell, IWebPanelProvider, IWorkspaceService } from '../../../client/common/application/types'; +import { WebPanelProvider } from '../../../client/common/application/webPanels/webPanelProvider'; +import { WorkspaceService } from '../../../client/common/application/workspace'; +import { PythonSettings } from '../../../client/common/configSettings'; +import { ConfigurationService } from '../../../client/common/configuration/service'; +import { ExperimentsManager } from '../../../client/common/experiments/manager'; +import { IConfigurationService, IExperimentsManager } from '../../../client/common/types'; +import { CodeCssGenerator } from '../../../client/datascience/codeCssGenerator'; +import { DataViewer } from '../../../client/datascience/data-viewing/dataViewer'; +import { JupyterVariableDataProvider } from '../../../client/datascience/data-viewing/jupyterVariableDataProvider'; +import { IDataViewerDataProvider } from '../../../client/datascience/data-viewing/types'; +import { ThemeFinder } from '../../../client/datascience/themeFinder'; +import { ICodeCssGenerator, IDataViewer, IThemeFinder } from '../../../client/datascience/types'; + +suite('Data Science - DataViewer', () => { + let dataViewer: IDataViewer; + let webPanelProvider: IWebPanelProvider; + let configService: IConfigurationService; + let codeCssGenerator: ICodeCssGenerator; + let themeFinder: IThemeFinder; + let workspaceService: IWorkspaceService; + let applicationShell: IApplicationShell; + let experimentManager: IExperimentsManager; + let dataProvider: IDataViewerDataProvider; + const title: string = 'Data Viewer - Title'; + + setup(async () => { + webPanelProvider = mock(WebPanelProvider); + configService = mock(ConfigurationService); + codeCssGenerator = mock(CodeCssGenerator); + themeFinder = mock(ThemeFinder); + workspaceService = mock(WorkspaceService); + applicationShell = mock(ApplicationShell); + experimentManager = mock(ExperimentsManager); + dataProvider = mock(JupyterVariableDataProvider); + const settings = mock(PythonSettings); + const settingsChangedEvent = new EventEmitter(); + + when(settings.onDidChange).thenReturn(settingsChangedEvent.event); + when(configService.getSettings(anything())).thenReturn(instance(settings)); + + const configChangeEvent = new EventEmitter(); + when(workspaceService.onDidChangeConfiguration).thenReturn(configChangeEvent.event); + + dataViewer = new DataViewer( + instance(webPanelProvider), + instance(configService), + instance(codeCssGenerator), + instance(themeFinder), + instance(workspaceService), + instance(applicationShell), + instance(experimentManager), + false + ); + }); + test('Data viewer showData calls gets dataFrame info from data provider', async () => { + await dataViewer.showData(instance(dataProvider), title); + + verify(dataProvider.getDataFrameInfo()).once(); + }); + test('Data viewer calls data provider dispose', async () => { + await dataViewer.showData(instance(dataProvider), title); + dataViewer.dispose(); + + verify(dataProvider.dispose()).once(); + }); +}); diff --git a/src/test/datascience/data-viewing/dataViewerProvider.unit.test.ts b/src/test/datascience/data-viewing/dataViewerProvider.unit.test.ts deleted file mode 100644 index deaa544a2f35..000000000000 --- a/src/test/datascience/data-viewing/dataViewerProvider.unit.test.ts +++ /dev/null @@ -1,80 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -'use strict'; - -import { assert } from 'chai'; -import * as path from 'path'; -import { SemVer } from 'semver'; -import { anything, instance, mock, verify, when } from 'ts-mockito'; -import { AsyncDisposableRegistry } from '../../../client/common/asyncDisposableRegistry'; -import { Architecture } from '../../../client/common/utils/platform'; -import { DataViewer } from '../../../client/datascience/data-viewing/dataViewer'; -import { DataViewerDependencyService } from '../../../client/datascience/data-viewing/dataViewerDependencyService'; -import { DataViewerProvider } from '../../../client/datascience/data-viewing/dataViewerProvider'; -import { JupyterNotebookBase } from '../../../client/datascience/jupyter/jupyterNotebook'; -import { IDataViewer, IJupyterVariable, INotebook } from '../../../client/datascience/types'; -import { InterpreterType, PythonInterpreter } from '../../../client/interpreter/contracts'; -import { ServiceContainer } from '../../../client/ioc/container'; -import { IServiceContainer } from '../../../client/ioc/types'; - -suite('Data Science - DataViewerProvider', () => { - let dataViewerProvider: DataViewerProvider; - let dataViewer: IDataViewer; - let serviceContainer: IServiceContainer; - let dependencyService: DataViewerDependencyService; - let jupyterVariable: IJupyterVariable; - let notebook: INotebook; - let interpreter: PythonInterpreter; - setup(async () => { - jupyterVariable = { - count: 1, - name: '', - shape: '', - size: 1, - supportsDataExplorer: true, - truncated: false, - type: '', - value: '' - }; - interpreter = { - architecture: Architecture.Unknown, - displayName: '', - path: path.join('users', 'python', 'bin', 'python.exe'), - sysPrefix: '', - sysVersion: '', - type: InterpreterType.Unknown, - version: new SemVer('3.3.3') - }; - notebook = mock(JupyterNotebookBase); - serviceContainer = mock(ServiceContainer); - dataViewer = mock(DataViewer); - // tslint:disable-next-line: no-any - (instance(dataViewer) as any).then = undefined; - // tslint:disable-next-line: no-any - // (dataViewer as any).then = undefined; - const asyncRegistry = mock(AsyncDisposableRegistry); - dependencyService = mock(DataViewerDependencyService); - when(serviceContainer.get(IDataViewer)).thenReturn(instance(dataViewer)); - dataViewerProvider = new DataViewerProvider( - instance(serviceContainer), - instance(asyncRegistry), - instance(dependencyService) - ); - }); - test('Check and install missing dependencies before displaying variable explorer', async () => { - const callOrder: string[] = []; - when(notebook.getMatchingInterpreter()).thenReturn(interpreter); - when(dependencyService.checkAndInstallMissingDependencies(anything())).thenCall(() => - callOrder.push('First Check') - ); - when(dataViewer.showVariable(anything(), anything())).thenCall(() => callOrder.push('Then Show')); - - await dataViewerProvider.create(jupyterVariable, instance(notebook)); - - verify(dependencyService.checkAndInstallMissingDependencies(interpreter)).once(); - verify(dataViewer.showVariable(jupyterVariable, instance(notebook))).once(); - // Couldn't get `calledBefore` working, hence a diryt simple hack. - assert.deepEqual(callOrder, ['First Check', 'Then Show']); - }); -}); diff --git a/src/test/datascience/dataScienceIocContainer.ts b/src/test/datascience/dataScienceIocContainer.ts index 4f00d2d19722..1fad979375ff 100644 --- a/src/test/datascience/dataScienceIocContainer.ts +++ b/src/test/datascience/dataScienceIocContainer.ts @@ -179,7 +179,8 @@ import { Identifiers, JUPYTER_OUTPUT_CHANNEL } from '../../client/datascience/co import { ActiveEditorContextService } from '../../client/datascience/context/activeEditorContext'; import { DataViewer } from '../../client/datascience/data-viewing/dataViewer'; import { DataViewerDependencyService } from '../../client/datascience/data-viewing/dataViewerDependencyService'; -import { DataViewerProvider } from '../../client/datascience/data-viewing/dataViewerProvider'; +import { DataViewerFactory } from '../../client/datascience/data-viewing/dataViewerFactory'; +import { JupyterVariableDataProvider } from '../../client/datascience/data-viewing/jupyterVariableDataProvider'; import { DebugLocationTrackerFactory } from '../../client/datascience/debugLocationTrackerFactory'; import { CellHashProvider } from '../../client/datascience/editor-integration/cellhashprovider'; import { CodeLensFactory } from '../../client/datascience/editor-integration/codeLensFactory'; @@ -262,7 +263,7 @@ import { IDataScienceCommandListener, IDataScienceErrorHandler, IDataViewer, - IDataViewerProvider, + IDataViewerFactory, IDebugLocationTracker, IGatherLogger, IGatherProvider, @@ -279,8 +280,12 @@ import { IJupyterServerProvider, IJupyterSessionManagerFactory, IJupyterSubCommandExecutionService, + IJupyterVariable, + IJupyterVariableDataProvider, + IJupyterVariableDataProviderFactory, IJupyterVariables, IKernelDependencyService, + INotebook, INotebookAndInteractiveWindowUsageTracker, INotebookEditor, INotebookEditorProvider, @@ -608,7 +613,23 @@ export class DataScienceIocContainer extends UnitTestIocContainer { ); this.serviceManager.addSingletonInstance(UseProposedApi, false); this.serviceManager.addSingletonInstance(UseCustomEditorApi, useCustomEditor); - this.serviceManager.addSingleton(IDataViewerProvider, DataViewerProvider); + this.serviceManager.addSingleton(IDataViewerFactory, DataViewerFactory); + this.serviceManager.addFactory(IJupyterVariableDataProviderFactory, (context) => { + return (variable: IJupyterVariable, notebook: INotebook) => { + const variableManager: IJupyterVariables = context.container.getNamed( + IJupyterVariables, + Identifiers.ALL_VARIABLES + ); + const dependencyService: DataViewerDependencyService = context.container.get( + DataViewerDependencyService + ); + return new JupyterVariableDataProvider(variableManager, dependencyService, variable, notebook); + }; + }); + this.serviceManager.add( + IJupyterVariableDataProvider, + JupyterVariableDataProvider + ); this.serviceManager.addSingleton(IPlotViewerProvider, PlotViewerProvider); this.serviceManager.add(IInteractiveWindow, InteractiveWindow); this.serviceManager.add(IDataViewer, DataViewer); diff --git a/src/test/datascience/dataviewer.functional.test.tsx b/src/test/datascience/dataviewer.functional.test.tsx index 04fa2d1c8575..823b93309b8d 100644 --- a/src/test/datascience/dataviewer.functional.test.tsx +++ b/src/test/datascience/dataviewer.functional.test.tsx @@ -13,8 +13,16 @@ import * as uuid from 'uuid/v4'; import { Disposable, Uri } from 'vscode'; import { Identifiers } from '../../client/datascience/constants'; -import { DataViewerMessages } from '../../client/datascience/data-viewing/types'; -import { IDataViewer, IDataViewerProvider, INotebook, INotebookProvider } from '../../client/datascience/types'; +import { DataViewerMessages, IDataViewerDataProvider } from '../../client/datascience/data-viewing/types'; +import { + IDataViewer, + IDataViewerFactory, + IJupyterVariable, + IJupyterVariableDataProvider, + IJupyterVariableDataProviderFactory, + INotebook, + INotebookProvider +} from '../../client/datascience/types'; import { MainPanel } from '../../datascience-ui/data-explorer/mainPanel'; import { ReactSlickGrid } from '../../datascience-ui/data-explorer/reactSlickGrid'; import { noop, sleep } from '../core'; @@ -25,7 +33,8 @@ import { waitForMessage } from './testHelpers'; // import { asyncDump } from '../common/asyncDump'; suite('DataScience DataViewer tests', () => { const disposables: Disposable[] = []; - let dataProvider: IDataViewerProvider; + let dataViewerFactory: IDataViewerFactory; + let jupyterVariableDataProviderFactory: IJupyterVariableDataProviderFactory; let ioc: DataScienceIocContainer; let notebook: INotebook | undefined; const snapshot = takeSnapshot(); @@ -57,7 +66,10 @@ suite('DataScience DataViewer tests', () => { ioc.createWebView(() => mount()); // Make sure the data explorer provider and execution factory in the container is created (the extension does this on startup in the extension) - dataProvider = ioc.get(IDataViewerProvider); + dataViewerFactory = ioc.get(IDataViewerFactory); + jupyterVariableDataProviderFactory = ioc.get( + IJupyterVariableDataProviderFactory + ); return ioc.wrapper!; } @@ -81,20 +93,35 @@ suite('DataScience DataViewer tests', () => { // asyncDump(); }); - async function createDataViewer(variable: string, type: string): Promise { - return dataProvider.create( - { - name: variable, - value: '', - supportsDataExplorer: true, - type, - size: 0, - truncated: true, - shape: '', - count: 0 - }, - notebook! + function createJupyterVariable(variable: string, type: string): IJupyterVariable { + return { + name: variable, + value: '', + supportsDataExplorer: true, + type, + size: 0, + truncated: true, + shape: '', + count: 0 + }; + } + + async function createJupyterVariableDataProvider( + jupyterVariable: IJupyterVariable + ): Promise { + return jupyterVariableDataProviderFactory(jupyterVariable, notebook!); + } + + async function createDataViewer(dataProvider: IDataViewerDataProvider, title: string): Promise { + return dataViewerFactory.create(dataProvider, title); + } + + async function createJupyterVariableDataViewer(variable: string, type: string): Promise { + const jupyterVariable: IJupyterVariable = createJupyterVariable(variable, type); + const jupyterVariableDataProvider: IJupyterVariableDataProvider = await createJupyterVariableDataProvider( + jupyterVariable ); + return createDataViewer(jupyterVariableDataProvider, jupyterVariable.name); } async function injectCode(code: string): Promise { @@ -209,7 +236,7 @@ suite('DataScience DataViewer tests', () => { runMountedTest('Data Frame', async (wrapper) => { await injectCode('import pandas as pd\r\ndf = pd.DataFrame([0, 1, 2, 3])'); const gotAllRows = getCompletedPromise(); - const dv = await createDataViewer('df', 'DataFrame'); + const dv = await createJupyterVariableDataViewer('df', 'DataFrame'); assert.ok(dv, 'DataViewer not created'); await gotAllRows; @@ -219,7 +246,7 @@ suite('DataScience DataViewer tests', () => { runMountedTest('List', async (wrapper) => { await injectCode('ls = [0, 1, 2, 3]'); const gotAllRows = getCompletedPromise(); - const dv = await createDataViewer('ls', 'list'); + const dv = await createJupyterVariableDataViewer('ls', 'list'); assert.ok(dv, 'DataViewer not created'); await gotAllRows; @@ -229,7 +256,7 @@ suite('DataScience DataViewer tests', () => { runMountedTest('Series', async (wrapper) => { await injectCode('import pandas as pd\r\ns = pd.Series([0, 1, 2, 3])'); const gotAllRows = getCompletedPromise(); - const dv = await createDataViewer('s', 'Series'); + const dv = await createJupyterVariableDataViewer('s', 'Series'); assert.ok(dv, 'DataViewer not created'); await gotAllRows; @@ -239,7 +266,7 @@ suite('DataScience DataViewer tests', () => { runMountedTest('np.array', async (wrapper) => { await injectCode('import numpy as np\r\nx = np.array([0, 1, 2, 3])'); const gotAllRows = getCompletedPromise(); - const dv = await createDataViewer('x', 'ndarray'); + const dv = await createJupyterVariableDataViewer('x', 'ndarray'); assert.ok(dv, 'DataViewer not created'); await gotAllRows; @@ -249,7 +276,7 @@ suite('DataScience DataViewer tests', () => { runMountedTest('Failure', async (_wrapper) => { await injectCode('import numpy as np\r\nx = np.array([0, 1, 2, 3])'); try { - await createDataViewer('unknown variable', 'ndarray'); + await createJupyterVariableDataViewer('unknown variable', 'ndarray'); assert.fail('Exception should have been thrown'); } catch { noop(); @@ -259,7 +286,7 @@ suite('DataScience DataViewer tests', () => { runMountedTest('Sorting', async (wrapper) => { await injectCode('import numpy as np\r\nx = np.array([0, 1, 2, 3])'); const gotAllRows = getCompletedPromise(); - const dv = await createDataViewer('x', 'ndarray'); + const dv = await createJupyterVariableDataViewer('x', 'ndarray'); assert.ok(dv, 'DataViewer not created'); await gotAllRows; @@ -271,7 +298,7 @@ suite('DataScience DataViewer tests', () => { runMountedTest('Filter', async (wrapper) => { await injectCode('import numpy as np\r\nx = np.array([0, 1, 2, 3])'); const gotAllRows = getCompletedPromise(); - const dv = await createDataViewer('x', 'ndarray'); + const dv = await createJupyterVariableDataViewer('x', 'ndarray'); assert.ok(dv, 'DataViewer not created'); await gotAllRows; From 85783ffcc99868631f977c20771a19f0aff3800b Mon Sep 17 00:00:00 2001 From: Sergio Villalobos Date: Thu, 28 May 2020 15:57:47 -0700 Subject: [PATCH 2/5] Issue#12033 Add headers and comments to important updated/new code --- src/client/api.ts | 6 ++++++ src/client/datascience/data-viewing/dataViewer.ts | 2 ++ src/client/datascience/data-viewing/dataViewerFactory.ts | 3 ++- .../data-viewing/jupyterVariableDataProvider.ts | 7 +++++++ 4 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/client/api.ts b/src/client/api.ts index c6f88172c351..926a77beb05f 100644 --- a/src/client/api.ts +++ b/src/client/api.ts @@ -67,6 +67,12 @@ export interface IExtensionApi { getExecutionCommand(resource?: Resource): string[] | undefined; }; datascience: { + /** + * Launches Data Viewer component. + * @param {IDataViewerDataProvider} dataProvider Instance that will be used by the Data Viewer component to fetch data. + * @param {string} title Data Viewer title + * @returns {Promise} + */ showDataViewer(dataProvider: IDataViewerDataProvider, title: string): Promise; }; } diff --git a/src/client/datascience/data-viewing/dataViewer.ts b/src/client/datascience/data-viewing/dataViewer.ts index a88a2036df68..5da9cbcb0968 100644 --- a/src/client/datascience/data-viewing/dataViewer.ts +++ b/src/client/datascience/data-viewing/dataViewer.ts @@ -73,6 +73,7 @@ export class DataViewer extends WebViewHost implements IData public async showData(dataProvider: IDataViewerDataProvider, title: string): Promise { if (!this.isDisposed) { + // Save the data provider this.dataProvider = dataProvider; // Load the web panel using our current directory as we don't expect to load any other files @@ -94,6 +95,7 @@ export class DataViewer extends WebViewHost implements IData super.dispose(); if (this.dataProvider) { + // Call dispose on the data provider this.dataProvider.dispose(); this.dataProvider = undefined; } diff --git a/src/client/datascience/data-viewing/dataViewerFactory.ts b/src/client/datascience/data-viewing/dataViewerFactory.ts index a4b102ffba77..cbd239674fd2 100644 --- a/src/client/datascience/data-viewing/dataViewerFactory.ts +++ b/src/client/datascience/data-viewing/dataViewerFactory.ts @@ -27,12 +27,13 @@ export class DataViewerFactory implements IDataViewerFactory, IAsyncDisposable { public async create(dataProvider: IDataViewerDataProvider, title: string): Promise { let result: IDataViewer | undefined; - // Create the data explorer (this should show the window) + // Create the data explorer const dataExplorer = this.serviceContainer.get(IDataViewer); try { // Then load the data. this.activeExplorers.push(dataExplorer); + // Show the window and the data await dataExplorer.showData(dataProvider, title); result = dataExplorer; } finally { diff --git a/src/client/datascience/data-viewing/jupyterVariableDataProvider.ts b/src/client/datascience/data-viewing/jupyterVariableDataProvider.ts index 1e163576939d..3b5f9899a6af 100644 --- a/src/client/datascience/data-viewing/jupyterVariableDataProvider.ts +++ b/src/client/datascience/data-viewing/jupyterVariableDataProvider.ts @@ -23,6 +23,12 @@ export class JupyterVariableDataProvider implements IJupyterVariableDataProvider private notebook: INotebook ) {} + /** + * Normalizes column types to the types the UI component understands. + * Defaults to 'string'. + * @param columns + * @returns Array of columns with normalized type + */ private static getNormalizedColumns(columns: { key: string; type: string }[]): { key: string; type: ColumnType }[] { return columns.map((column: { key: string; type: string }) => { let normalizedType: ColumnType; @@ -88,6 +94,7 @@ export class JupyterVariableDataProvider implements IJupyterVariableDataProvider } private async ensureInitialized(): Promise { + // Postpone pre-req and variable initialization until data is requested. if (!this.initialized) { await this.dependencyService.checkAndInstallMissingDependencies(this.notebook.getMatchingInterpreter()); this.variable = await this.variableManager.getDataFrameInfo(this.variable, this.notebook); From 3a6ad6c97457a7b9238b0a677e236e4c9bfc9bf7 Mon Sep 17 00:00:00 2001 From: Sergio Villalobos Date: Fri, 29 May 2020 12:00:33 -0700 Subject: [PATCH 3/5] Issue12033 Fix review comments: 1. Do not trim title in DataViewer 2. More strucuted type to represent rows responses from IDataViewerDataProvider. Created IRowsResponse type {data: any[]} 3. Change createion of JupyterVariableDataProvider through a Factory Class 4. Fix initialization lock/flag in jupyterVariableDataProvider to ensure only one execution 5. showDataViewer API return void 6. Cache dataFrameInfo in DataViewer to prevent multiple fetches 7. Fix automotter change in reactSlickGrid.txt imports 8. Move IDataViewer and IDataViewerFactory to data-viewing/types --- src/client/api.ts | 10 ++-- .../datascience/data-viewing/dataViewer.ts | 30 ++++++------ .../data-viewing/dataViewerFactory.ts | 3 +- .../jupyterVariableDataProvider.ts | 47 +++++++++++-------- .../jupyterVariableDataProviderFactory.ts | 27 +++++++++++ src/client/datascience/data-viewing/types.ts | 27 ++++++++--- .../interactive-common/interactiveBase.ts | 5 +- .../interactive-ipynb/nativeEditor.ts | 2 +- .../nativeEditorOldWebView.ts | 2 +- .../interactive-window/interactiveWindow.ts | 2 +- src/client/datascience/serviceRegistry.ts | 15 ++---- src/client/datascience/types.ts | 19 ++------ .../data-explorer/mainPanel.tsx | 9 ++-- .../data-explorer/reactSlickGrid.tsx | 6 +-- .../data-viewing/dataViewer.unit.test.ts | 4 +- .../datascience/dataScienceIocContainer.ts | 22 +++------ .../dataviewer.functional.test.tsx | 14 +++--- 17 files changed, 134 insertions(+), 110 deletions(-) create mode 100644 src/client/datascience/data-viewing/jupyterVariableDataProviderFactory.ts diff --git a/src/client/api.ts b/src/client/api.ts index 926a77beb05f..7ffd8ac193d3 100644 --- a/src/client/api.ts +++ b/src/client/api.ts @@ -7,8 +7,7 @@ import { isTestExecution } from './common/constants'; import { DebugAdapterNewPtvsd } from './common/experiments/groups'; import { traceError } from './common/logger'; import { IConfigurationService, IExperimentsManager, Resource } from './common/types'; -import { IDataViewerDataProvider } from './datascience/data-viewing/types'; -import { IDataViewer, IDataViewerFactory } from './datascience/types'; +import { IDataViewerDataProvider, IDataViewerFactory } from './datascience/data-viewing/types'; import { getDebugpyLauncherArgs, getDebugpyPackagePath, @@ -71,9 +70,8 @@ export interface IExtensionApi { * Launches Data Viewer component. * @param {IDataViewerDataProvider} dataProvider Instance that will be used by the Data Viewer component to fetch data. * @param {string} title Data Viewer title - * @returns {Promise} */ - showDataViewer(dataProvider: IDataViewerDataProvider, title: string): Promise; + showDataViewer(dataProvider: IDataViewerDataProvider, title: string): Promise; }; } @@ -131,9 +129,9 @@ export function buildApi( } }, datascience: { - async showDataViewer(dataProvider: IDataViewerDataProvider, title: string) { + async showDataViewer(dataProvider: IDataViewerDataProvider, title: string): Promise { const dataViewerProviderService = serviceContainer.get(IDataViewerFactory); - return dataViewerProviderService.create(dataProvider, title); + await dataViewerProviderService.create(dataProvider, title); } } }; diff --git a/src/client/datascience/data-viewing/dataViewer.ts b/src/client/datascience/data-viewing/dataViewer.ts index 5da9cbcb0968..7691ea688639 100644 --- a/src/client/datascience/data-viewing/dataViewer.ts +++ b/src/client/datascience/data-viewing/dataViewer.ts @@ -18,12 +18,13 @@ import { StopWatch } from '../../common/utils/stopWatch'; import { sendTelemetryEvent } from '../../telemetry'; import { HelpLinks, Telemetry } from '../constants'; import { JupyterDataRateLimitError } from '../jupyter/jupyterDataRateLimitError'; -import { ICodeCssGenerator, IDataViewer, IThemeFinder } from '../types'; +import { ICodeCssGenerator, IThemeFinder } from '../types'; import { WebViewHost } from '../webViewHost'; import { DataViewerMessageListener } from './dataViewerMessageListener'; import { DataViewerMessages, IDataFrameInfo, + IDataViewer, IDataViewerDataProvider, IDataViewerMapping, IGetRowsRequest @@ -35,6 +36,7 @@ export class DataViewer extends WebViewHost implements IData private dataProvider: IDataViewerDataProvider | undefined; private rowsTimer: StopWatch | undefined; private pendingRowsCount: number = 0; + private dataFrameInfoPromise: Promise | undefined; constructor( @inject(IWebPanelProvider) provider: IWebPanelProvider, @@ -63,14 +65,6 @@ export class DataViewer extends WebViewHost implements IData ); } - private static trimTitle(title: string): string { - const TRIM_LENGTH = 40; - if (title && title.length > TRIM_LENGTH) { - title = `${title.substr(0, TRIM_LENGTH)}...`; - } - return title; - } - public async showData(dataProvider: IDataViewerDataProvider, title: string): Promise { if (!this.isDisposed) { // Save the data provider @@ -79,12 +73,12 @@ export class DataViewer extends WebViewHost implements IData // Load the web panel using our current directory as we don't expect to load any other files await super.loadWebPanel(process.cwd()).catch(traceError); - super.setTitle(DataViewer.trimTitle(title)); + super.setTitle(title); // Then show our web panel. Eventually we need to consume the data await super.show(true); - const dataFrameInfo = await this.prepDataFrameInfo(dataProvider); + const dataFrameInfo = await this.prepDataFrameInfo(); // Send a message with our data this.postMessage(DataViewerMessages.InitializeData, dataFrameInfo).ignoreErrors(); @@ -123,9 +117,16 @@ export class DataViewer extends WebViewHost implements IData super.onMessage(message, payload); } - private async prepDataFrameInfo(dataProvider: IDataViewerDataProvider): Promise { + private getDataFrameInfo(): Promise { + if (!this.dataFrameInfoPromise) { + this.dataFrameInfoPromise = this.dataProvider ? this.dataProvider.getDataFrameInfo() : Promise.resolve({}); + } + return this.dataFrameInfoPromise; + } + + private async prepDataFrameInfo(): Promise { this.rowsTimer = new StopWatch(); - const output = await dataProvider.getDataFrameInfo(); + const output = await this.getDataFrameInfo(); // Log telemetry about number of rows try { @@ -156,9 +157,10 @@ export class DataViewer extends WebViewHost implements IData private getRowChunk(request: IGetRowsRequest) { return this.wrapRequest(async () => { if (this.dataProvider) { + const dataFrameInfo = await this.getDataFrameInfo(); const rows = await this.dataProvider.getRows( request.start, - Math.min(request.end, (await this.dataProvider.getDataFrameInfo()).rowCount) + Math.min(request.end, dataFrameInfo.rowCount ? dataFrameInfo.rowCount : 0) ); return this.postMessage(DataViewerMessages.GetRowsResponse, { rows, diff --git a/src/client/datascience/data-viewing/dataViewerFactory.ts b/src/client/datascience/data-viewing/dataViewerFactory.ts index cbd239674fd2..0f08a0da6f1e 100644 --- a/src/client/datascience/data-viewing/dataViewerFactory.ts +++ b/src/client/datascience/data-viewing/dataViewerFactory.ts @@ -7,8 +7,7 @@ import { inject, injectable } from 'inversify'; import { IAsyncDisposable, IAsyncDisposableRegistry } from '../../common/types'; import { IServiceContainer } from '../../ioc/types'; -import { IDataViewer, IDataViewerFactory } from '../types'; -import { IDataViewerDataProvider } from './types'; +import { IDataViewer, IDataViewerFactory, IDataViewerDataProvider } from './types'; @injectable() export class DataViewerFactory implements IDataViewerFactory, IAsyncDisposable { diff --git a/src/client/datascience/data-viewing/jupyterVariableDataProvider.ts b/src/client/datascience/data-viewing/jupyterVariableDataProvider.ts index 3b5f9899a6af..2c9d9257af06 100644 --- a/src/client/datascience/data-viewing/jupyterVariableDataProvider.ts +++ b/src/client/datascience/data-viewing/jupyterVariableDataProvider.ts @@ -5,22 +5,20 @@ import '../../common/extensions'; import { inject, injectable, named } from 'inversify'; -import { JSONObject } from '@phosphor/coreutils'; - import { Identifiers } from '../constants'; -import { IJupyterVariable, IJupyterVariableDataProvider, IJupyterVariables, INotebook } from '../types'; +import { IJupyterVariable, IJupyterVariables, INotebook, IJupyterVariableDataProvider } from '../types'; import { DataViewerDependencyService } from './dataViewerDependencyService'; -import { ColumnType, IDataFrameInfo } from './types'; +import { ColumnType, IDataFrameInfo, IRowsResponse } from './types'; @injectable() export class JupyterVariableDataProvider implements IJupyterVariableDataProvider { private initialized: boolean = false; + private notebook: INotebook | undefined; + private variable: IJupyterVariable | undefined; constructor( @inject(IJupyterVariables) @named(Identifiers.ALL_VARIABLES) private variableManager: IJupyterVariables, - @inject(DataViewerDependencyService) private dependencyService: DataViewerDependencyService, - private variable: IJupyterVariable, - private notebook: INotebook + @inject(DataViewerDependencyService) private dependencyService: DataViewerDependencyService ) {} /** @@ -59,21 +57,30 @@ export class JupyterVariableDataProvider implements IJupyterVariableDataProvider return; } + public setDependencies(variable: IJupyterVariable, notebook: INotebook): void { + this.notebook = notebook; + this.variable = variable; + } + public async getDataFrameInfo(): Promise { + let dataFrameInfo: IDataFrameInfo = {}; await this.ensureInitialized(); - return { - columns: this.variable.columns - ? JupyterVariableDataProvider.getNormalizedColumns(this.variable.columns) - : this.variable.columns, - indexColumn: this.variable.indexColumn, - rowCount: this.variable.rowCount || 0 - }; + if (this.variable && this.notebook) { + dataFrameInfo = { + columns: this.variable.columns + ? JupyterVariableDataProvider.getNormalizedColumns(this.variable.columns) + : this.variable.columns, + indexColumn: this.variable.indexColumn, + rowCount: this.variable.rowCount + }; + } + return dataFrameInfo; } public async getAllRows() { - let allRows: JSONObject = {}; + let allRows: IRowsResponse = {}; await this.ensureInitialized(); - if (this.variable.rowCount) { + if (this.variable && this.variable.rowCount && this.notebook) { allRows = await this.variableManager.getDataFrameRows( this.variable, this.notebook, @@ -85,9 +92,9 @@ export class JupyterVariableDataProvider implements IJupyterVariableDataProvider } public async getRows(start: number, end: number) { - let rows: JSONObject = {}; + let rows: IRowsResponse = {}; await this.ensureInitialized(); - if (this.variable.rowCount) { + if (this.variable && this.variable.rowCount && this.notebook) { rows = await this.variableManager.getDataFrameRows(this.variable, this.notebook, start, end); } return rows; @@ -95,10 +102,10 @@ export class JupyterVariableDataProvider implements IJupyterVariableDataProvider private async ensureInitialized(): Promise { // Postpone pre-req and variable initialization until data is requested. - if (!this.initialized) { + if (!this.initialized && this.variable && this.notebook) { + this.initialized = true; await this.dependencyService.checkAndInstallMissingDependencies(this.notebook.getMatchingInterpreter()); this.variable = await this.variableManager.getDataFrameInfo(this.variable, this.notebook); - this.initialized = true; } } } diff --git a/src/client/datascience/data-viewing/jupyterVariableDataProviderFactory.ts b/src/client/datascience/data-viewing/jupyterVariableDataProviderFactory.ts new file mode 100644 index 000000000000..b8b48b45a4b3 --- /dev/null +++ b/src/client/datascience/data-viewing/jupyterVariableDataProviderFactory.ts @@ -0,0 +1,27 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +'use strict'; +import '../../common/extensions'; + +import { inject, injectable } from 'inversify'; + +import { IServiceContainer } from '../../ioc/types'; +import { + IJupyterVariable, + IJupyterVariableDataProvider, + IJupyterVariableDataProviderFactory, + INotebook +} from '../types'; + +@injectable() +export class JupyterVariableDataProviderFactory implements IJupyterVariableDataProviderFactory { + constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer) {} + + public async create(variable: IJupyterVariable, notebook: INotebook): Promise { + const jupyterVariableDataProvider = this.serviceContainer.get( + IJupyterVariableDataProvider + ); + jupyterVariableDataProvider.setDependencies(variable, notebook); + return jupyterVariableDataProvider; + } +} diff --git a/src/client/datascience/data-viewing/types.ts b/src/client/datascience/data-viewing/types.ts index e20421dcb1bf..4747fcf24af9 100644 --- a/src/client/datascience/data-viewing/types.ts +++ b/src/client/datascience/data-viewing/types.ts @@ -2,7 +2,7 @@ // Licensed under the MIT License. 'use strict'; -import { JSONObject } from '@phosphor/coreutils'; +import { IDisposable } from '../../common/types'; import { SharedMessages } from '../messages'; export const CellFetchAllLimit = 100000; @@ -33,7 +33,7 @@ export interface IGetRowsRequest { } export interface IGetRowsResponse { - rows: JSONObject; + rows: IRowsResponse; start: number; end: number; } @@ -44,7 +44,7 @@ export type IDataViewerMapping = { [DataViewerMessages.UpdateSettings]: string; [DataViewerMessages.InitializeData]: IDataFrameInfo; [DataViewerMessages.GetAllRowsRequest]: never | undefined; - [DataViewerMessages.GetAllRowsResponse]: JSONObject; + [DataViewerMessages.GetAllRowsResponse]: IRowsResponse; [DataViewerMessages.GetRowsRequest]: IGetRowsRequest; [DataViewerMessages.GetRowsResponse]: IGetRowsResponse; [DataViewerMessages.CompletedData]: never | undefined; @@ -53,14 +53,14 @@ export type IDataViewerMapping = { export interface IDataFrameInfo { columns?: { key: string; type: ColumnType }[]; indexColumn?: string; - rowCount: number; + rowCount?: number; } export interface IDataViewerDataProvider { dispose(): void; getDataFrameInfo(): Promise; - getAllRows(): Promise; - getRows(start: number, end: number): Promise; + getAllRows(): Promise; + getRows(start: number, end: number): Promise; } export enum ColumnType { @@ -68,3 +68,18 @@ export enum ColumnType { Number = 'number', Bool = 'bool' } + +export interface IRowsResponse { + // tslint:disable-next-line: no-any + data?: any[]; +} + +export const IDataViewerFactory = Symbol('IDataViewerFactory'); +export interface IDataViewerFactory { + create(dataProvider: IDataViewerDataProvider, title: string): Promise; +} + +export const IDataViewer = Symbol('IDataViewer'); +export interface IDataViewer extends IDisposable { + showData(dataProvider: IDataViewerDataProvider, title: string): Promise; +} diff --git a/src/client/datascience/interactive-common/interactiveBase.ts b/src/client/datascience/interactive-common/interactiveBase.ts index cfc8aa3dc4a9..facd6a872df4 100644 --- a/src/client/datascience/interactive-common/interactiveBase.ts +++ b/src/client/datascience/interactive-common/interactiveBase.ts @@ -49,7 +49,7 @@ import { generateCellRangesFromDocument } from '../cellFactory'; import { CellMatcher } from '../cellMatcher'; import { addToUriList } from '../common'; import { Commands, Identifiers, Telemetry } from '../constants'; -import { ColumnWarningSize } from '../data-viewing/types'; +import { ColumnWarningSize, IDataViewerFactory } from '../data-viewing/types'; import { IAddedSysInfo, ICopyCode, @@ -74,7 +74,6 @@ import { ICell, ICodeCssGenerator, IDataScienceErrorHandler, - IDataViewerFactory, IInteractiveBase, IInteractiveWindowInfo, IInteractiveWindowListener, @@ -904,7 +903,7 @@ export abstract class InteractiveBase extends WebViewHost { try { if (await this.checkColumnSize(request.columnSize)) { - const jupyterVariableDataProvider = this.jupyterVariableDataProviderFactory( + const jupyterVariableDataProvider = await this.jupyterVariableDataProviderFactory.create( request.variable, this._notebook! ); diff --git a/src/client/datascience/interactive-ipynb/nativeEditor.ts b/src/client/datascience/interactive-ipynb/nativeEditor.ts index bdf0265e196f..66d56e795717 100644 --- a/src/client/datascience/interactive-ipynb/nativeEditor.ts +++ b/src/client/datascience/interactive-ipynb/nativeEditor.ts @@ -61,7 +61,6 @@ import { ICell, ICodeCssGenerator, IDataScienceErrorHandler, - IDataViewerFactory, IInteractiveWindowInfo, IInteractiveWindowListener, IJupyterDebugger, @@ -87,6 +86,7 @@ import cloneDeep = require('lodash/cloneDeep'); import { concatMultilineStringInput, splitMultilineString } from '../../../datascience-ui/common'; import { ServerStatus } from '../../../datascience-ui/interactive-common/mainState'; import { isTestExecution, UseCustomEditorApi } from '../../common/constants'; +import { IDataViewerFactory } from '../data-viewing/types'; import { getCellHashProvider } from '../editor-integration/cellhashprovider'; import { KernelSwitcher } from '../jupyter/kernels/kernelSwitcher'; diff --git a/src/client/datascience/interactive-ipynb/nativeEditorOldWebView.ts b/src/client/datascience/interactive-ipynb/nativeEditorOldWebView.ts index bc8f6d7560df..7e21a1d11656 100644 --- a/src/client/datascience/interactive-ipynb/nativeEditorOldWebView.ts +++ b/src/client/datascience/interactive-ipynb/nativeEditorOldWebView.ts @@ -30,12 +30,12 @@ import * as localize from '../../common/utils/localize'; import { noop } from '../../common/utils/misc'; import { captureTelemetry } from '../../telemetry'; import { Commands, Identifiers, Telemetry } from '../constants'; +import { IDataViewerFactory } from '../data-viewing/types'; import { InteractiveWindowMessages } from '../interactive-common/interactiveWindowTypes'; import { KernelSwitcher } from '../jupyter/kernels/kernelSwitcher'; import { ICodeCssGenerator, IDataScienceErrorHandler, - IDataViewerFactory, IInteractiveWindowListener, IJupyterDebugger, IJupyterExecution, diff --git a/src/client/datascience/interactive-window/interactiveWindow.ts b/src/client/datascience/interactive-window/interactiveWindow.ts index 0a33612b3faa..6ea3664bd189 100644 --- a/src/client/datascience/interactive-window/interactiveWindow.ts +++ b/src/client/datascience/interactive-window/interactiveWindow.ts @@ -32,6 +32,7 @@ import { EXTENSION_ROOT_DIR } from '../../constants'; import { PythonInterpreter } from '../../interpreter/contracts'; import { captureTelemetry, sendTelemetryEvent } from '../../telemetry'; import { EditorContexts, Identifiers, Telemetry } from '../constants'; +import { IDataViewerFactory } from '../data-viewing/types'; import { InteractiveBase } from '../interactive-common/interactiveBase'; import { INotebookIdentity, @@ -45,7 +46,6 @@ import { ICell, ICodeCssGenerator, IDataScienceErrorHandler, - IDataViewerFactory, IInteractiveWindow, IInteractiveWindowInfo, IInteractiveWindowListener, diff --git a/src/client/datascience/serviceRegistry.ts b/src/client/datascience/serviceRegistry.ts index 3ea4ae867935..93bb45ba8518 100644 --- a/src/client/datascience/serviceRegistry.ts +++ b/src/client/datascience/serviceRegistry.ts @@ -21,6 +21,8 @@ import { DataViewer } from './data-viewing/dataViewer'; import { DataViewerDependencyService } from './data-viewing/dataViewerDependencyService'; import { DataViewerFactory } from './data-viewing/dataViewerFactory'; import { JupyterVariableDataProvider } from './data-viewing/jupyterVariableDataProvider'; +import { JupyterVariableDataProviderFactory } from './data-viewing/jupyterVariableDataProviderFactory'; +import { IDataViewer, IDataViewerFactory } from './data-viewing/types'; import { DataScience } from './datascience'; import { DataScienceSurveyBannerLogger } from './dataScienceSurveyBanner'; import { DebugLocationTrackerFactory } from './debugLocationTrackerFactory'; @@ -115,8 +117,6 @@ import { IDataScienceCodeLensProvider, IDataScienceCommandListener, IDataScienceErrorHandler, - IDataViewer, - IDataViewerFactory, IDebugLocationTracker, IGatherLogger, IGatherProvider, @@ -133,12 +133,10 @@ import { IJupyterServerProvider, IJupyterSessionManagerFactory, IJupyterSubCommandExecutionService, - IJupyterVariable, IJupyterVariableDataProvider, IJupyterVariableDataProviderFactory, IJupyterVariables, IKernelDependencyService, - INotebook, INotebookAndInteractiveWindowUsageTracker, INotebookEditor, INotebookEditorProvider, @@ -261,13 +259,8 @@ export function registerTypes(serviceManager: IServiceManager) { serviceManager.add(IProtocolParser, ProtocolParser); serviceManager.addSingleton(IJupyterDebugService, MultiplexingDebugService, Identifiers.MULTIPLEXING_DEBUGSERVICE); serviceManager.addSingleton(IJupyterDebugService, JupyterDebugService, Identifiers.RUN_BY_LINE_DEBUGSERVICE); - serviceManager.addFactory(IJupyterVariableDataProviderFactory, (context) => { - return (variable: IJupyterVariable, notebook: INotebook) => { - const variableManager: IJupyterVariables = context.container.getNamed(IJupyterVariables, Identifiers.ALL_VARIABLES); - const dependencyService: DataViewerDependencyService = context.container.get(DataViewerDependencyService); - return new JupyterVariableDataProvider(variableManager, dependencyService, variable, notebook); - }; - }); + serviceManager.add(IJupyterVariableDataProvider, JupyterVariableDataProvider); + serviceManager.addSingleton(IJupyterVariableDataProviderFactory, JupyterVariableDataProviderFactory); registerGatherTypes(serviceManager); registerNotebookTypes(serviceManager); diff --git a/src/client/datascience/types.ts b/src/client/datascience/types.ts index 556d80363b7c..fb7ccd1eeff2 100644 --- a/src/client/datascience/types.ts +++ b/src/client/datascience/types.ts @@ -782,12 +782,13 @@ export interface IJupyterVariable { } export const IJupyterVariableDataProvider = Symbol('IJupyterVariableDataProvider'); -export interface IJupyterVariableDataProvider extends IDataViewerDataProvider {} +export interface IJupyterVariableDataProvider extends IDataViewerDataProvider { + setDependencies(variable: IJupyterVariable, notebook: INotebook): void; +} export const IJupyterVariableDataProviderFactory = Symbol('IJupyterVariableDataProviderFactory'); -export interface IJupyterVariableDataProviderFactory extends Function { - // tslint:disable-next-line: callable-types - (variable: IJupyterVariable, notebook: INotebook): IJupyterVariableDataProvider; +export interface IJupyterVariableDataProviderFactory { + create(variable: IJupyterVariable, notebook: INotebook): Promise; } export const IJupyterVariables = Symbol('IJupyterVariables'); @@ -829,16 +830,6 @@ export interface IJupyterVariablesResponse { pageResponse: IJupyterVariable[]; } -export const IDataViewerFactory = Symbol('IDataViewerFactory'); -export interface IDataViewerFactory { - create(dataProvider: IDataViewerDataProvider, title: string): Promise; -} - -export const IDataViewer = Symbol('IDataViewer'); -export interface IDataViewer extends IDisposable { - showData(dataProvider: IDataViewerDataProvider, title: string): Promise; -} - export const IPlotViewerProvider = Symbol('IPlotViewerProvider'); export interface IPlotViewerProvider { showPlot(imageHtml: string): Promise; diff --git a/src/datascience-ui/data-explorer/mainPanel.tsx b/src/datascience-ui/data-explorer/mainPanel.tsx index 25db35975fae..b3f82fb4e70b 100644 --- a/src/datascience-ui/data-explorer/mainPanel.tsx +++ b/src/datascience-ui/data-explorer/mainPanel.tsx @@ -3,7 +3,7 @@ 'use strict'; import './mainPanel.css'; -import { JSONArray, JSONObject } from '@phosphor/coreutils'; +import { JSONArray } from '@phosphor/coreutils'; import * as React from 'react'; import * as uuid from 'uuid/v4'; @@ -15,7 +15,8 @@ import { DataViewerMessages, IDataFrameInfo, IDataViewerMapping, - IGetRowsResponse + IGetRowsResponse, + IRowsResponse } from '../../client/datascience/data-viewing/types'; import { SharedMessages } from '../../client/datascience/messages'; import { IDataScienceExtraSettings } from '../../client/datascience/types'; @@ -140,7 +141,7 @@ export class MainPanel extends React.Component break; case DataViewerMessages.GetAllRowsResponse: - this.handleGetAllRowsResponse(payload as JSONObject); + this.handleGetAllRowsResponse(payload as IRowsResponse); break; case DataViewerMessages.GetRowsResponse: @@ -245,7 +246,7 @@ export class MainPanel extends React.Component this.sendMessage(DataViewerMessages.GetRowsRequest, { start: chunkStart, end: chunkEnd }); } - private handleGetAllRowsResponse(response: JSONObject) { + private handleGetAllRowsResponse(response: IRowsResponse) { const rows = response.data ? (response.data as JSONArray) : []; const normalized = this.normalizeRows(rows); diff --git a/src/datascience-ui/data-explorer/reactSlickGrid.tsx b/src/datascience-ui/data-explorer/reactSlickGrid.tsx index 516d36d2860b..12a67ede2f66 100644 --- a/src/datascience-ui/data-explorer/reactSlickGrid.tsx +++ b/src/datascience-ui/data-explorer/reactSlickGrid.tsx @@ -19,9 +19,6 @@ const slickgridJQ = require('slickgrid/lib/jquery-1.11.2.min'); // Adding comments to ensure order of imports does not change due to auto formatters. // tslint:disable-next-line: ordered-imports -// Adding comments to ensure order of imports does not change due to auto formatters. -// tslint:disable-next-line: ordered-imports -import 'slickgrid/plugins/slick.autotooltips'; import 'slickgrid/slick.core'; // Adding comments to ensure order of imports does not change due to auto formatters. // tslint:disable-next-line: ordered-imports @@ -31,6 +28,9 @@ import 'slickgrid/slick.dataview'; import 'slickgrid/slick.grid'; // Adding comments to ensure order of imports does not change due to auto formatters. // tslint:disable-next-line: ordered-imports +import 'slickgrid/plugins/slick.autotooltips'; +// Adding comments to ensure order of imports does not change due to auto formatters. +// tslint:disable-next-line: ordered-imports import 'slickgrid/slick.grid.css'; // Make sure our css comes after the slick grid css. We override some of its styles. // tslint:disable-next-line: ordered-imports diff --git a/src/test/datascience/data-viewing/dataViewer.unit.test.ts b/src/test/datascience/data-viewing/dataViewer.unit.test.ts index a5c6ae8b723a..fa02b0592c06 100644 --- a/src/test/datascience/data-viewing/dataViewer.unit.test.ts +++ b/src/test/datascience/data-viewing/dataViewer.unit.test.ts @@ -16,9 +16,9 @@ import { IConfigurationService, IExperimentsManager } from '../../../client/comm import { CodeCssGenerator } from '../../../client/datascience/codeCssGenerator'; import { DataViewer } from '../../../client/datascience/data-viewing/dataViewer'; import { JupyterVariableDataProvider } from '../../../client/datascience/data-viewing/jupyterVariableDataProvider'; -import { IDataViewerDataProvider } from '../../../client/datascience/data-viewing/types'; +import { IDataViewer, IDataViewerDataProvider } from '../../../client/datascience/data-viewing/types'; import { ThemeFinder } from '../../../client/datascience/themeFinder'; -import { ICodeCssGenerator, IDataViewer, IThemeFinder } from '../../../client/datascience/types'; +import { ICodeCssGenerator, IThemeFinder } from '../../../client/datascience/types'; suite('Data Science - DataViewer', () => { let dataViewer: IDataViewer; diff --git a/src/test/datascience/dataScienceIocContainer.ts b/src/test/datascience/dataScienceIocContainer.ts index 1fad979375ff..17a764335196 100644 --- a/src/test/datascience/dataScienceIocContainer.ts +++ b/src/test/datascience/dataScienceIocContainer.ts @@ -181,6 +181,8 @@ import { DataViewer } from '../../client/datascience/data-viewing/dataViewer'; import { DataViewerDependencyService } from '../../client/datascience/data-viewing/dataViewerDependencyService'; import { DataViewerFactory } from '../../client/datascience/data-viewing/dataViewerFactory'; import { JupyterVariableDataProvider } from '../../client/datascience/data-viewing/jupyterVariableDataProvider'; +import { JupyterVariableDataProviderFactory } from '../../client/datascience/data-viewing/jupyterVariableDataProviderFactory'; +import { IDataViewer, IDataViewerFactory } from '../../client/datascience/data-viewing/types'; import { DebugLocationTrackerFactory } from '../../client/datascience/debugLocationTrackerFactory'; import { CellHashProvider } from '../../client/datascience/editor-integration/cellhashprovider'; import { CodeLensFactory } from '../../client/datascience/editor-integration/codeLensFactory'; @@ -262,8 +264,6 @@ import { IDataScienceCodeLensProvider, IDataScienceCommandListener, IDataScienceErrorHandler, - IDataViewer, - IDataViewerFactory, IDebugLocationTracker, IGatherLogger, IGatherProvider, @@ -280,12 +280,10 @@ import { IJupyterServerProvider, IJupyterSessionManagerFactory, IJupyterSubCommandExecutionService, - IJupyterVariable, IJupyterVariableDataProvider, IJupyterVariableDataProviderFactory, IJupyterVariables, IKernelDependencyService, - INotebook, INotebookAndInteractiveWindowUsageTracker, INotebookEditor, INotebookEditorProvider, @@ -614,22 +612,14 @@ export class DataScienceIocContainer extends UnitTestIocContainer { this.serviceManager.addSingletonInstance(UseProposedApi, false); this.serviceManager.addSingletonInstance(UseCustomEditorApi, useCustomEditor); this.serviceManager.addSingleton(IDataViewerFactory, DataViewerFactory); - this.serviceManager.addFactory(IJupyterVariableDataProviderFactory, (context) => { - return (variable: IJupyterVariable, notebook: INotebook) => { - const variableManager: IJupyterVariables = context.container.getNamed( - IJupyterVariables, - Identifiers.ALL_VARIABLES - ); - const dependencyService: DataViewerDependencyService = context.container.get( - DataViewerDependencyService - ); - return new JupyterVariableDataProvider(variableManager, dependencyService, variable, notebook); - }; - }); this.serviceManager.add( IJupyterVariableDataProvider, JupyterVariableDataProvider ); + this.serviceManager.addSingleton( + IJupyterVariableDataProviderFactory, + JupyterVariableDataProviderFactory + ); this.serviceManager.addSingleton(IPlotViewerProvider, PlotViewerProvider); this.serviceManager.add(IInteractiveWindow, InteractiveWindow); this.serviceManager.add(IDataViewer, DataViewer); diff --git a/src/test/datascience/dataviewer.functional.test.tsx b/src/test/datascience/dataviewer.functional.test.tsx index 823b93309b8d..d2545f4c4989 100644 --- a/src/test/datascience/dataviewer.functional.test.tsx +++ b/src/test/datascience/dataviewer.functional.test.tsx @@ -13,12 +13,14 @@ import * as uuid from 'uuid/v4'; import { Disposable, Uri } from 'vscode'; import { Identifiers } from '../../client/datascience/constants'; -import { DataViewerMessages, IDataViewerDataProvider } from '../../client/datascience/data-viewing/types'; import { + DataViewerMessages, IDataViewer, - IDataViewerFactory, + IDataViewerDataProvider, + IDataViewerFactory +} from '../../client/datascience/data-viewing/types'; +import { IJupyterVariable, - IJupyterVariableDataProvider, IJupyterVariableDataProviderFactory, INotebook, INotebookProvider @@ -108,8 +110,8 @@ suite('DataScience DataViewer tests', () => { async function createJupyterVariableDataProvider( jupyterVariable: IJupyterVariable - ): Promise { - return jupyterVariableDataProviderFactory(jupyterVariable, notebook!); + ): Promise { + return jupyterVariableDataProviderFactory.create(jupyterVariable, notebook!); } async function createDataViewer(dataProvider: IDataViewerDataProvider, title: string): Promise { @@ -118,7 +120,7 @@ suite('DataScience DataViewer tests', () => { async function createJupyterVariableDataViewer(variable: string, type: string): Promise { const jupyterVariable: IJupyterVariable = createJupyterVariable(variable, type); - const jupyterVariableDataProvider: IJupyterVariableDataProvider = await createJupyterVariableDataProvider( + const jupyterVariableDataProvider: IDataViewerDataProvider = await createJupyterVariableDataProvider( jupyterVariable ); return createDataViewer(jupyterVariableDataProvider, jupyterVariable.name); From fc0dcef495a4112e5a703426f97bfb1ff5956383 Mon Sep 17 00:00:00 2001 From: Sergio Villalobos Date: Fri, 29 May 2020 14:15:12 -0700 Subject: [PATCH 4/5] Issue#12033 Fix hygiene --- src/client/datascience/data-viewing/dataViewerFactory.ts | 2 +- .../datascience/data-viewing/jupyterVariableDataProvider.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client/datascience/data-viewing/dataViewerFactory.ts b/src/client/datascience/data-viewing/dataViewerFactory.ts index 0f08a0da6f1e..ce64481e6c41 100644 --- a/src/client/datascience/data-viewing/dataViewerFactory.ts +++ b/src/client/datascience/data-viewing/dataViewerFactory.ts @@ -7,7 +7,7 @@ import { inject, injectable } from 'inversify'; import { IAsyncDisposable, IAsyncDisposableRegistry } from '../../common/types'; import { IServiceContainer } from '../../ioc/types'; -import { IDataViewer, IDataViewerFactory, IDataViewerDataProvider } from './types'; +import { IDataViewer, IDataViewerDataProvider, IDataViewerFactory } from './types'; @injectable() export class DataViewerFactory implements IDataViewerFactory, IAsyncDisposable { diff --git a/src/client/datascience/data-viewing/jupyterVariableDataProvider.ts b/src/client/datascience/data-viewing/jupyterVariableDataProvider.ts index 2c9d9257af06..b5e5d7ada1c3 100644 --- a/src/client/datascience/data-viewing/jupyterVariableDataProvider.ts +++ b/src/client/datascience/data-viewing/jupyterVariableDataProvider.ts @@ -6,7 +6,7 @@ import '../../common/extensions'; import { inject, injectable, named } from 'inversify'; import { Identifiers } from '../constants'; -import { IJupyterVariable, IJupyterVariables, INotebook, IJupyterVariableDataProvider } from '../types'; +import { IJupyterVariable, IJupyterVariableDataProvider, IJupyterVariables, INotebook } from '../types'; import { DataViewerDependencyService } from './dataViewerDependencyService'; import { ColumnType, IDataFrameInfo, IRowsResponse } from './types'; From d161502b3d08a3fca2e16760a8b3897dc65706f8 Mon Sep 17 00:00:00 2001 From: Sergio Villalobos Date: Fri, 29 May 2020 16:28:46 -0700 Subject: [PATCH 5/5] Issue#12033 Change IRowsResponse to be of type any[] --- .../data-viewing/jupyterVariableDataProvider.ts | 10 ++++++---- src/client/datascience/data-viewing/types.ts | 6 ++---- src/datascience-ui/data-explorer/mainPanel.tsx | 6 +++--- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/client/datascience/data-viewing/jupyterVariableDataProvider.ts b/src/client/datascience/data-viewing/jupyterVariableDataProvider.ts index b5e5d7ada1c3..58a4982edea9 100644 --- a/src/client/datascience/data-viewing/jupyterVariableDataProvider.ts +++ b/src/client/datascience/data-viewing/jupyterVariableDataProvider.ts @@ -78,24 +78,26 @@ export class JupyterVariableDataProvider implements IJupyterVariableDataProvider } public async getAllRows() { - let allRows: IRowsResponse = {}; + let allRows: IRowsResponse = []; await this.ensureInitialized(); if (this.variable && this.variable.rowCount && this.notebook) { - allRows = await this.variableManager.getDataFrameRows( + const dataFrameRows = await this.variableManager.getDataFrameRows( this.variable, this.notebook, 0, this.variable.rowCount ); + allRows = dataFrameRows && dataFrameRows.data ? (dataFrameRows.data as IRowsResponse) : []; } return allRows; } public async getRows(start: number, end: number) { - let rows: IRowsResponse = {}; + let rows: IRowsResponse = []; await this.ensureInitialized(); if (this.variable && this.variable.rowCount && this.notebook) { - rows = await this.variableManager.getDataFrameRows(this.variable, this.notebook, start, end); + const dataFrameRows = await this.variableManager.getDataFrameRows(this.variable, this.notebook, start, end); + rows = dataFrameRows && dataFrameRows.data ? (dataFrameRows.data as IRowsResponse) : []; } return rows; } diff --git a/src/client/datascience/data-viewing/types.ts b/src/client/datascience/data-viewing/types.ts index 4747fcf24af9..df3c97bc5cd5 100644 --- a/src/client/datascience/data-viewing/types.ts +++ b/src/client/datascience/data-viewing/types.ts @@ -69,10 +69,8 @@ export enum ColumnType { Bool = 'bool' } -export interface IRowsResponse { - // tslint:disable-next-line: no-any - data?: any[]; -} +// tslint:disable-next-line: no-any +export type IRowsResponse = any[]; export const IDataViewerFactory = Symbol('IDataViewerFactory'); export interface IDataViewerFactory { diff --git a/src/datascience-ui/data-explorer/mainPanel.tsx b/src/datascience-ui/data-explorer/mainPanel.tsx index b3f82fb4e70b..7df86002ae67 100644 --- a/src/datascience-ui/data-explorer/mainPanel.tsx +++ b/src/datascience-ui/data-explorer/mainPanel.tsx @@ -78,7 +78,7 @@ export class MainPanel extends React.Component }; // Fire off a timer to mimic dynamic loading - setTimeout(() => this.handleGetAllRowsResponse({ data: data.rows }), 1000); + setTimeout(() => this.handleGetAllRowsResponse(data.rows), 1000); } else { this.state = { gridColumns: [], @@ -247,7 +247,7 @@ export class MainPanel extends React.Component } private handleGetAllRowsResponse(response: IRowsResponse) { - const rows = response.data ? (response.data as JSONArray) : []; + const rows = response ? (response as JSONArray) : []; const normalized = this.normalizeRows(rows); // Update our fetched count and actual rows @@ -262,7 +262,7 @@ export class MainPanel extends React.Component private handleGetRowChunkResponse(response: IGetRowsResponse) { // We have a new fetched row count - const rows = response.rows.data ? (response.rows.data as JSONArray) : []; + const rows = response.rows ? (response.rows as JSONArray) : []; const normalized = this.normalizeRows(rows); const newFetched = this.state.fetchedRowCount + (response.end - response.start);