Skip to content
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

Change the default cell marker #7782

Merged
merged 5 commits into from
Oct 7, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions news/2 Fixes/7674.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Change the default cell marker to '# %%' instead of '#%%' to prevent linter errors in python files with markers.
Also added a new setting to change this - 'python.dataScience.defaultCellMarker'.
6 changes: 6 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -1562,6 +1562,12 @@
"description": "Regular expression used to identify code cells. All code until the next match is considered part of this cell. \nDefaults to '^(#\\s*%%|#\\s*\\<codecell\\>|#\\s*In\\[\\d*?\\]|#\\s*In\\[ \\])' if left blank",
"scope": "resource"
},
"python.dataScience.defaultCellMarker": {
"type": "string",
"default": "# %%",
"description": "Cell marker used for delineating a cell in a python file.",
"scope": "resource"
},
"python.dataScience.markdownRegularExpression": {
"type": "string",
"default": "^(#\\s*%%\\s*\\[markdown\\]|#\\s*\\<markdowncell\\>)",
Expand Down
4 changes: 2 additions & 2 deletions package.nls.json
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@
"DataScience.exportingFormat": "Exporting {0}",
"DataScience.exportCancel": "Cancel",
"Common.canceled": "Canceled",
"DataScience.importChangeDirectoryComment": "#%% Change working directory from the workspace root to the ipynb file location. Turn this addition off with the DataScience.changeDirOnImportExport setting",
"DataScience.importChangeDirectoryComment": "{0} Change working directory from the workspace root to the ipynb file location. Turn this addition off with the DataScience.changeDirOnImportExport setting",
"DataScience.exportChangeDirectoryComment": "# Change directory to VSCode workspace root so that relative path loads work correctly. Turn this addition off with the DataScience.changeDirOnImportExport setting",
"DataScience.interruptKernelStatus": "Interrupting IPython Kernel",
"DataScience.restartKernelAfterInterruptMessage": "Interrupting the kernel timed out. Do you want to restart the kernel instead? All variables will be lost.",
Expand Down Expand Up @@ -341,7 +341,7 @@
"DataScience.jupyterDebuggerInstallPtvsdYes": "Yes",
"DataScience.jupyterDebuggerInstallPtvsdNo": "No",
"DataScience.cellStopOnErrorFormatMessage": "{0} cells were canceled due to an error in the previous cell.",
"DataScience.instructionComments": "# To add a new cell, type '#%%'\n# To add a new markdown cell, type '#%% [markdown]'\n",
"DataScience.instructionComments": "# To add a new cell, type '{0}'\n# To add a new markdown cell, type '{0} [markdown]'\n",
"DataScience.scrollToCellTitleFormatMessage": "Go to [{0}]",
"DataScience.remoteDebuggerNotSupported": "Debugging while attached to a remote server is not currently supported.",
"DataScience.save": "Save notebook",
Expand Down
2 changes: 1 addition & 1 deletion package.nls.nl.json
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@
"DataScience.exportingFormat": "Aan het exporteren {0}",
"DataScience.exportCancel": "Annuleren",
"Common.canceled": "Geannuleerd",
"DataScience.importChangeDirectoryComment": "#%% De werkmap van de werkruimte root naar de ipynb-bestandslocatie veranderen. Schakel deze toevoeging uit met de instelling DataScience.changeDirOnImportExport",
"DataScience.importChangeDirectoryComment": "{0} De werkmap van de werkruimte root naar de ipynb-bestandslocatie veranderen. Schakel deze toevoeging uit met de instelling DataScience.changeDirOnImportExport",
"DataScience.exportChangeDirectoryComment": "# De map wijzigen naar de VSCode-werktuimte root zodat de relatieve pad-ladingen correct werken. Schakel deze toevoeging uit met de instelling DataScience.changeDirOnImportExport",
"DataScience.interruptKernelStatus": "IPython-kernel onderbreken",
"DataScience.restartKernelAfterInterruptMessage": "Het onderbreken van de kernel duurde te lang. Wil je de kernel in plaats daarvan herstarten? Alle variabelen zullen verloren gaan.",
Expand Down
4 changes: 2 additions & 2 deletions snippets/python.json
Original file line number Diff line number Diff line change
Expand Up @@ -242,12 +242,12 @@
},
"add/new/cell": {
"prefix": "add/new/cell",
"body": "#%%",
"body": "# %%",
"description": "Code snippet to add a new cell"
},
"mark/markdown": {
"prefix": "mark/markdown",
"body": "#%% [markdown]",
"body": "# %% [markdown]",
"description": "Code snippet to add a new markdown cell"
}
}
1 change: 1 addition & 0 deletions src/client/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,7 @@ export interface IDataScienceSettings {
runMagicCommands?: string;
runStartupCommands: string;
debugJustMyCode: boolean;
defaultCellMarker?: string;
}

