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

Sort available environments by assumed usefulness #16559

Merged
merged 15 commits into from
Jul 8, 2021
Merged
1 change: 1 addition & 0 deletions news/1 Enhancements/16520.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Sort environments in the selection quickpick by assumed usefulness.
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -1129,6 +1129,7 @@
"pythonDiscoveryModuleWithoutWatcher",
"pythonJediLSP",
"pythonSurveyNotification",
"pythonSortEnvs",
"All"
]
},
Expand All @@ -1146,6 +1147,7 @@
"pythonDiscoveryModuleWithoutWatcher",
"pythonJediLSP",
"pythonSurveyNotification",
"pythonSortEnvs",
"All"
]
},
Expand Down
5 changes: 5 additions & 0 deletions src/client/common/experiments/groups.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,8 @@ export enum FindInterpreterVariants {
export enum TorchProfiler {
experiment = 'PythonPyTorchProfiler',
}

// Experiment to use the new environment sorting algorithm in the interpreter quickpick.
export enum EnvironmentSorting {
experiment = 'pythonSortEnvs',
kimadeline marked this conversation as resolved.
Show resolved Hide resolved
}
188 changes: 188 additions & 0 deletions src/client/interpreter/configuration/environmentTypeComparer.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,188 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

import { injectable, inject } from 'inversify';
import { getArchitectureDisplayName } from '../../common/platform/registry';
import { isParentPath } from '../../pythonEnvironments/common/externalDependencies';
import { EnvironmentType, PythonEnvironment } from '../../pythonEnvironments/info';
import { PythonVersion } from '../../pythonEnvironments/info/pythonVersion';
import { IInterpreterHelper } from '../contracts';
import { IInterpreterComparer } from './types';

/*
* Enum description:
* - Local environments (.venv);
* - Global environments (pipenv, conda);
* - Globally-installed interpreters (/usr/bin/python3, Windows Store).
*/
enum EnvTypeHeuristic {
Local = 1,
Global = 2,
GlobalInterpreters = 3,
}

@injectable()
export class EnvironmentTypeComparer implements IInterpreterComparer {
private workspaceFolderPath: string;

constructor(@inject(IInterpreterHelper) private readonly interpreterHelper: IInterpreterHelper) {
this.workspaceFolderPath = this.interpreterHelper.getActiveWorkspaceUri(undefined)?.folderUri.fsPath ?? '';
}

/**
* Compare 2 Python environments, sorting them by assumed usefulness.
* Return 0 if both environments are equal, -1 if a should be closer to the beginning of the list, or 1 if a comes after b.
*
* The comparison guidelines are:
* 1. Local environments first (same path as the workspace root);
* 2. Global environments next (anything not local), with conda environments at a lower priority, and "base" being last;
* 3. Globally-installed interpreters (/usr/bin/python3, Windows Store).
*
* Always sort with newest version of Python first within each subgroup.
*/
public compare(a: PythonEnvironment, b: PythonEnvironment): number {
// Check environment type.
const envTypeComparison = compareEnvironmentType(a, b, this.workspaceFolderPath);
if (envTypeComparison !== 0) {
return envTypeComparison;
}

// Check Python version.
const versionComparison = comparePythonVersionDescending(a.version, b.version);
if (versionComparison !== 0) {
return versionComparison;
}

// Prioritize non-Conda environments.
if (isCondaEnvironment(a) && !isCondaEnvironment(b)) {
return 1;
}

if (!isCondaEnvironment(a) && isCondaEnvironment(b)) {
return -1;
}

// If we have the "base" Conda env, put it last in its Python version subgroup.
if (isBaseCondaEnvironment(a)) {
return 1;
}

// Check alphabetical order (same way as the InterpreterComparer class).
const nameA = getSortName(a, this.interpreterHelper);
const nameB = getSortName(b, this.interpreterHelper);
if (nameA === nameB) {
return 0;
}

return nameA > nameB ? 1 : -1;
}
}

// This function is exported because the InterpreterComparer class uses the same logic.
// Once it gets removed as we ramp up #16520, we can restrict this function to this file.
export function getSortName(info: PythonEnvironment, interpreterHelper: IInterpreterHelper): string {
const sortNameParts: string[] = [];
const envSuffixParts: string[] = [];

// Sort order for interpreters is:
// * Version
// * Architecture
// * Interpreter Type
// * Environment name
if (info.version) {
sortNameParts.push(info.version.raw);
}
if (info.architecture) {
sortNameParts.push(getArchitectureDisplayName(info.architecture));
}
if (info.companyDisplayName && info.companyDisplayName.length > 0) {
sortNameParts.push(info.companyDisplayName.trim());
} else {
sortNameParts.push('Python');
}

if (info.envType) {
const name = interpreterHelper.getInterpreterTypeDisplayName(info.envType);
if (name) {
envSuffixParts.push(name);
}
}
if (info.envName && info.envName.length > 0) {
envSuffixParts.push(info.envName);
}

const envSuffix = envSuffixParts.length === 0 ? '' : `(${envSuffixParts.join(': ')})`;
return `${sortNameParts.join(' ')} ${envSuffix}`.trim();
}

function isCondaEnvironment(environment: PythonEnvironment): boolean {
return environment.envType === EnvironmentType.Conda;
}

function isBaseCondaEnvironment(environment: PythonEnvironment): boolean {
return isCondaEnvironment(environment) && (environment.envName === 'base' || environment.envName === 'miniconda');
}

/**
* Compare 2 Python versions in decending order, most recent one comes first.
*/
function comparePythonVersionDescending(a: PythonVersion | undefined, b: PythonVersion | undefined): number {
if (!a) {
return 1;
}

if (!b) {
return -1;
}

if (a.raw === b.raw) {
return 0;
}

if (a.major === b.major) {
if (a.minor === b.minor) {
if (a.patch === b.patch) {
return a.build.join(' ') > b.build.join(' ') ? -1 : 1;
}
return a.patch > b.patch ? -1 : 1;
}
return a.minor > b.minor ? -1 : 1;
}

return a.major > b.major ? -1 : 1;
}

/**
* Compare 2 environment types: return 0 if they are the same, -1 if a comes before b, 1 otherwise.
*/
function compareEnvironmentType(a: PythonEnvironment, b: PythonEnvironment, workspacePath: string): number {
const aHeuristic = getEnvTypeHeuristic(a, workspacePath);
const bHeuristic = getEnvTypeHeuristic(b, workspacePath);

return Math.sign(aHeuristic - bHeuristic);
}

/**
* Return a heuristic value depending on the environment type.
*/
function getEnvTypeHeuristic(environment: PythonEnvironment, workspacePath: string): EnvTypeHeuristic {
const { envType } = environment;

if (workspacePath.length > 0 && environment.envPath && isParentPath(environment.envPath, workspacePath)) {
return EnvTypeHeuristic.Local;
}

switch (envType) {
case EnvironmentType.Venv:
case EnvironmentType.Conda:
case EnvironmentType.VirtualEnv:
case EnvironmentType.VirtualEnvWrapper:
case EnvironmentType.Pipenv:
case EnvironmentType.Poetry:
return EnvTypeHeuristic.Global;
// The default case covers global environments.
// For now this includes: pyenv, Windows Store, Global, System and Unknown environment types.
default:
return EnvTypeHeuristic.GlobalInterpreters;
}
}
40 changes: 3 additions & 37 deletions src/client/interpreter/configuration/interpreterComparer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,54 +4,20 @@
'use strict';

import { inject, injectable } from 'inversify';
import { getArchitectureDisplayName } from '../../common/platform/registry';
import { PythonEnvironment } from '../../pythonEnvironments/info';
import { IInterpreterHelper } from '../contracts';
import { getSortName } from './environmentTypeComparer';
import { IInterpreterComparer } from './types';

@injectable()
export class InterpreterComparer implements IInterpreterComparer {
constructor(@inject(IInterpreterHelper) private readonly interpreterHelper: IInterpreterHelper) {}
public compare(a: PythonEnvironment, b: PythonEnvironment): number {
const nameA = this.getSortName(a);
const nameB = this.getSortName(b);
const nameA = getSortName(a, this.interpreterHelper);
const nameB = getSortName(b, this.interpreterHelper);
if (nameA === nameB) {
return 0;
}
return nameA > nameB ? 1 : -1;
}
private getSortName(info: PythonEnvironment): string {
const sortNameParts: string[] = [];
const envSuffixParts: string[] = [];

// Sort order for interpreters is:
// * Version
// * Architecture
// * Interpreter Type
// * Environment name
if (info.version) {
sortNameParts.push(info.version.raw);
}
if (info.architecture) {
sortNameParts.push(getArchitectureDisplayName(info.architecture));
}
if (info.companyDisplayName && info.companyDisplayName.length > 0) {
sortNameParts.push(info.companyDisplayName.trim());
} else {
sortNameParts.push('Python');
}

if (info.envType) {
const name = this.interpreterHelper.getInterpreterTypeDisplayName(info.envType);
if (name) {
envSuffixParts.push(name);
}
}
if (info.envName && info.envName.length > 0) {
envSuffixParts.push(info.envName);
}

const envSuffix = envSuffixParts.length === 0 ? '' : `(${envSuffixParts.join(': ')})`;
return `${sortNameParts.join(' ')} ${envSuffix}`.trim();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,33 @@

'use strict';

import { inject, injectable } from 'inversify';
import { inject, injectable, named } from 'inversify';
import { Disposable, Uri } from 'vscode';
import { IPathUtils, Resource } from '../../../common/types';
import { EnvironmentSorting } from '../../../common/experiments/groups';
import { IExperimentService, IPathUtils, Resource } from '../../../common/types';
import { PythonEnvironment } from '../../../pythonEnvironments/info';
import { IInterpreterService } from '../../contracts';
import { IInterpreterComparer, IInterpreterQuickPickItem, IInterpreterSelector } from '../types';
import {
IInterpreterComparer,
IInterpreterQuickPickItem,
IInterpreterSelector,
InterpreterComparisonType,
} from '../types';

@injectable()
export class InterpreterSelector implements IInterpreterSelector {
private disposables: Disposable[] = [];

constructor(
@inject(IInterpreterService) private readonly interpreterManager: IInterpreterService,
@inject(IInterpreterComparer) private readonly interpreterComparer: IInterpreterComparer,
@inject(IInterpreterComparer)
@named(InterpreterComparisonType.Default)
private readonly interpreterComparer: IInterpreterComparer,
@inject(IInterpreterComparer)
@named(InterpreterComparisonType.EnvType)
private readonly envTypeComparer: IInterpreterComparer,
@inject(IPathUtils) private readonly pathUtils: IPathUtils,
@inject(IExperimentService) private readonly experimentService: IExperimentService,
) {}

public dispose(): void {
Expand All @@ -29,7 +41,13 @@ export class InterpreterSelector implements IInterpreterSelector {
onSuggestion: true,
ignoreCache,
});
interpreters.sort(this.interpreterComparer.compare.bind(this.interpreterComparer));

if (await this.experimentService.inExperiment(EnvironmentSorting.experiment)) {
interpreters.sort(this.envTypeComparer.compare.bind(this.envTypeComparer));
} else {
interpreters.sort(this.interpreterComparer.compare.bind(this.interpreterComparer));
}

return Promise.all(interpreters.map((item) => this.suggestionToQuickPickItem(item, resource)));
}

Expand Down
5 changes: 5 additions & 0 deletions src/client/interpreter/configuration/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ export interface IFindInterpreterQuickPickItem {
alwaysShow: boolean;
}

export enum InterpreterComparisonType {
Default = 'defaultComparison',
EnvType = 'environmentTypeComparison',
}

export const IInterpreterComparer = Symbol('IInterpreterComparer');
export interface IInterpreterComparer {
compare(a: PythonEnvironment, b: PythonEnvironment): number;
Expand Down
13 changes: 12 additions & 1 deletion src/client/interpreter/serviceRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
IInterpreterAutoSelectionService,
IInterpreterAutoSelectionProxyService,
} from './autoSelection/types';
import { EnvironmentTypeComparer } from './configuration/environmentTypeComparer';
import { InterpreterComparer } from './configuration/interpreterComparer';
import { ResetInterpreterCommand } from './configuration/interpreterSelector/commands/resetInterpreter';
import { SetInterpreterCommand } from './configuration/interpreterSelector/commands/setInterpreter';
Expand All @@ -31,6 +32,7 @@ import { PythonPathUpdaterServiceFactory } from './configuration/pythonPathUpdat
import {
IInterpreterComparer,
IInterpreterSelector,
InterpreterComparisonType,
IPythonPathUpdaterServiceFactory,
IPythonPathUpdaterServiceManager,
} from './configuration/types';
Expand Down Expand Up @@ -91,7 +93,16 @@ export function registerInterpreterTypes(serviceManager: IServiceManager): void
serviceManager.addSingleton<IShebangCodeLensProvider>(IShebangCodeLensProvider, ShebangCodeLensProvider);
serviceManager.addSingleton<IInterpreterHelper>(IInterpreterHelper, InterpreterHelper);

serviceManager.addSingleton<IInterpreterComparer>(IInterpreterComparer, InterpreterComparer);
serviceManager.addSingleton<IInterpreterComparer>(
IInterpreterComparer,
InterpreterComparer,
InterpreterComparisonType.Default,
);
serviceManager.addSingleton<IInterpreterComparer>(
IInterpreterComparer,
EnvironmentTypeComparer,
InterpreterComparisonType.EnvType,
);

serviceManager.addSingleton<IExtensionSingleActivationService>(
IExtensionSingleActivationService,
Expand Down
Loading