Proposal: Isomorphic pythonVariableRequester

microsoft · May 26, 2022 · e5cf36f · e5cf36f
1 parent 2f5f881
commit e5cf36f
Show file tree

Hide file tree

Showing 10 changed files with 66 additions and 53 deletions.
diff --git a/src/kernels/installer/poetry.node.ts b/src/kernels/installer/poetry.node.ts
@@ -18,7 +18,7 @@ import {
     isVirtualenvEnvironment,
     pathExists
 } from '../../platform/common/platform/fileUtils.node';
-import { isTestExecution } from '../../platform/common/constants.node';
+import { isTestExecution } from '../../platform/common/constants';
 
 /**
  * Global virtual env dir for a project is named as:

diff --git a/src/kernels/kernel.node.ts b/src/kernels/kernel.node.ts
@@ -2,6 +2,7 @@
 // Licensed under the MIT License.
 
 'use strict';
+import * as path from '../platform/vscode-path/path';
 import { NotebookController, Uri } from 'vscode';
 import { IApplicationShell, IWorkspaceService } from '../platform/common/application/types';
 import { traceInfo, traceError } from '../platform/logging';
@@ -21,11 +22,12 @@ import {
     KernelActionSource,
     KernelConnectionMetadata
 } from './types';
-import { AddRunCellHook } from '../platform/common/constants.node';
+import { AddRunCellHook } from '../platform/common/scriptConstants';
 import { IStatusProvider } from '../platform/progress/types';
 import { getAssociatedNotebookDocument } from '../notebooks/controllers/kernelSelector';
 import { sendTelemetryForPythonKernelExecutable } from './helpers.node';
 import { BaseKernel } from './kernel.base';
+import { EXTENSION_ROOT_DIR } from '../platform/constants.node';
 
 export class Kernel extends BaseKernel {
     constructor(
@@ -79,11 +81,12 @@ export class Kernel extends BaseKernel {
         if (getAssociatedNotebookDocument(this)?.notebookType === InteractiveWindowView) {
             // If using ipykernel 6, we need to set the IPYKERNEL_CELL_NAME so that
             // debugging can work. However this code is harmless for IPYKERNEL 5 so just always do it
-            if (await this.fs.localFileExists(AddRunCellHook.ScriptPath)) {
-                const fileContents = await this.fs.readLocalFile(AddRunCellHook.ScriptPath);
+            const scriptPath = path.join(EXTENSION_ROOT_DIR, AddRunCellHook.ScriptPath);
+            if (await this.fs.localFileExists(scriptPath)) {
+                const fileContents = await this.fs.readLocalFile(scriptPath);
                 return fileContents.splitLines({ trim: false });
             }
-            traceError(`Cannot run non-existent script file: ${AddRunCellHook.ScriptPath}`);
+            traceError(`Cannot run non-existent script file: ${scriptPath}`);
         }
         return [];
     }

diff --git a/src/kernels/raw/launcher/kernelLauncher.node.ts b/src/kernels/raw/launcher/kernelLauncher.node.ts
@@ -32,7 +32,7 @@ import { IKernelLauncher, IKernelProcess, IKernelConnection } from '../types';
 import { KernelEnvironmentVariablesService } from './kernelEnvVarsService.node';
 import { KernelProcess } from './kernelProcess.node';
 import { JupyterPaths } from '../finder/jupyterPaths.node';
-import { isTestExecution } from '../../../platform/common/constants.node';
+import { isTestExecution } from '../../../platform/common/constants';
 import { getDisplayPathFromLocalFile } from '../../../platform/common/platform/fs-paths.node';
 import { noop } from '../../../platform/common/utils/misc';
 

diff --git a/src/kernels/serviceRegistry.node.ts b/src/kernels/serviceRegistry.node.ts
@@ -32,7 +32,7 @@ import { DebuggerVariables } from './variables/debuggerVariables.node';
 import { JupyterVariables } from './variables/jupyterVariables';
 import { KernelVariables } from './variables/kernelVariables';
 import { PreWarmActivatedJupyterEnvironmentVariables } from './variables/preWarmVariables.node';
-import { PythonVariablesRequester } from './variables/pythonVariableRequester.node';
+import { PythonVariablesRequester } from './variables/pythonVariableRequester';
 import { IInteractiveWindowDebugger } from '../interactive-window/types';
 import { MultiplexingDebugService } from './debugger/multiplexingDebugService';
 import { JupyterVariableDataProvider } from '../webviews/extension-side/dataviewer/jupyterVariableDataProvider';

diff --git a/src/kernels/variables/debuggerVariables.node.ts b/src/kernels/variables/debuggerVariables.node.ts
@@ -8,22 +8,24 @@ import { DebugAdapterTracker, Disposable, Event, EventEmitter } from 'vscode';
 import { DebugProtocol } from 'vscode-debugprotocol';
 import { getAssociatedNotebookDocument } from '../../notebooks/controllers/kernelSelector';
 import { IDebugService, IVSCodeNotebook } from '../../platform/common/application/types';
-import { DataFrameLoading, GetVariableInfo } from '../../platform/common/constants.node';
+import { DataFrameLoading, GetVariableInfo } from '../../platform/common/scriptConstants';
 import { traceError } from '../../platform/logging';
 import { IConfigurationService, Resource } from '../../platform/common/types';
 import { DebugLocationTracker } from '../debugger/debugLocationTracker';
 import { sendTelemetryEvent } from '../../telemetry';
 import { Identifiers, Telemetry } from '../../webviews/webview-side/common/constants';
 import { IDebuggingManager, IJupyterDebugService, KernelDebugMode } from '../debugger/types';
 import { IKernel } from '../types';
-import { parseDataFrame } from './pythonVariableRequester.node';
+import { parseDataFrame } from './pythonVariableRequester';
 import {
     IConditionalJupyterVariables,
     IJupyterVariable,
     IJupyterVariablesRequest,
     IJupyterVariablesResponse
 } from './types';
 import { convertDebugProtocolVariableToIJupyterVariable, DataViewableTypes } from './helpers';
+import { EXTENSION_ROOT_DIR } from '../../platform/constants.node';
+import { IFileSystemNode } from '../../platform/common/platform/types.node';
 
 const KnownExcludedVariables = new Set<string>(['In', 'Out', 'exit', 'quit']);
 const MaximumRowChunkSizeForDebugger = 100;
@@ -46,7 +48,8 @@ export class DebuggerVariables
         @inject(IJupyterDebugService) @named(Identifiers.MULTIPLEXING_DEBUGSERVICE) private debugService: IDebugService,
         @inject(IDebuggingManager) private readonly debuggingManager: IDebuggingManager,
         @inject(IConfigurationService) private configService: IConfigurationService,
-        @inject(IVSCodeNotebook) private readonly vscNotebook: IVSCodeNotebook
+        @inject(IVSCodeNotebook) private readonly vscNotebook: IVSCodeNotebook,
+        @inject(IFileSystemNode) private readonly fs: IFileSystemNode
     ) {
         super(undefined);
         this.debuggingManager.onDoneDebugging(() => this.refreshEventEmitter.fire(), this);
@@ -316,7 +319,9 @@ export class DebuggerVariables
             // Run our dataframe scripts only once per session because they're slow
             const key = this.debugService.activeDebugSession?.id;
             if (key && !this.importedDataFrameScriptsIntoKernel.has(key)) {
-                await this.evaluate(DataFrameLoading.DataFrameSysImport);
+                const scriptPath = path.join(EXTENSION_ROOT_DIR, DataFrameLoading.ScriptPath);
+                const contents = await this.fs.readLocalFile(scriptPath);
+                await this.evaluate(contents);
                 this.importedDataFrameScriptsIntoKernel.add(key);
             }
         } catch (exc) {
@@ -329,7 +334,9 @@ export class DebuggerVariables
             // Run our variable info scripts only once per session because they're slow
             const key = this.debugService.activeDebugSession?.id;
             if (key && !this.importedGetVariableInfoScriptsIntoKernel.has(key)) {
-                await this.evaluate(GetVariableInfo.GetVariableInfoSysImport);
+                const scriptPath = path.join(EXTENSION_ROOT_DIR, DataFrameLoading.ScriptPath);
+                const contents = await this.fs.readLocalFile(scriptPath);
+                await this.evaluate(contents);
                 this.importedGetVariableInfoScriptsIntoKernel.add(key);
             }
         } catch (exc) {

diff --git a/...variables/pythonVariableRequester.node.ts → ...nels/variables/pythonVariableRequester.ts b/...variables/pythonVariableRequester.node.ts → ...nels/variables/pythonVariableRequester.ts
@@ -1,10 +1,9 @@
 import { IDisposable } from '@fluentui/react';
 import type * as nbformat from '@jupyterlab/nbformat';
 import { inject, injectable } from 'inversify';
-import * as path from '../../platform/vscode-path/path';
-import { CancellationToken, NotebookDocument } from 'vscode';
+import { CancellationToken, NotebookDocument, Uri } from 'vscode';
 import { traceError } from '../../platform/logging';
-import { IFileSystemNode } from '../../platform/common/platform/types.node';
+import { IFileSystem } from '../../platform/common/platform/types';
 import { DataScience } from '../../platform/common/utils/localize';
 import { stripAnsi } from '../../platform/common/utils/regexp';
 import { JupyterDataRateLimitError } from '../../platform/errors/jupyterDataRateLimitError';
@@ -13,7 +12,9 @@ import { executeSilently } from '../helpers';
 import { IKernel } from '../types';
 import { IKernelVariableRequester, IJupyterVariable } from './types';
 import { getAssociatedNotebookDocument } from '../../notebooks/controllers/kernelSelector';
-import { DataFrameLoading, GetVariableInfo } from '../../platform/common/constants.node';
+import { DataFrameLoading, GetVariableInfo } from '../../platform/common/scriptConstants';
+import { joinPath } from '../../platform/vscode-path/resources';
+import { IExtensionContext } from '../../platform/common/types';
 
 type DataFrameSplitFormat = {
     index: (number | string)[];
@@ -42,7 +43,10 @@ export class PythonVariablesRequester implements IKernelVariableRequester {
     private importedDataFrameScripts = new WeakMap<NotebookDocument, boolean>();
     private importedGetVariableInfoScripts = new WeakMap<NotebookDocument, boolean>();
 
-    constructor(@inject(IFileSystemNode) private fs: IFileSystemNode) {}
+    constructor(
+        @inject(IFileSystem) private fs: IFileSystem,
+        @inject(IExtensionContext) private readonly context: IExtensionContext
+    ) {}
 
     public async getDataFrameInfo(
         targetVariable: IJupyterVariable,
@@ -65,9 +69,7 @@ export class PythonVariablesRequester implements IKernelVariableRequester {
               )
             : [];
 
-        const fileName = path.basename(
-            getAssociatedNotebookDocument(kernel)?.uri.fsPath || kernel.resourceUri?.fsPath || kernel.id.path
-        );
+        const fileName = getAssociatedNotebookDocument(kernel)?.uri || kernel.resourceUri || kernel.id;
 
         // Combine with the original result (the call only returns the new fields)
         return {
@@ -225,7 +227,7 @@ export class PythonVariablesRequester implements IKernelVariableRequester {
             disposables.push(kernel.onRestarted(handler));
 
             // First put the code from our helper files into the notebook
-            await this.runScriptFile(kernel, DataFrameLoading.ScriptPath);
+            await this.runScriptFile(kernel, joinPath(this.context.extensionUri, DataFrameLoading.ScriptPath));
 
             this.importedDataFrameScripts.set(key, true);
         }
@@ -243,19 +245,19 @@ export class PythonVariablesRequester implements IKernelVariableRequester {
             disposables.push(kernel.onDisposed(handler));
             disposables.push(kernel.onRestarted(handler));
 
-            await this.runScriptFile(kernel, GetVariableInfo.ScriptPath);
+            await this.runScriptFile(kernel, joinPath(this.context.extensionUri, GetVariableInfo.ScriptPath));
 
             this.importedGetVariableInfoScripts.set(key, true);
         }
     }
 
     // Read in a .py file and execute it silently in the given notebook
-    private async runScriptFile(kernel: IKernel, scriptFile: string) {
-        if (await this.fs.localFileExists(scriptFile)) {
-            const fileContents = await this.fs.readLocalFile(scriptFile);
+    private async runScriptFile(kernel: IKernel, scriptFile: Uri) {
+        if (await this.fs.exists(scriptFile)) {
+            const fileContents = await this.fs.readFile(scriptFile);
             return kernel.session ? executeSilently(kernel.session, fileContents) : [];
         }
-        traceError('Cannot run non-existant script file');
+        traceError('Cannot run non-existent script file');
     }
 
     private extractJupyterResultText(outputs: nbformat.IOutput[]): string {

diff --git a/src/kernels/variables/types.ts b/src/kernels/variables/types.ts
@@ -2,7 +2,7 @@
 // Licensed under the MIT License.
 'use strict';
 
-import { CancellationToken, Event } from 'vscode';
+import { CancellationToken, Event, Uri } from 'vscode';
 import { IKernel } from '../types';
 import type { JSONObject } from '@lumino/coreutils';
 
@@ -24,7 +24,7 @@ export interface IJupyterVariable {
     rowCount?: number;
     indexColumn?: string;
     maximumRowChunkSize?: number;
-    fileName?: string;
+    fileName?: Uri;
 }
 
 export const IJupyterVariables = Symbol('IJupyterVariables');

diff --git a/src/platform/common/constants.node.ts → src/platform/common/scriptConstants.ts b/src/platform/common/constants.node.ts → src/platform/common/scriptConstants.ts
@@ -1,11 +1,9 @@
-import { EXTENSION_ROOT_DIR } from '../constants.node';
-import * as path from '../../platform/vscode-path/path';
+import * as path from '../vscode-path/path';
 
 export * from './constants';
 
 export namespace DataFrameLoading {
-    export const SysPath = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'vscode_datascience_helpers', 'dataframes');
-    export const DataFrameSysImport = `import sys\nsys.path.append("${SysPath.replace(/\\/g, '\\\\')}")`;
+    export const SysPath = path.join('pythonFiles', 'vscode_datascience_helpers', 'dataframes');
     export const ScriptPath = path.join(SysPath, 'vscodeDataFrame.py');
 
     export const DataFrameInfoFunc = '_VSCODE_getDataFrameInfo';
@@ -18,13 +16,7 @@ export namespace DataFrameLoading {
 }
 
 export namespace GetVariableInfo {
-    export const SysPath = path.join(
-        EXTENSION_ROOT_DIR,
-        'pythonFiles',
-        'vscode_datascience_helpers',
-        'getVariableInfo'
-    );
-    export const GetVariableInfoSysImport = `import sys\nsys.path.append("${SysPath.replace(/\\/g, '\\\\')}")`;
+    export const SysPath = path.join('pythonFiles', 'vscode_datascience_helpers', 'getVariableInfo');
     export const ScriptPath = path.join(SysPath, 'vscodeGetVariableInfo.py');
     export const VariableInfoFunc = '_VSCODE_getVariableInfo';
     export const VariablePropertiesFunc = '_VSCODE_getVariableProperties';
@@ -36,6 +28,6 @@ export namespace GetVariableInfo {
 }
 
 export namespace AddRunCellHook {
-    export const SysPath = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'vscode_datascience_helpers', 'kernel');
+    export const SysPath = path.join('pythonFiles', 'vscode_datascience_helpers', 'kernel');
     export const ScriptPath = path.join(SysPath, 'addRunCellHook.py');
 }
diff --git a/src/test/datascience/widgets/notebooks/bqplot_widgets.ipynb b/src/test/datascience/widgets/notebooks/bqplot_widgets.ipynb
@@ -7,13 +7,11 @@
                 "# Prerequisites\n",
                 "\n",
                 "pip install bqplot"
-            ],
-            "execution_count": null,
-            "outputs": []
+            ]
         },
         {
             "cell_type": "code",
-            "execution_count": 126,
+            "execution_count": 40,
             "metadata": {},
             "outputs": [],
             "source": [
@@ -29,9 +27,24 @@
         },
         {
             "cell_type": "code",
-            "execution_count": 127,
+            "execution_count": 41,
             "metadata": {},
-            "outputs": [],
+            "outputs": [
+                {
+                    "data": {
+                        "application/vnd.jupyter.widget-view+json": {
+                            "model_id": "4d526b44599140a794c03e362abdc492",
+                            "version_major": 2,
+                            "version_minor": 0
+                        },
+                        "text/plain": [
+                            "VBox(children=(Figure(axes=[Axis(scale=LinearScale()), Axis(orientation='vertical', scale=LinearScale())], fig…"
+                        ]
+                    },
+                    "metadata": {},
+                    "output_type": "display_data"
+                }
+            ],
             "source": [
                 "plt.figure(title='My First Plot')\n",
                 "plt.plot(x_data, y_data)\n",
@@ -43,9 +56,7 @@
             "metadata": {},
             "source": [
                 "## Using `bqplot`'s interactive elements"
-            ],
-            "execution_count": null,
-            "outputs": []
+            ]
         },
         {
             "cell_type": "code",
@@ -73,9 +84,7 @@
             "metadata": {},
             "source": [
                 "# Change color of plots"
-            ],
-            "execution_count": null,
-            "outputs": []
+            ]
         },
         {
             "cell_type": "code",

diff --git a/src/webviews/extension-side/dataviewer/jupyterVariableDataProvider.ts b/src/webviews/extension-side/dataviewer/jupyterVariableDataProvider.ts
@@ -115,7 +115,7 @@ export class JupyterVariableDataProvider implements IJupyterVariableDataProvider
                 type: variable.type,
                 maximumRowChunkSize: variable.maximumRowChunkSize,
                 name: variable.name,
-                fileName: variable.fileName
+                fileName: variable.fileName?.path
             };
         }
         if (isRefresh) {