-
Notifications
You must be signed in to change notification settings - Fork 1.2k
NB Convert 6.0 support for export #14177
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
6be7c8d
ba1c13b
89729eb
2c13198
7d28992
f096cf5
f410e2c
c377034
9490d44
08d52e4
83ff6e9
d7e98d3
18a6a27
1444346
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Support nbconvert version 6+ for exporting notebooks to python code. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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(); | ||
} | ||
) {} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I moved the construction of the template files to the point where they were needed instead of construction. Didn't see the point of slowing down activation for a feature that's probably not a common every day use scenario. Plus triggering an export is already a user action with an expected wait time, so seems fine to build the file then. |
||
|
||
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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could be a bit simpler if we didn't worry about users changing the jupyter interpreter while running. But I figured that might be a possibility. So I tested that this does support moving your Jupyter interpreter to one with nbconvert 5 to one that uses nbconvert 6 and back without restarting. |
||
// 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); | ||
IanMatthewHuff marked this conversation as resolved.
Show resolved
Hide resolved
|
||
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 | ||
|
Uh oh!
There was an error while loading. Please reload this page.