export const IConfigurationService = Symbol('IConfigurationService');
Expand Down
4 changes: 2 additions & 2 deletions src/client/common/utils/localize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ export namespace DataScience {
export const runAllCellsLensCommandTitle = localize('python.command.python.datascience.runallcells.title', 'Run all cells');
export const runAllCellsAboveLensCommandTitle = localize('python.command.python.datascience.runallcellsabove.title', 'Run above');
export const runCellAndAllBelowLensCommandTitle = localize('python.command.python.datascience.runcellandallbelow.title', 'Run Below');
export const importChangeDirectoryComment = localize('DataScience.importChangeDirectoryComment', '#%% Change working directory from the workspace root to the ipynb file location. Turn this addition off with the DataScience.changeDirOnImportExport setting');
export const importChangeDirectoryComment = localize('DataScience.importChangeDirectoryComment', '{0} Change working directory from the workspace root to the ipynb file location. Turn this addition off with the DataScience.changeDirOnImportExport setting');
export const exportChangeDirectoryComment = localize('DataScience.exportChangeDirectoryComment', '# Change directory to VSCode workspace root so that relative path loads work correctly. Turn this addition off with the DataScience.changeDirOnImportExport setting');

export const restartKernelMessage = localize('DataScience.restartKernelMessage', 'Do you want to restart the Jupter kernel? All variables will be lost.');
Expand Down Expand Up @@ -263,7 +263,7 @@ export namespace DataScience {
export const jupyterDebuggerInstallPtvsdNo = localize('DataScience.jupyterDebuggerInstallPtvsdNo', 'No');
export const cellStopOnErrorFormatMessage = localize('DataScience.cellStopOnErrorFormatMessage', '{0} cells were canceled due to an error in the previous cell.');
export const scrollToCellTitleFormatMessage = localize('DataScience.scrollToCellTitleFormatMessage', 'Go to [{0}]');
export const instructionComments = localize('DataScience.instructionComments', '# To add a new cell, type "#%%"\n# To add a new markdown cell, type "#%% [markdown]"\n');
export const instructionComments = localize('DataScience.instructionComments', '# To add a new cell, type "{0}"\n# To add a new markdown cell, type "{0} [markdown]"\n');
export const invalidNotebookFileError = localize('DataScience.invalidNotebookFileError', 'Notebook is not in the correct format. Check the file for correct json.');
export const invalidNotebookFileErrorFormat = localize('DataScience.invalidNotebookFileError', '{0} is not a valid notebook file. Check the file for correct json.');
export const nativeEditorTitle = localize('DataScience.nativeEditorTitle', 'Notebook Editor');
Expand Down
3 changes: 2 additions & 1 deletion src/client/datascience/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -277,12 +277,13 @@ export namespace Identifiers {
export const SvgSizeTag = 'sizeTag={{0}, {1}}';
export const InteractiveWindowIdentity = 'history://EC155B3B-DC18-49DC-9E99-9A948AA2F27B';
export const InteractiveWindowIdentityScheme = 'history';
export const DefaultCodeCellMarker = '# %%';
}

export namespace CodeSnippits {
export const ChangeDirectory = ['{0}', '{1}', 'import os', 'try:', '\tos.chdir(os.path.join(os.getcwd(), \'{2}\'))', '\tprint(os.getcwd())', 'except:', '\tpass', ''];
export const ChangeDirectoryCommentIdentifier = '# ms-python.python added'; // Not translated so can compare.
export const ImportIPython = '#%%\nfrom IPython import get_ipython\n\n';
export const ImportIPython = '{0}\nfrom IPython import get_ipython\n\n{1}';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is there a second parameter here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When this is used it's concatenated with another string.


In reply to: 331724874 [](ancestors = 331724874)

export const MatplotLibInitSvg = `import matplotlib\n%matplotlib inline\n${Identifiers.MatplotLibDefaultParams} = dict(matplotlib.rcParams)\n%config InlineBackend.figure_formats = 'svg', 'png'`;
export const MatplotLibInitPng = `import matplotlib\n%matplotlib inline\n${Identifiers.MatplotLibDefaultParams} = dict(matplotlib.rcParams)\n%config InlineBackend.figure_formats = 'png'`;
}
Expand Down
16 changes: 11 additions & 5 deletions src/client/datascience/editor-integration/codewatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { StopWatch } from '../../common/utils/stopWatch';
import { captureTelemetry, sendTelemetryEvent } from '../../telemetry';
import { ICodeExecutionHelper } from '../../terminals/types';
import { CellMatcher } from '../cellMatcher';
import { Commands, Telemetry } from '../constants';
import { Commands, Identifiers, Telemetry } from '../constants';
import { ICodeLensFactory, ICodeWatcher, IDataScienceErrorHandler, IInteractiveWindowProvider } from '../types';

@injectable()
Expand Down Expand Up @@ -268,9 +268,10 @@ export class CodeWatcher implements ICodeWatcher {

public async addEmptyCellToBottom(): Promise<void> {
const editor = this.documentManager.activeTextEditor;
const cellDelineator = this.defaultCellMarker;
if (editor) {
editor.edit((editBuilder) => {
editBuilder.insert(new Position(editor.document.lineCount, 0), '\n\n#%%\n');
editBuilder.insert(new Position(editor.document.lineCount, 0), `\n\n${cellDelineator}\n`);
});

const newPosition = new Position(editor.document.lineCount + 3, 0); // +3 to account for the added spaces and to position after the new mark
Expand All @@ -286,6 +287,7 @@ export class CodeWatcher implements ICodeWatcher {
const editor = this.documentManager.activeTextEditor;
const cellMatcher = new CellMatcher();
let index = 0;
const cellDelineator = this.defaultCellMarker;

if (editor) {
editor.edit((editBuilder) => {
Expand All @@ -295,14 +297,14 @@ export class CodeWatcher implements ICodeWatcher {
if (cellMatcher.isCell(editor.document.lineAt(i).text)) {
lastCell = false;
index = i;
editBuilder.insert(new Position(i, 0), '#%%\n\n');
editBuilder.insert(new Position(i, 0), `${cellDelineator}\n\n`);
break;
}
}

if (lastCell) {
index = editor.document.lineCount;
editBuilder.insert(new Position(editor.document.lineCount, 0), '\n#%%\n');
editBuilder.insert(new Position(editor.document.lineCount, 0), `\n${cellDelineator}\n`);
}
});
}
Expand All @@ -313,6 +315,10 @@ export class CodeWatcher implements ICodeWatcher {
.then(() => this.advanceToRange(new Range(newPosition, newPosition)));
}

private get defaultCellMarker(): string {
return this.configService.getSettings().datascience.defaultCellMarker || Identifiers.DefaultCodeCellMarker;
}

private onCodeLensFactoryUpdated(): void {
// Update our code lenses.
if (this.document) {
Expand Down Expand Up @@ -419,7 +425,7 @@ export class CodeWatcher implements ICodeWatcher {

if (editor) {
editor.edit((editBuilder) => {
editBuilder.insert(new Position(currentRange.end.line + 1, 0), '\n\n#%%\n');
editBuilder.insert(new Position(currentRange.end.line + 1, 0), `\n\n${this.defaultCellMarker}\n`);
});
}

Expand Down
7 changes: 6 additions & 1 deletion src/client/datascience/gather/gather.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { Common } from '../../common/utils/localize';
import { noop } from '../../common/utils/misc';
import { CellMatcher } from '../cellMatcher';
import { concatMultilineString } from '../common';
import { Identifiers } from '../constants';
import { CellState, ICell as IVscCell, IGatherExecution, INotebookExecutionLogger } from '../types';

/**
Expand Down Expand Up @@ -79,9 +80,13 @@ export class GatherExecution implements IGatherExecution, INotebookExecutionLogg
if (cell === undefined) {
return '';
}

// Get the default cell marker as we need to replace #%% with it.
const defaultCellMarker = this.configService.getSettings().datascience.defaultCellMarker || Identifiers.DefaultCodeCellMarker;

// Call internal slice method
const slices = this._executionSlicer.sliceAllExecutions(cell);
const program = slices[0].cellSlices.reduce(concat, '');
const program = slices.length > 0 ? slices[0].cellSlices.reduce(concat, '').replace(/#%%/g, defaultCellMarker) : '';

// Add a comment at the top of the file explaining what gather does
const descriptor = '# This file contains the minimal amount of code required to produce the code cell you gathered.\n';
Expand Down
7 changes: 4 additions & 3 deletions src/client/datascience/interactive-common/interactiveBase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -998,20 +998,21 @@ export abstract class InteractiveBase extends WebViewHost<IInteractiveWindowMapp
const hasCellsAlready = ranges.length > 0;
const line = editor.selection.start.line;
const revealLine = line + 1;
const defaultCellMarker = this.configService.getSettings().datascience.defaultCellMarker || Identifiers.DefaultCodeCellMarker;
let newCode = `${source}${os.EOL}`;
if (hasCellsAlready) {
// See if inside of a range or not.
const matchingRange = ranges.find(r => r.range.start.line <= line && r.range.end.line >= line);

// If in the middle, wrap the new code
if (matchingRange && matchingRange.range.start.line < line && line < editor.document.lineCount - 1) {
newCode = `#%%${os.EOL}${source}${os.EOL}#%%${os.EOL}`;
newCode = `${defaultCellMarker}${os.EOL}${source}${os.EOL}${defaultCellMarker}${os.EOL}`;
} else {
newCode = `#%%${os.EOL}${source}${os.EOL}`;
newCode = `${defaultCellMarker}${os.EOL}${source}${os.EOL}`;
}
} else if (editor.document.lineCount <= 0 || editor.document.isUntitled) {
// No lines in the document at all, just insert new code
newCode = `#%%${os.EOL}${source}${os.EOL}`;
newCode = `${defaultCellMarker}${os.EOL}${source}${os.EOL}`;
}

await editor.edit((editBuilder) => {
Expand Down
19 changes: 12 additions & 7 deletions src/client/datascience/jupyter/jupyterImporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import * as fs from 'fs-extra';
import { inject, injectable } from 'inversify';
import * as os from 'os';
import * as path from 'path';
import '../../common/extensions';

import { IWorkspaceService } from '../../common/application/types';
import { IFileSystem, IPlatformService } from '../../common/platform/types';
Expand All @@ -20,16 +21,16 @@ import { InvalidNotebookFileError } from './invalidNotebookFileError';
export class JupyterImporter implements INotebookImporter {
public isDisposed: boolean = false;
// Template that changes markdown cells to have # %% [markdown] in the comments
private readonly nbconvertTemplate =
private readonly nbconvertTemplateFormat =
// tslint:disable-next-line:no-multiline-string
`{%- extends 'null.tpl' -%}
{% block codecell %}
#%%
{0}
{{ super() }}
{% endblock codecell %}
{% block in_prompt %}{% endblock in_prompt %}
{% block input %}{{ cell.source | ipython2python }}{% endblock input %}
{% block markdowncell scoped %}#%% [markdown]
{% block markdowncell scoped %}{0} [markdown]
{{ cell.source | comment_lines }}
{% endblock markdowncell %}`;

Expand Down Expand Up @@ -116,16 +117,20 @@ export class JupyterImporter implements INotebookImporter {
}

private addInstructionComments = (pythonOutput: string): string => {
const comments = localize.DataScience.instructionComments();
const comments = localize.DataScience.instructionComments().format(this.defaultCellMarker);
return comments.concat(pythonOutput);
}

private get defaultCellMarker(): string {
return this.configuration.getSettings().datascience.defaultCellMarker || Identifiers.DefaultCodeCellMarker;
}

private addIPythonImport = (pythonOutput: string): string => {
return CodeSnippits.ImportIPython.concat(pythonOutput);
return CodeSnippits.ImportIPython.format(this.defaultCellMarker, pythonOutput);
}

private addDirectoryChange = (pythonOutput: string, directoryChange: string): string => {
const newCode = CodeSnippits.ChangeDirectory.join(os.EOL).format(localize.DataScience.importChangeDirectoryComment(), CodeSnippits.ChangeDirectoryCommentIdentifier, directoryChange);
const newCode = CodeSnippits.ChangeDirectory.join(os.EOL).format(localize.DataScience.importChangeDirectoryComment().format(this.defaultCellMarker), CodeSnippits.ChangeDirectoryCommentIdentifier, directoryChange);
return newCode.concat(pythonOutput);
}

Expand Down Expand Up @@ -173,7 +178,7 @@ export class JupyterImporter implements INotebookImporter {
try {
// Save this file into our disposables so the temp file goes away
this.disposableRegistry.push(file);
await fs.appendFile(file.filePath, this.nbconvertTemplate);
await fs.appendFile(file.filePath, this.nbconvertTemplateFormat.format(this.defaultCellMarker));

// Now we should have a template that will convert
return file.filePath;
Expand Down
4 changes: 3 additions & 1 deletion src/test/datascience/gather/gather.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ suite('DataScience code gathering unit tests', () => {

dataScienceSettings.setup(d => d.gatherRules).returns(() => gatherRules);
dataScienceSettings.setup(d => d.enabled).returns(() => true);
dataScienceSettings.setup(d => d.defaultCellMarker).returns(() => '# %%');
pythonSettings.setup(p => p.datascience).returns(() => dataScienceSettings.object);
configurationService.setup(c => c.getSettings(TypeMoq.It.isAny())).returns(() => pythonSettings.object);
appShell.setup(a => a.showInformationMessage(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => Promise.resolve(''));
Expand All @@ -140,9 +141,10 @@ suite('DataScience code gathering unit tests', () => {
});

test('Gathers program slices for a cell', async () => {
const defaultCellMarker = '# %%';
const cell: IVscCell = codeCells[codeCells.length - 1];
const program = gatherExecution.gatherCode(cell);
const expectedProgram = `# This file contains the minimal amount of code required to produce the code cell you gathered.\n#%%\nfrom bokeh.plotting import show, figure, output_notebook\n\n#%%\nx = [1,2,3,4,5]\ny = [21,9,15,17,4]\n\n#%%\np=figure(title='demo',x_axis_label='x',y_axis_label='y')\n\n#%%\np.line(x,y,line_width=2)\n\n#%%\nshow(p)\n`;
const expectedProgram = `# This file contains the minimal amount of code required to produce the code cell you gathered.\n${defaultCellMarker}\nfrom bokeh.plotting import show, figure, output_notebook\n\n${defaultCellMarker}\nx = [1,2,3,4,5]\ny = [21,9,15,17,4]\n\n${defaultCellMarker}\np=figure(title='demo',x_axis_label='x',y_axis_label='y')\n\n${defaultCellMarker}\np.line(x,y,line_width=2)\n\n${defaultCellMarker}\nshow(p)\n`;
assert.equal(program.trim(), expectedProgram.trim());
});
});
Loading