Skip to content

Commit

Permalink
Refactored importing jupyter notebook as python file (microsoft#12575)
Browse files Browse the repository at this point in the history
* refactored import notebook

* added wait for status

* added wait for status

* fixed tests

* added export stuff to serviceContainer

* made exported python document dirty

* fixed tests
  • Loading branch information
techwithtim committed Jun 26, 2020
1 parent f0153fc commit ba587d3
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 31 deletions.
8 changes: 6 additions & 2 deletions src/client/datascience/export/exportManagerFileOpener.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { inject, injectable } from 'inversify';
import { Uri } from 'vscode';
import { Position, Uri } from 'vscode';
import { getLocString } from '../../../datascience-ui/react-common/locReactSide';
import { IApplicationShell, IDocumentManager } from '../../common/application/types';
import { PYTHON_LANGUAGE } from '../../common/constants';
Expand Down Expand Up @@ -67,7 +67,11 @@ export class ExportManagerFileOpener implements IExportManager {
const contents = await this.fileSystem.readFile(uri.fsPath);
await this.fileSystem.deleteFile(uri.fsPath);
const doc = await this.documentManager.openTextDocument({ language: PYTHON_LANGUAGE, content: contents });
await this.documentManager.showTextDocument(doc);
const editor = await this.documentManager.showTextDocument(doc);
// Edit the document so that it is dirty (add a space at the end)
editor.edit((editBuilder) => {
editBuilder.insert(new Position(editor.document.lineCount, 0), '\n');
});
}

private showExportFailed(msg: string) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@ import '../../common/extensions';

import { inject, injectable } from 'inversify';
import * as uuid from 'uuid/v4';
import { Position, Range, TextDocument, Uri, ViewColumn } from 'vscode';
import { Range, TextDocument, Uri } from 'vscode';
import { CancellationToken, CancellationTokenSource } from 'vscode-jsonrpc';

import { IApplicationShell, ICommandManager, IDocumentManager } from '../../common/application/types';
import { CancellationError } from '../../common/cancellation';
import { PYTHON_LANGUAGE } from '../../common/constants';
Expand All @@ -19,6 +18,8 @@ import { captureTelemetry } from '../../telemetry';
import { CommandSource } from '../../testing/common/constants';
import { generateCellRangesFromDocument, generateCellsFromDocument } from '../cellFactory';
import { Commands, Identifiers, Telemetry } from '../constants';
import { ExportFormat, IExportManager } from '../export/types';
import { INotebookStorageProvider } from '../interactive-ipynb/notebookStorageProvider';
import { JupyterInstallError } from '../jupyter/jupyterInstallError';
import {
IDataScienceCommandListener,
Expand All @@ -28,7 +29,6 @@ import {
IJupyterExecution,
INotebookEditorProvider,
INotebookExporter,
INotebookImporter,
INotebookServer,
IStatusProvider
} from '../types';
Expand All @@ -45,9 +45,10 @@ export class InteractiveWindowCommandListener implements IDataScienceCommandList
@inject(IFileSystem) private fileSystem: IFileSystem,
@inject(IConfigurationService) private configuration: IConfigurationService,
@inject(IStatusProvider) private statusProvider: IStatusProvider,
@inject(INotebookImporter) private jupyterImporter: INotebookImporter,
@inject(IDataScienceErrorHandler) private dataScienceErrorHandler: IDataScienceErrorHandler,
@inject(INotebookEditorProvider) protected ipynbProvider: INotebookEditorProvider
@inject(INotebookEditorProvider) protected ipynbProvider: INotebookEditorProvider,
@inject(IExportManager) private exportManager: IExportManager,
@inject(INotebookStorageProvider) private notebookStorageProvider: INotebookStorageProvider
) {}

public register(commandManager: ICommandManager): void {
Expand Down Expand Up @@ -458,8 +459,9 @@ export class InteractiveWindowCommandListener implements IDataScienceCommandList
// Don't call the other overload as we'll end up with double telemetry.
await this.waitForStatus(
async () => {
const contents = await this.jupyterImporter.importFromFile(uris[0].fsPath);
await this.viewDocument(contents);
const contents = await this.fileSystem.readFile(uris[0].fsPath);
const model = await this.notebookStorageProvider.createNew(contents);
await this.exportManager.export(ExportFormat.python, model);
},
localize.DataScience.importingFormat(),
uris[0].fsPath
Expand All @@ -472,25 +474,16 @@ export class InteractiveWindowCommandListener implements IDataScienceCommandList
if (file && file.length > 0) {
await this.waitForStatus(
async () => {
const contents = await this.jupyterImporter.importFromFile(file);
await this.viewDocument(contents);
const contents = await this.fileSystem.readFile(file);
const model = await this.notebookStorageProvider.createNew(contents);
await this.exportManager.export(ExportFormat.python, model);
},
localize.DataScience.importingFormat(),
file
);
}
}

private viewDocument = async (contents: string): Promise<void> => {
const doc = await this.documentManager.openTextDocument({ language: 'python', content: contents });
const editor = await this.documentManager.showTextDocument(doc, ViewColumn.One);

// Edit the document so that it is dirty (add a space at the end)
editor.edit((editBuilder) => {
editBuilder.insert(new Position(editor.document.lineCount, 0), '\n');
});
};

private async scrollToCell(id: string): Promise<void> {
if (id) {
const interactiveWindow = await this.interactiveWindowProvider.getOrCreateActive();
Expand Down
26 changes: 25 additions & 1 deletion src/test/datascience/dataScienceIocContainer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ import { EnvironmentVariablesService } from '../../client/common/variables/envir
import { EnvironmentVariablesProvider } from '../../client/common/variables/environmentVariablesProvider';
import { IEnvironmentVariablesProvider, IEnvironmentVariablesService } from '../../client/common/variables/types';
import { CodeCssGenerator } from '../../client/datascience/codeCssGenerator';
import { ExportCommands } from '../../client/datascience/commands/exportCommands';
import { Identifiers, JUPYTER_OUTPUT_CHANNEL } from '../../client/datascience/constants';
import { ActiveEditorContextService } from '../../client/datascience/context/activeEditorContext';
import { DataViewer } from '../../client/datascience/data-viewing/dataViewer';
Expand All @@ -201,6 +202,18 @@ import { DataScienceCodeLensProvider } from '../../client/datascience/editor-int
import { CodeWatcher } from '../../client/datascience/editor-integration/codewatcher';
import { HoverProvider } from '../../client/datascience/editor-integration/hoverProvider';
import { DataScienceErrorHandler } from '../../client/datascience/errorHandler/errorHandler';
import { ExportBase } from '../../client/datascience/export/exportBase';
import { ExportManager } from '../../client/datascience/export/exportManager';
import { ExportManagerDependencyChecker } from '../../client/datascience/export/exportManagerDependencyChecker';
import { ExportManagerFileOpener } from '../../client/datascience/export/exportManagerFileOpener';
import {
ExportManagerFilePicker,
IExportManagerFilePicker
} from '../../client/datascience/export/exportManagerFilePicker';
import { ExportToHTML } from '../../client/datascience/export/exportToHTML';
import { ExportToPDF } from '../../client/datascience/export/exportToPDF';
import { ExportToPython } from '../../client/datascience/export/exportToPython';
import { ExportFormat, IExport, IExportManager } from '../../client/datascience/export/types';
import { GatherProvider } from '../../client/datascience/gather/gather';
import { GatherListener } from '../../client/datascience/gather/gatherListener';
import { GatherLogger } from '../../client/datascience/gather/gatherLogger';
Expand Down Expand Up @@ -601,7 +614,18 @@ export class DataScienceIocContainer extends UnitTestIocContainer {
instance(this.webPanelProvider)
);
}

this.serviceManager.addSingleton<IExportManager>(ExportManager, ExportManager);
this.serviceManager.addSingleton<IExportManager>(
ExportManagerDependencyChecker,
ExportManagerDependencyChecker
);
this.serviceManager.addSingleton<IExportManager>(IExportManager, ExportManagerFileOpener);
this.serviceManager.addSingleton<IExport>(IExport, ExportToPDF, ExportFormat.pdf);
this.serviceManager.addSingleton<IExport>(IExport, ExportToHTML, ExportFormat.html);
this.serviceManager.addSingleton<IExport>(IExport, ExportToPython, ExportFormat.python);
this.serviceManager.addSingleton<IExport>(IExport, ExportBase, 'Export Base');
this.serviceManager.addSingleton<ExportCommands>(ExportCommands, ExportCommands);
this.serviceManager.addSingleton<IExportManagerFilePicker>(IExportManagerFilePicker, ExportManagerFilePicker);
this.serviceManager.addSingleton<IMountedWebViewFactory>(IMountedWebViewFactory, MountedWebViewFactory);
this.registerFileSystemTypes();
this.serviceManager.rebindInstance<IFileSystem>(IFileSystem, new MockFileSystem());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
'use strict';

import { anything, instance, mock, verify, when } from 'ts-mockito';
import { Uri } from 'vscode';
import { TextEditor, Uri } from 'vscode';
import { IApplicationShell, IDocumentManager } from '../../../client/common/application/types';
import { IFileSystem } from '../../../client/common/platform/types';
import { IBrowserService, IDisposable } from '../../../client/common/types';
Expand All @@ -29,10 +29,13 @@ suite('Data Science - Export File Opener', () => {
applicationShell = mock<IApplicationShell>();
browserService = mock<IBrowserService>();
const reporter = mock(ProgressReporter);
const editor = mock<TextEditor>();
// tslint:disable-next-line: no-any
(instance(editor) as any).then = undefined;
// tslint:disable-next-line: no-any
when(reporter.createProgressIndicator(anything())).thenReturn(instance(mock<IDisposable>()) as any);
when(documentManager.openTextDocument(anything())).thenResolve();
when(documentManager.showTextDocument(anything())).thenResolve();
when(documentManager.showTextDocument(anything())).thenReturn(Promise.resolve(instance(editor)));
when(fileSystem.readFile(anything())).thenResolve();
fileOpener = new ExportManagerFileOpener(
instance(exporter),
Expand All @@ -45,15 +48,13 @@ suite('Data Science - Export File Opener', () => {

test('No file is opened if nothing is exported', async () => {
when(exporter.export(anything(), anything())).thenResolve();

await fileOpener.export(ExportFormat.python, model);

verify(documentManager.showTextDocument(anything())).never();
});
test('Python File is opened if exported', async () => {
const uri = Uri.file('test.python');
when(exporter.export(anything(), anything())).thenResolve(uri);

await fileOpener.export(ExportFormat.python, model);

verify(documentManager.showTextDocument(anything())).once();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import { Matcher } from 'ts-mockito/lib/matcher/type/Matcher';
import * as TypeMoq from 'typemoq';
import * as uuid from 'uuid/v4';
import { EventEmitter, Uri } from 'vscode';

import { ApplicationShell } from '../../client/common/application/applicationShell';
import { IApplicationShell } from '../../client/common/application/types';
import { PythonSettings } from '../../client/common/configSettings';
Expand All @@ -20,7 +19,9 @@ import * as localize from '../../client/common/utils/localize';
import { generateCells } from '../../client/datascience/cellFactory';
import { Commands } from '../../client/datascience/constants';
import { DataScienceErrorHandler } from '../../client/datascience/errorHandler/errorHandler';
import { ExportFormat, IExportManager } from '../../client/datascience/export/types';
import { NativeEditorProvider } from '../../client/datascience/interactive-ipynb/nativeEditorProvider';
import { NotebookStorageProvider } from '../../client/datascience/interactive-ipynb/notebookStorageProvider';
import { InteractiveWindowCommandListener } from '../../client/datascience/interactive-window/interactiveWindowCommandListener';
import { InteractiveWindowProvider } from '../../client/datascience/interactive-window/interactiveWindowProvider';
import { JupyterExecutionFactory } from '../../client/datascience/jupyter/jupyterExecutionFactory';
Expand Down Expand Up @@ -72,6 +73,8 @@ suite('Interactive window command listener', async () => {
const documentManager = new MockDocumentManager();
const statusProvider = new MockStatusProvider();
const commandManager = new MockCommandManager();
const exportManager = mock<IExportManager>();
const notebookStorageProvider = mock(NotebookStorageProvider);
let notebookEditorProvider: INotebookEditorProvider;
const server = createTypeMoq<INotebookServer>('jupyter server');
let lastFileContents: any;
Expand Down Expand Up @@ -211,9 +214,10 @@ suite('Interactive window command listener', async () => {
instance(fileSystem),
instance(configService),
statusProvider,
instance(notebookImporter),
instance(dataScienceErrorHandler),
instance(notebookEditorProvider)
instance(notebookEditorProvider),
instance(exportManager),
instance(notebookStorageProvider)
);
result.register(commandManager);

Expand All @@ -226,12 +230,12 @@ suite('Interactive window command listener', async () => {
Promise.resolve([Uri.file('foo')])
);
await commandManager.executeCommand(Commands.ImportNotebook, undefined, undefined);
assert.ok(documentManager.activeTextEditor, 'Imported file was not opened');
verify(exportManager.export(ExportFormat.python, anything())).once();
});
test('Import File', async () => {
createCommandListener();
await commandManager.executeCommand(Commands.ImportNotebook, Uri.file('bar.ipynb'), undefined);
assert.ok(documentManager.activeTextEditor, 'Imported file was not opened');
verify(exportManager.export(ExportFormat.python, anything())).twice();
});
test('Export File', async () => {
createCommandListener();
Expand Down

0 comments on commit ba587d3

Please sign in to comment.