Port NB Convert Fix to point release branch (#14200)

Author: IanMatthewHuff

CHANGELOG.md

@@ -1,5 +1,67 @@
 # Changelog
 
+## 2020.9.2 (1 October 2020)
+
+### Fixes
+
+1. Support nbconvert version 6+ for exporting notebooks to python code.
+   ([#14169](https://github.com/Microsoft/vscode-python/issues/14169))
+
+### Thanks
+
+Thanks to the following projects which we fully rely on to provide some of
+our features:
+
+-   [debugpy](https://pypi.org/project/debugpy/)
+-   [isort](https://pypi.org/project/isort/)
+-   [jedi](https://pypi.org/project/jedi/)
+    and [parso](https://pypi.org/project/parso/)
+-   [Microsoft Python Language Server](https://github.com/microsoft/python-language-server)
+-   [Pylance](https://github.com/microsoft/pylance-release)
+-   [exuberant ctags](http://ctags.sourceforge.net/) (user-installed)
+-   [rope](https://pypi.org/project/rope/) (user-installed)
+
+Also thanks to the various projects we provide integrations with which help
+make this extension useful:
+
+-   Debugging support:
+    [Django](https://pypi.org/project/Django/),
+    [Flask](https://pypi.org/project/Flask/),
+    [gevent](https://pypi.org/project/gevent/),
+    [Jinja](https://pypi.org/project/Jinja/),
+    [Pyramid](https://pypi.org/project/pyramid/),
+    [PySpark](https://pypi.org/project/pyspark/),
+    [Scrapy](https://pypi.org/project/Scrapy/),
+    [Watson](https://pypi.org/project/Watson/)
+-   Formatting:
+    [autopep8](https://pypi.org/project/autopep8/),
+    [black](https://pypi.org/project/black/),
+    [yapf](https://pypi.org/project/yapf/)
+-   Interpreter support:
+    [conda](https://conda.io/),
+    [direnv](https://direnv.net/),
+    [pipenv](https://pypi.org/project/pipenv/),
+    [pyenv](https://github.com/pyenv/pyenv),
+    [venv](https://docs.python.org/3/library/venv.html#module-venv),
+    [virtualenv](https://pypi.org/project/virtualenv/)
+-   Linting:
+    [bandit](https://pypi.org/project/bandit/),
+    [flake8](https://pypi.org/project/flake8/),
+    [mypy](https://pypi.org/project/mypy/),
+    [prospector](https://pypi.org/project/prospector/),
+    [pylint](https://pypi.org/project/pylint/),
+    [pydocstyle](https://pypi.org/project/pydocstyle/),
+    [pylama](https://pypi.org/project/pylama/)
+-   Testing:
+    [nose](https://pypi.org/project/nose/),
+    [pytest](https://pypi.org/project/pytest/),
+    [unittest](https://docs.python.org/3/library/unittest.html#module-unittest)
+
+And finally thanks to the [Python](https://www.python.org/) development team and
+community for creating a fantastic programming language and community to be a
+part of!
+
+
 ## 2020.9.1 (29 September 2020)
 
 ### Fixes

package.nls.json

@@ -31,7 +31,7 @@
     "DataScience.openExportFileYes": "Yes",
     "DataScience.openExportFileNo": "No",
     "DataScience.failedExportMessage": "Export failed.",
-    "DataScience.exportFailedGeneralMessage": "Export failed. Please check the 'Python' [output](command:python.viewOutput) panel for further details.",
+    "DataScience.exportFailedGeneralMessage": "Please check the 'Python' [output](command:python.viewOutput) panel for further details.",
     "DataScience.exportToPDFDependencyMessage": "If you have not installed xelatex (TeX) you will need to do so before you can export to PDF, for further instructions go to https://nbconvert.readthedocs.io/en/latest/install.html#installing-tex. \r\nTo avoid installing xelatex (TeX) you might want to try exporting to HTML and using your browsers \"Print to PDF\" feature.",
     "DataScience.launchNotebookTrustPrompt": "A notebook could execute harmful code when opened. Some outputs have been hidden. Do you trust this notebook? [Learn more.](https://aka.ms/trusted-notebooks)",
     "DataScience.launchNotebookTrustPrompt.yes": "Trust",

src/client/common/utils/localize.ts

@@ -892,7 +892,7 @@ export namespace DataScience {
     export const openExportFileNo = localize('DataScience.openExportFileNo', 'No');
     export const exportFailedGeneralMessage = localize(
         'DataScience.exportFailedGeneralMessage',
-        `Export failed. Please check the 'Python' [output](command:python.viewOutput) panel for further details.`
+        `Please check the 'Python' [output](command:python.viewOutput) panel for further details.`
     );
     export const exportToPDFDependencyMessage = localize(
         'DataScience.exportToPDFDependencyMessage',

src/client/datascience/common.ts

@@ -3,6 +3,7 @@
 'use strict';
 import type { nbformat } from '@jupyterlab/coreutils';
 import * as os from 'os';
+import { parse, SemVer } from 'semver';
 import { Memento, Uri } from 'vscode';
 import { splitMultilineString } from '../../datascience-ui/common';
 import { traceError, traceInfo } from '../common/logger';
@@ -188,3 +189,14 @@ export async function getRealPath(
         }
     }
 }
+
+// For the given string parse it out to a SemVer or return undefined
+export function parseSemVer(versionString: string): SemVer | undefined {
+    const versionMatch = /^\s*(\d+)\.(\d+)\.(.+)\s*$/.exec(versionString);
+    if (versionMatch && versionMatch.length > 2) {
+        const major = parseInt(versionMatch[1], 10);
+        const minor = parseInt(versionMatch[2], 10);
+        const build = parseInt(versionMatch[3], 10);
+        return parse(`${major}.${minor}.${build}`, true) ?? undefined;
+    }
+}

src/client/datascience/constants.ts

@@ -569,7 +569,7 @@ export namespace LiveShare {
 
 export namespace LiveShareCommands {
     export const isNotebookSupported = 'isNotebookSupported';
-    export const isImportSupported = 'isImportSupported';
+    export const getImportPackageVersion = 'getImportPackageVersion';
     export const connectToNotebookServer = 'connectToNotebookServer';
     export const getUsableJupyterPython = 'getUsableJupyterPython';
     export const executeObservable = 'executeObservable';

src/client/datascience/data-viewing/dataViewerDependencyService.ts

@@ -4,7 +4,7 @@
 'use strict';
 
 import { inject, injectable } from 'inversify';
-import { parse, SemVer } from 'semver';
+import { SemVer } from 'semver';
 import { CancellationToken } from 'vscode';
 import { IApplicationShell } from '../../common/application/types';
 import { Cancellation, createPromiseFromCancellation, wrapCancellationTokens } from '../../common/cancellation';
@@ -14,6 +14,7 @@ import { IInstaller, InstallerResponse, Product } from '../../common/types';
 import { Common, DataScience } from '../../common/utils/localize';
 import { PythonEnvironment } from '../../pythonEnvironments/info';
 import { sendTelemetryEvent } from '../../telemetry';
+import { parseSemVer } from '../common';
 import { Telemetry } from '../constants';
 
 const minimumSupportedPandaVersion = '0.20.0';
@@ -104,13 +105,8 @@ export class DataViewerDependencyService {
                 throwOnStdErr: true,
                 token
             });
-            const versionMatch = /^\s*(\d+)\.(\d+)\.(.+)\s*$/.exec(result.stdout);
-            if (versionMatch && versionMatch.length > 2) {
-                const major = parseInt(versionMatch[1], 10);
-                const minor = parseInt(versionMatch[2], 10);
-                const build = parseInt(versionMatch[3], 10);
-                return parse(`${major}.${minor}.${build}`, true) ?? undefined;
-            }
+
+            return parseSemVer(result.stdout);
         } catch (ex) {
             traceWarning('Failed to get version of Pandas to use Data Viewer', ex);
             return;

src/client/datascience/export/exportDependencyChecker.ts

@@ -17,9 +17,9 @@ export class ExportDependencyChecker {
         // Before we try the import, see if we don't support it, if we don't give a chance to install dependencies
         const reporter = this.progressReporter.createProgressIndicator(`Exporting to ${format}`);
         try {
-            if (!(await this.jupyterExecution.isImportSupported())) {
+            if (!(await this.jupyterExecution.getImportPackageVersion())) {
                 await this.dependencyManager.installMissingDependencies();
-                if (!(await this.jupyterExecution.isImportSupported())) {
+                if (!(await this.jupyterExecution.getImportPackageVersion())) {
                     throw new Error(localize.DataScience.jupyterNbConvertNotSupported());
                 }
             }

src/client/datascience/jupyter/interpreter/jupyterInterpreterDependencyService.ts

@@ -4,6 +4,7 @@
 'use strict';
 
 import { inject, injectable } from 'inversify';
+import { SemVer } from 'semver';
 import { CancellationToken } from 'vscode';
 import { IApplicationShell } from '../../../common/application/types';
 import { Cancellation, createPromiseFromCancellation, wrapCancellationTokens } from '../../../common/cancellation';
@@ -14,6 +15,7 @@ import { Common, DataScience } from '../../../common/utils/localize';
 import { noop } from '../../../common/utils/misc';
 import { PythonEnvironment } from '../../../pythonEnvironments/info';
 import { sendTelemetryEvent } from '../../../telemetry';
+import { parseSemVer } from '../../common';
 import { HelpLinks, JupyterCommands, Telemetry } from '../../constants';
 import { reportAction } from '../../progress/decorator';
 import { ReportableAction } from '../../progress/types';
@@ -241,6 +243,23 @@ export class JupyterInterpreterDependencyService {
         return installed;
     }
 
+    public async getNbConvertVersion(
+        interpreter: PythonEnvironment,
+        _token?: CancellationToken
+    ): Promise<SemVer | undefined> {
+        const command = this.commandFactory.createInterpreterCommand(
+            JupyterCommands.ConvertCommand,
+            'jupyter',
+            ['-m', 'jupyter', 'nbconvert'],
+            interpreter,
+            false
+        );
+
+        const result = await command.exec(['--version'], { throwOnStdErr: true });
+
+        return parseSemVer(result.stdout);
+    }
+
     /**
      * Gets a list of the dependencies not installed, dependencies that are required to launch the jupyter notebook server.
      *

src/client/datascience/jupyter/interpreter/jupyterInterpreterSubCommandExecutionService.ts

@@ -5,6 +5,7 @@
 
 import { inject, injectable, named } from 'inversify';
 import * as path from 'path';
+import { SemVer } from 'semver';
 import { CancellationToken, Uri } from 'vscode';
 import { Cancellation } from '../../../common/cancellation';
 import { traceError, traceInfo, traceWarning } from '../../../common/logger';
@@ -76,12 +77,16 @@ export class JupyterInterpreterSubCommandExecutionService
         }
         return this.jupyterDependencyService.areDependenciesInstalled(interpreter, token);
     }
-    public async isExportSupported(token?: CancellationToken): Promise<boolean> {
+    public async getExportPackageVersion(token?: CancellationToken): Promise<SemVer | undefined> {
         const interpreter = await this.jupyterInterpreter.getSelectedInterpreter(token);
         if (!interpreter) {
-            return false;
+            return;
+        }
+
+        // If nbconvert is there check and return the version
+        if (await this.jupyterDependencyService.isExportSupported(interpreter, token)) {
+            return this.jupyterDependencyService.getNbConvertVersion(interpreter, token);
         }
-        return this.jupyterDependencyService.isExportSupported(interpreter, token);
     }
     public async getReasonForJupyterNotebookNotBeingSupported(token?: CancellationToken): Promise<string> {
         let interpreter = await this.jupyterInterpreter.getSelectedInterpreter(token);
@@ -176,11 +181,21 @@ export class JupyterInterpreterSubCommandExecutionService
         const args = template
             ? [file.fsPath, '--to', 'python', '--stdout', '--template', template]
             : [file.fsPath, '--to', 'python', '--stdout'];
+
         // Ignore stderr, as nbconvert writes conversion result to stderr.
         // stdout contains the generated python code.
         return daemon
             .execModule('jupyter', ['nbconvert'].concat(args), { throwOnStdErr: false, encoding: 'utf8', token })
-            .then((output) => output.stdout);
+            .then((output) => {
+                // We can't check stderr (as nbconvert puts diag output there) but we need to verify here that we actually
+                // converted something. If it's zero size then just raise an error
+                if (output.stdout === '') {
+                    traceError('nbconvert zero size output');
+                    throw new Error(output.stderr);
+                } else {
+                    return output.stdout;
+                }
+            });
     }
     public async openNotebook(notebookFile: string): Promise<void> {
         const interpreter = await this.getSelectedInterpreterAndThrowIfNotAvailable();

src/client/datascience/jupyter/jupyterExecution.ts

@@ -2,6 +2,7 @@
 // Licensed under the MIT License.
 'use strict';
 import * as path from 'path';
+import { SemVer } from 'semver';
 import * as uuid from 'uuid/v4';
 import { CancellationToken, CancellationTokenSource, Event, EventEmitter, Uri } from 'vscode';
 
@@ -123,9 +124,9 @@ export class JupyterExecutionBase implements IJupyterExecution {
     }
 
     @reportAction(ReportableAction.CheckingIfImportIsSupported)
-    public async isImportSupported(cancelToken?: CancellationToken): Promise<boolean> {
+    public async getImportPackageVersion(cancelToken?: CancellationToken): Promise<SemVer | undefined> {
         // See if we can find the command nbconvert
-        return this.jupyterInterpreterService.isExportSupported(cancelToken);
+        return this.jupyterInterpreterService.getExportPackageVersion(cancelToken);
     }
 
     public isSpawnSupported(cancelToken?: CancellationToken): Promise<boolean> {

src/client/datascience/jupyter/jupyterExecutionFactory.ts

@@ -2,6 +2,7 @@
 // Licensed under the MIT License.
 'use strict';
 import { inject, injectable, named } from 'inversify';
+import { SemVer } from 'semver';
 import { CancellationToken, Event, EventEmitter, Uri } from 'vscode';
 
 import { IApplicationShell, ILiveShareApi, IWorkspaceService } from '../../common/application/types';
@@ -117,9 +118,9 @@ export class JupyterExecutionFactory implements IJupyterExecution, IAsyncDisposa
         return execution.getNotebookError();
     }
 
-    public async isImportSupported(cancelToken?: CancellationToken): Promise<boolean> {
+    public async getImportPackageVersion(cancelToken?: CancellationToken): Promise<SemVer | undefined> {
         const execution = await this.executionFactory.get();
-        return execution.isImportSupported(cancelToken);
+        return execution.getImportPackageVersion(cancelToken);
     }
     public async isSpawnSupported(cancelToken?: CancellationToken): Promise<boolean> {
         const execution = await this.executionFactory.get();

src/client/datascience/jupyter/jupyterImporter.ts

@@ -26,20 +26,22 @@ import {
 export class JupyterImporter implements INotebookImporter {
     public isDisposed: boolean = false;
     // Template that changes markdown cells to have # %% [markdown] in the comments
-    private readonly nbconvertTemplateFormat =
+    private readonly nbconvertBaseTemplateFormat =
         // tslint:disable-next-line:no-multiline-string
-        `{%- extends 'null.tpl' -%}
+        `{%- extends '{0}' -%}
 {% block codecell %}
-{0}
+{1}
 {{ super() }}
 {% endblock codecell %}
 {% block in_prompt %}{% endblock in_prompt %}
 {% block input %}{{ cell.source | ipython2python }}{% endblock input %}
 {% block markdowncell scoped %}{0} [markdown]
 {{ cell.source | comment_lines }}
 {% endblock markdowncell %}`;
-
-    private templatePromise: Promise<string | undefined>;
+    private readonly nbconvert5Null = 'null.tpl';
+    private readonly nbconvert6Null = 'base/null.j2';
+    private template5Promise?: Promise<string | undefined>;
+    private template6Promise?: Promise<string | undefined>;
 
     constructor(
         @inject(IDataScienceFileSystem) private fs: IDataScienceFileSystem,
@@ -50,13 +52,9 @@ export class JupyterImporter implements INotebookImporter {
         @inject(IPlatformService) private readonly platform: IPlatformService,
         @inject(IJupyterInterpreterDependencyManager)
         private readonly dependencyManager: IJupyterInterpreterDependencyManager
-    ) {
-        this.templatePromise = this.createTemplateFile();
-    }
+    ) {}
 
     public async importFromFile(sourceFile: Uri): Promise<string> {
-        const template = await this.templatePromise;
-
         // If the user has requested it, add a cd command to the imported file so that relative paths still work
         const settings = this.configuration.getSettings();
         let directoryChange: string | undefined;
@@ -65,12 +63,30 @@ export class JupyterImporter implements INotebookImporter {
         }
 
         // Before we try the import, see if we don't support it, if we don't give a chance to install dependencies
-        if (!(await this.jupyterExecution.isImportSupported())) {
+        if (!(await this.jupyterExecution.getImportPackageVersion())) {
             await this.dependencyManager.installMissingDependencies();
         }
 
+        const nbConvertVersion = await this.jupyterExecution.getImportPackageVersion();
         // Use the jupyter nbconvert functionality to turn the notebook into a python file
-        if (await this.jupyterExecution.isImportSupported()) {
+        if (nbConvertVersion) {
+            // nbconvert 5 and 6 use a different base template file
+            // Create and select the correct one
+            let template: string | undefined;
+            if (nbConvertVersion.major >= 6) {
+                if (!this.template6Promise) {
+                    this.template6Promise = this.createTemplateFile(true);
+                }
+
+                template = await this.template6Promise;
+            } else {
+                if (!this.template5Promise) {
+                    this.template5Promise = this.createTemplateFile(false);
+                }
+
+                template = await this.template5Promise;
+            }
+
             let fileOutput: string = await this.jupyterExecution.importNotebook(sourceFile, template);
             if (fileOutput.includes('get_ipython()')) {
                 fileOutput = this.addIPythonImport(fileOutput);
@@ -153,7 +169,7 @@ export class JupyterImporter implements INotebookImporter {
         }
     }
 
-    private async createTemplateFile(): Promise<string | undefined> {
+    private async createTemplateFile(nbconvert6: boolean): Promise<string | undefined> {
         // Create a temp file on disk
         const file = await this.fs.createTemporaryLocalFile('.tpl');
 
@@ -164,7 +180,10 @@ export class JupyterImporter implements INotebookImporter {
                 this.disposableRegistry.push(file);
                 await this.fs.appendLocalFile(
                     file.filePath,
-                    this.nbconvertTemplateFormat.format(this.defaultCellMarker)
+                    this.nbconvertBaseTemplateFormat.format(
+                        nbconvert6 ? this.nbconvert6Null : this.nbconvert5Null,
+                        this.defaultCellMarker
+                    )
                 );
 
                 // Now we should have a template that will convert