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

Remove more unnecessary interpreter types #31

Merged
merged 1 commit into from
Sep 14, 2020
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
8 changes: 7 additions & 1 deletion .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,12 @@
"@typescript-eslint/no-empty-interface": "off",
"@typescript-eslint/no-non-null-assertion": "off",
"no-unused-vars": "off",
"@typescript-eslint/no-unused-vars": "error",
"@typescript-eslint/no-unused-vars": [
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For existing code

"error",
{
"argsIgnorePattern": "^_"
}
],
"no-use-before-define": "off",
"@typescript-eslint/no-use-before-define": [
"error",
Expand Down Expand Up @@ -87,6 +92,7 @@
"no-control-regex": "off",
"function-paren-newline": "off",
"consistent-return": "off",
"class-methods-use-this": "off",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For existing code.

"no-extend-native": "off",
"no-multi-str": "off",
"no-param-reassign": "off",
Expand Down
2 changes: 1 addition & 1 deletion src/client/activation/activationService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ export class LanguageServerExtensionActivationService
workspacePathNameForGlobalWorkspaces
);
interpreter = interpreter ? interpreter : await this.interpreterService.getActiveInterpreter(resource);
const interperterPortion = interpreter ? `${interpreter.path}-${interpreter.envName}` : '';
const interperterPortion = interpreter ? `${interpreter.path}` : '';
return `${resourcePortion}-${interperterPortion}`;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export class KernelDependencyService implements IKernelDependencyService {
token
});
const message = DataScience.libraryRequiredToLaunchJupyterKernelNotInstalledInterpreter().format(
interpreter.displayName || interpreter.envName || interpreter.path,
interpreter.displayName || interpreter.path,
ProductNames.get(Product.ipykernel)!
);
const installerToken = wrapCancellationTokens(token);
Expand Down
1 change: 0 additions & 1 deletion src/client/datascience/jupyter/kernels/kernelService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ function isInterpreter(item: nbformat.IKernelspecMetadata | PythonEnvironment):
// Interpreters will not have a `display_name` property, but have `path` and `type` properties.
return (
!!(item as PythonEnvironment).path &&
!!(item as PythonEnvironment).envType &&
!(item as nbformat.IKernelspecMetadata).display_name
);
}
Expand Down
22 changes: 0 additions & 22 deletions src/client/pythonEnvironments/info/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,8 @@

'use strict';

import { Architecture } from '../../common/utils/platform';
import { PythonVersion } from './pythonVersion';

/**
* The supported Python environment types.
*/
export enum EnvironmentType {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not used in DS code.

Unknown = 'Unknown',
Conda = 'Conda',
VirtualEnv = 'VirtualEnv',
Pipenv = 'PipEnv',
Pyenv = 'Pyenv',
Venv = 'Venv',
WindowsStore = 'WindowsStore',
Poetry = 'Poetry',
VirtualEnvWrapper = 'VirtualEnvWrapper',
Global = 'Global',
System = 'System'
}

type ReleaseLevel = 'alpha' | 'beta' | 'candidate' | 'final' | 'unknown';

