Skip to content

Commit

Permalink
Sort available environments by assumed usefulness (#16559)
Browse files Browse the repository at this point in the history
* New comparison logic

* Add experiment group

* Register and call it

* Add service registry tests

* Add interpreter selector unit tests

* Add comparison unit tests

* Add intepreter selector test

* News file

* Adjust comments

* Reuse getSortName

* Add experiment to package.json

* Check if environment is local before anything else

* Update comment on comparison guidelines

* Remove 'unknown' exception
  • Loading branch information
kimadeline authored Jul 8, 2021
1 parent c60d03b commit a0f0337
Show file tree
Hide file tree
Showing 11 changed files with 626 additions and 54 deletions.
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',
}
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

0 comments on commit a0f0337

Please sign in to comment.