/**
Expand All @@ -46,7 +28,6 @@ export type InterpreterInformation = {
path: string;
version?: PythonVersion;
sysVersion?: string;
architecture: Architecture;
sysPrefix: string;
};

Expand All @@ -64,7 +45,4 @@ export type InterpreterInformation = {
// and doesn't really belong here.
export type PythonEnvironment = InterpreterInformation & {
displayName?: string;
envType: EnvironmentType;
envName?: string;
envPath?: string;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

None of these were used in DS code, only in tests.

};
4 changes: 1 addition & 3 deletions src/client/pythonEnvironments/info/interpreter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,9 @@
// Licensed under the MIT License.

import { InterpreterInformation } from '.';
import { parsePythonVersion } from '../../../test/interpreters/pythonVersion';
import { interpreterInfo as getInterpreterInfoCommand, PythonEnvInfo } from '../../common/process/internal/scripts';
import { Architecture } from '../../common/utils/platform';
import { copyPythonExecInfo, PythonExecInfo } from '../exec';
import { parsePythonVersion } from './pythonVersion';

/**
* Compose full interpreter information based on the given data.
Expand All @@ -18,7 +17,6 @@ import { parsePythonVersion } from './pythonVersion';
export function extractInterpreterInfo(python: string, raw: PythonEnvInfo): InterpreterInformation {
const rawVersion = `${raw.versionInfo.slice(0, 3).join('.')}-${raw.versionInfo[3]}`;
return {
architecture: raw.is64Bit ? Architecture.x64 : Architecture.x86,
path: python,
version: parsePythonVersion(rawVersion),
sysVersion: raw.sysVersion,
Expand Down
47 changes: 0 additions & 47 deletions src/client/pythonEnvironments/info/pythonVersion.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

import { SemVer } from 'semver';
import '../../common/extensions'; // For string.splitLines()

/**
* A representation of a Python runtime's version.
*
Expand All @@ -28,47 +25,3 @@ export type PythonVersion = {
build: string[];
prerelease: string[];
};

/**
* Convert a Python version string.
*
* The supported formats are:
*
* * MAJOR.MINOR.MICRO-RELEASE_LEVEL
*
* (where RELEASE_LEVEL is one of {alpha,beta,candidate,final})
*
* Everything else, including an empty string, results in `undefined`.
*/
// Eventually we will want to also support the release serial
// (e.g. beta1, candidate3) and maybe even release abbreviations
// (e.g. 3.9.2b1, 3.8.10rc3).
export function parsePythonVersion(raw: string): PythonVersion | undefined {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only used in tests, hence moved to tests folder.

if (!raw || raw.trim().length === 0) {
return;
}
const versionParts = (raw || '')
.split('.')
.map((item) => item.trim())
.filter((item) => item.length > 0)
.filter((_, index) => index < 4);

if (versionParts.length > 0 && versionParts[versionParts.length - 1].indexOf('-') > 0) {
const lastPart = versionParts[versionParts.length - 1];
versionParts[versionParts.length - 1] = lastPart.split('-')[0].trim();
versionParts.push(lastPart.split('-')[1].trim());
}
while (versionParts.length < 4) {
versionParts.push('');
}
// Exclude PII from `version_info` to ensure we don't send this up via telemetry.
for (let index = 0; index < 3; index += 1) {
versionParts[index] = /^\d+$/.test(versionParts[index]) ? versionParts[index] : '0';
}
if (['alpha', 'beta', 'candidate', 'final'].indexOf(versionParts[3]) === -1) {
versionParts.pop();
}
const numberParts = `${versionParts[0]}.${versionParts[1]}.${versionParts[2]}`;
const rawVersion = versionParts.length === 4 ? `${numberParts}-${versionParts[3]}` : numberParts;
return new SemVer(rawVersion);
}
19 changes: 0 additions & 19 deletions src/client/telemetry/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import { ExportFormat } from '../datascience/export/types';
import { DebugConfigurationType } from '../debugger/extension/types';
import { ConsoleType, TriggerType } from '../debugger/types';
import { LinterId } from '../linters/types';
import { EnvironmentType } from '../pythonEnvironments/info';
import { EventName, PlatformErrors } from './constants';
import { LinterTrigger } from './types';

Expand Down Expand Up @@ -982,12 +981,6 @@ export interface IEventNamePropertyMapping {
* @type {string}
*/
pythonVersion?: string;
/**
* The type of the interpreter used
*
* @type {EnvironmentType}
*/
interpreterType: EnvironmentType;
};
/**
* Telemetry event sent when getting activation commands for terminal when interpreter is not specified
Expand All @@ -1011,12 +1004,6 @@ export interface IEventNamePropertyMapping {
* @type {string}
*/
pythonVersion?: string;
/**
* The type of the interpreter used
*
* @type {EnvironmentType}
*/
interpreterType: EnvironmentType;
};
[EventName.PYTHON_INTERPRETER_AUTO_SELECTION]: {
/**
Expand Down Expand Up @@ -1447,12 +1434,6 @@ export interface IEventNamePropertyMapping {
* @type {string}
*/
pythonVersion?: string;
/**
* The Python interpreter type: Conda, Virtualenv, Venv, Pipenv etc.
*
* @type {EnvironmentType}
*/
interpreterType?: EnvironmentType;
};
/**
* Telemetry sent when building workspace symbols
Expand Down
15 changes: 12 additions & 3 deletions src/test/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,8 @@ export function isOs(...OSes: OSType[]): boolean {
return true;
}

export function getOSType(platform: string = process.platform): OSType {
export function getOSType(): OSType {
const platform: string = process.platform;
if (/^win/.test(platform)) {
return OSType.Windows;
} else if (/^darwin/.test(platform)) {
Expand Down Expand Up @@ -685,8 +686,16 @@ export class TestEventHandler<T extends void | any = any> implements IDisposable
export function createEventHandler<T, K extends keyof T>(
obj: T,
eventName: K,
dispoables: IDisposable[] = []
disposables: IDisposable[] = []
): T[K] extends Event<infer TArgs> ? TestEventHandler<TArgs> : TestEventHandler<void> {
// tslint:disable-next-line: no-any
return new TestEventHandler(obj[eventName] as any, eventName as string, dispoables) as any;
return new TestEventHandler(obj[eventName] as any, eventName as string, disposables) as any;
}

export function arePathsSame(path1: string, path2: string) {
if (getOSType() === OSType.Windows) {
return path1.toLowerCase() === path2.toLowerCase();
} else {
return path1 === path2;
}
}
2 changes: 1 addition & 1 deletion src/test/common/process/pythonDaemon.functional.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ import { IDisposable } from '../../../client/common/types';
import { Architecture } from '../../../client/common/utils/platform';
import { EXTENSION_ROOT_DIR } from '../../../client/constants';
import { PythonVersionInfo } from '../../../client/pythonEnvironments/info';
import { parsePythonVersion } from '../../../client/pythonEnvironments/info/pythonVersion';
import { isPythonVersion, PYTHON_PATH } from '../../common';
import { parsePythonVersion } from '../../interpreters/pythonVersion';
import { createTemporaryFile } from '../../utils/fs';
use(chaiPromised);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ import { Architecture } from '../../../client/common/utils/platform';
import { EXTENSION_ROOT_DIR } from '../../../client/constants';
import { JupyterDaemonModule } from '../../../client/datascience/constants';
import { PythonVersionInfo } from '../../../client/pythonEnvironments/info';
import { parsePythonVersion } from '../../../client/pythonEnvironments/info/pythonVersion';
import { isPythonVersion, PYTHON_PATH, waitForCondition } from '../../common';
import { parsePythonVersion } from '../../interpreters/pythonVersion';
import { createTemporaryFile } from '../../utils/fs';
use(chaiPromised);

Expand Down
7 changes: 2 additions & 5 deletions src/test/common/process/pythonExecutionFactory.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,22 +26,19 @@ import {
IPythonExecutionService
} from '../../../client/common/process/types';
import { IConfigurationService, IDisposableRegistry } from '../../../client/common/types';
import { Architecture } from '../../../client/common/utils/platform';
import { IEnvironmentActivationService } from '../../../client/interpreter/activation/types';
import { IInterpreterService } from '../../../client/interpreter/contracts';
import { IWindowsStoreInterpreter } from '../../../client/interpreter/locators/types';
import { ServiceContainer } from '../../../client/ioc/container';
import { EnvironmentType, PythonEnvironment } from '../../../client/pythonEnvironments/info';
import { PythonEnvironment } from '../../../client/pythonEnvironments/info';

// tslint:disable:no-any max-func-body-length chai-vague-errors

const pythonInterpreter: PythonEnvironment = {
path: '/foo/bar/python.exe',
version: new SemVer('3.6.6-final'),
sysVersion: '1.0.0.0',
sysPrefix: 'Python',
envType: EnvironmentType.Unknown,
architecture: Architecture.x64
sysPrefix: 'Python'
};

function title(resource?: Uri, interpreter?: PythonEnvironment) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import { IKernelFinder } from '../../../client/datascience/kernel-launcher/types
import { NativeEditorProvider } from '../../../client/datascience/notebookStorage/nativeEditorProvider';
import { IInteractiveWindowProvider, INotebookEditorProvider } from '../../../client/datascience/types';
import { IInterpreterService } from '../../../client/interpreter/contracts';
import { EnvironmentType } from '../../../client/pythonEnvironments/info';

// tslint:disable: max-func-body-length no-any
suite('DataScience - Notebook Commands', () => {
Expand Down Expand Up @@ -58,7 +57,6 @@ suite('DataScience - Notebook Commands', () => {
};
const selectedInterpreter = {
path: '',
envType: EnvironmentType.Conda,
architecture: Architecture.Unknown,
sysPrefix: '',
sysVersion: ''
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,8 @@ import { PythonExecutionFactory } from '../../../client/common/process/pythonExe
import { IPythonExecutionFactory, IPythonExecutionService } from '../../../client/common/process/types';
import { IInstaller, Product } from '../../../client/common/types';
import { Common, DataScience } from '../../../client/common/utils/localize';
import { Architecture } from '../../../client/common/utils/platform';
import { DataViewerDependencyService } from '../../../client/datascience/data-viewing/dataViewerDependencyService';
import { EnvironmentType, PythonEnvironment } from '../../../client/pythonEnvironments/info';
import { PythonEnvironment } from '../../../client/pythonEnvironments/info';

suite('DataScience - DataViewerDependencyService', () => {
let dependencyService: DataViewerDependencyService;
Expand All @@ -27,12 +26,10 @@ suite('DataScience - DataViewerDependencyService', () => {
let pythonExecService: IPythonExecutionService;
setup(async () => {
interpreter = {
architecture: Architecture.Unknown,
displayName: '',
path: path.join('users', 'python', 'bin', 'python.exe'),
sysPrefix: '',
sysVersion: '',
envType: EnvironmentType.Unknown,
version: new SemVer('3.3.3')
};
pythonExecService = mock<IPythonExecutionService>();
Expand Down
11 changes: 3 additions & 8 deletions src/test/datascience/dataScienceIocContainer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,6 @@ import {
import { sleep } from '../../client/common/utils/async';
import { noop } from '../../client/common/utils/misc';
import { IMultiStepInputFactory, MultiStepInputFactory } from '../../client/common/utils/multiStepInput';
import { Architecture } from '../../client/common/utils/platform';
import { EnvironmentVariablesService } from '../../client/common/variables/environment';
import { EnvironmentVariablesProvider } from '../../client/common/variables/environmentVariablesProvider';
import { IEnvironmentVariablesProvider, IEnvironmentVariablesService } from '../../client/common/variables/types';
Expand Down Expand Up @@ -306,7 +305,7 @@ import {
import { ProtocolParser } from '../../client/debugger/extension/helpers/protocolParser';
import { IProtocolParser } from '../../client/debugger/extension/types';
import { IInterpreterService } from '../../client/interpreter/contracts';
import { EnvironmentType, PythonEnvironment } from '../../client/pythonEnvironments/info';
import { PythonEnvironment } from '../../client/pythonEnvironments/info';
import { CodeExecutionHelper } from '../../client/terminals/codeExecution/helper';
import { ICodeExecutionHelper } from '../../client/terminals/types';
import { MockOutputChannel } from '../mockClasses';
Expand Down Expand Up @@ -379,18 +378,14 @@ export class DataScienceIocContainer extends UnitTestIocContainer {
version: new SemVer('3.6.6-final'),
sysVersion: '1.0.0.0',
sysPrefix: 'Python',
displayName: 'Python',
envType: EnvironmentType.Unknown,
architecture: Architecture.x64
displayName: 'Python'
};
private workingPython2: PythonEnvironment = {
path: '/foo/baz/python.exe',
version: new SemVer('3.6.7-final'),
sysVersion: '1.0.0.0',
sysPrefix: 'Python',
displayName: 'Python',
envType: EnvironmentType.Unknown,
architecture: Architecture.x64
displayName: 'Python'
};

private webPanelProvider = mock(WebviewPanelProvider);
Expand Down
Loading