Ability to install modules using pip or conda #380

DonJayamanne · 2017-12-09T01:09:37Z

Add support for pip and/or conda when installing modules
Fix display versions for environments (fixes Incorrect display name for conda environment #378)
Refactor installer to use the pip/conda module installer for installation of modules
Performance improvement of detecting virtual env and setting it (fixes Improve performance of automatic environment detection #372)
Code refactor to ensure interpreter locators use the new DI framework
Fixes Installing pylint fails if pip module is not installed #266

Archive of 0.7.0

* 'master' of https://github.com/Microsoft/vscode-python: Fixes #56 list all environments (#219) Fixes #57 Disable activation on debugging (#220) Fixes #26 Do not run linters when linters are disabled (#222)

* upstream/master: Fix typo in README.md (#252) Disable linter without workspaces (#241)

* upstream/master: Ensure python path is not set if already set in user settings (#369) Use 'an' rather than 'a' before vowel words (#373)

* upstream/master: Use new environment variable parser (#362)

brettcannon · 2017-12-11T18:39:19Z

src/client/common/installer/moduleInstaller.ts

+        const executionInfo = await this.getExecutionInfo(name, resource);
+        const terminalService = this.serviceContainer.get<ITerminalService>(ITerminalService);
+
+        if (executionInfo.moduleName) {


Why does having a module name decide to use pip versus something else? And won't the concrete implementations like PipInstaller handle the actual installation versus this default use of pip?

If its a module, then we have a separate set of installers. I wanted to add that separation early on so we could create separate installers for python modules vs stand alone tools such as ctags or the like.

brettcannon · 2017-12-11T18:43:43Z

src/client/common/platform/types.ts

+    x86 = 2,
+    x64 = 3
+}
+export enum Hive {


What is Hive?

will rename to RegistryHive

brettcannon · 2017-12-11T18:47:21Z

src/client/common/terminal/service.ts

+        this.terminal.show(false);
+
+        // Sometimes the terminal takes some time to start up before it can start accepting input.
+        // However if we have already sent text to the terminal, then no need to wait


Missing a period.

brettcannon · 2017-12-11T19:02:43Z

src/client/linters/errorHandlers/main.ts

 import { IServiceContainer } from '../../ioc/types';
 import { IErrorHandler, ILinterHelper } from '../types';
 import { BaseErrorHandler } from './baseErrorHandler';
 import { ModuleNotInstalledErrorHandler } from './notInstalled';
 import { StandardErrorHandler } from './standard';

 export class ErrorHandler implements IErrorHandler {
-    private handler: BaseErrorHandler;
+    private handlder: BaseErrorHandler;


You made your favourite spelling mistake again. 😉

brettcannon · 2017-12-11T19:03:27Z

src/client/linters/errorHandlers/notInstalled.ts

@@ -20,7 +19,7 @@ export class ModuleNotInstalledErrorHandler extends BaseErrorHandler {

        const pythonExecutionService = await this.serviceContainer.get<IPythonExecutionFactory>(IPythonExecutionFactory).create(resource);
        const isModuleInstalled = await pythonExecutionService.isModuleInstalled(execInfo.moduleName!);
-        if (!isModuleInstalled) {
+        if (isModuleInstalled) {


Just want to double-check this was a bug previously that's being fixed.

Yes, but not reported. I identified it while testing this PR.

brettcannon · 2017-12-11T19:04:31Z

src/client/linters/types.ts

@@ -3,8 +3,7 @@

 import { Uri } from 'vscode';
 import { ILintingSettings } from '../common/configSettings';
-import { Product } from '../common/installer';
-import { ExecutionInfo } from '../common/types';
+import { ExecutionInfo, Product } from '../common/types';


Why are all of these modules importing Product but not using it?

They are being used. The change made here is to move Product enum from installer into common area.

brettcannon · 2017-12-11T19:07:00Z

src/client/unittests/common/testUtils.ts

@@ -47,17 +48,39 @@ export class TestsHelper implements ITestsHelper {
    constructor( @inject(ITestVisitor) @named('TestFlatteningVisitor') private flatteningVisitor: TestFlatteningVisitor) { }
    public parseProviderName(product: UnitTestProduct): TestProvider {
        switch (product) {
-            case Product.nosetest: {
-                return 'nosetest';
+            case Product.nosetest: return 'nosetest';


Don't we already have this mapping somewhere else? I know this is like the second mapping we have for names.

Yes, that's in the installers. to translate the products to corresponding python modules. But this is used as a unique identifier within the extension which maps to names used in package.json and other settings.

For instance, the id of nose tests is nosetest, that's whats used in package.json (settings). When installing this, the product nosetest is translated to the module name nose and when running the tests, the module is translated to the module nosetest.

…e/pythonVSCode into CondaModuleInstaller

DonJayamanne · 2017-12-11T20:56:25Z

@brettcannon ready for review

brettcannon · 2017-12-11T21:03:27Z

gulpfile.js

@@ -221,8 +221,8 @@ const hygiene = (options) => {
        .pipe(filter(tslintFilter));

    if (!options.skipFormatCheck) {
-        result = result
-            .pipe(formatting);
+        // result = result


Left in commented-out code.

octref and others added 30 commits November 3, 2017 13:11

Fix microsoft/vscode#37627 (#1368)

ecc1ca9

Version 0.7.0 of extension (#1381)

7c5778c

Archive of 0.7.0

Update README.md

9d1bf82

Update README.md

ffba179

sync fork with upstream

905c713

fix readme

acc2109

run without debugging

d32a546

send terminated event when process ends and fix linters

3f14d0a

gracefully handle program termination

f37803f

use vscode infrastructure to launch code in terminals

d69940e

merged upstream

d392e8b

merged

11c9514

ensure to check if launchArgs is not null

38befc0

ensure to check if launchArgs is not null

bea91fc

ensure to check if launchArgs is not null

0bb9054

fix code review comments

a105a20

fixed code review comments

8b2fd7c

Merge branch 'master' into RunWithoutDebug

5a7d21e

Merge branch 'master' into RunWithoutDebug

acd90c5

fixes #250, do not copy all env variables

09b3359

bug fix, launch using vscode and then fallback

9616eef

fix linter issue

f9fe2a8

platform process services

4963632

tests for decoder

edbe63e

tests for process service

47acd08

test to check source

284e221

tests for process utils

b27496b

Merge remote-tracking branch 'upstream/master'

92f775f

* upstream/master: Fix typo in README.md (#252) Disable linter without workspaces (#241)

fix imports

23fe3da

DonJayamanne added 15 commits December 7, 2017 14:48

merge master

0a0fbb4

add conda module installer

951ce86

Merge remote-tracking branch 'upstream/master'

0df7f16

* upstream/master: Ensure python path is not set if already set in user settings (#369) Use 'an' rather than 'a' before vowel words (#373)

conda installer with necessary args

1e298d1

perf improvement

a85724a

better ex handling

7b30809

check conda versions

b8b24f2

fixes #378

ef33c16

fix errors in conda installer

76e8fba

Merge remote-tracking branch 'upstream/master'

3ccc881

* upstream/master: Use new environment variable parser (#362)

merge master

cd328df

fixed errors in merge

2d859c9

fixed unit tests

d18575e

Fixed tests

c1ab174

remove unwanted test

bdf9ebd

brettcannon suggested changes Dec 11, 2017

View reviewed changes

DonJayamanne added 6 commits December 11, 2017 11:48

fix review comments, disable format check (not working on win)

a5de30d

Merge branch 'master' into CondaModuleInstaller

31b3dd7

Merge branch 'master' into CondaModuleInstaller

4ae4cd7

fixed typo

e3ac25d

Merge branch 'CondaModuleInstaller' of https://github.com/DonJayamann…

33f4232

…e/pythonVSCode into CondaModuleInstaller

Fix unit test (send unicode output as unicode string)

39f77bc

DonJayamanne mentioned this pull request Dec 11, 2017

Fix interpreter display #391

Merged

reverted change to test oopsy

89373bb

added missing period

b5a3bae

brettcannon approved these changes Dec 11, 2017

View reviewed changes

DonJayamanne merged commit b0fb5c0 into microsoft:master Dec 11, 2017

DonJayamanne deleted the CondaModuleInstaller branch December 12, 2017 21:21

lock bot locked as resolved and limited conversation to collaborators Jul 31, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ability to install modules using pip or conda #380

Ability to install modules using pip or conda #380

DonJayamanne commented Dec 9, 2017 •

edited

Loading

brettcannon Dec 11, 2017

DonJayamanne Dec 11, 2017

brettcannon Dec 11, 2017

DonJayamanne Dec 11, 2017

brettcannon Dec 11, 2017

brettcannon Dec 11, 2017

brettcannon Dec 11, 2017

DonJayamanne Dec 11, 2017

brettcannon Dec 11, 2017

DonJayamanne Dec 11, 2017

brettcannon Dec 11, 2017

DonJayamanne Dec 11, 2017

DonJayamanne commented Dec 11, 2017

brettcannon Dec 11, 2017

Ability to install modules using pip or conda #380

Ability to install modules using pip or conda #380

Conversation

DonJayamanne commented Dec 9, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DonJayamanne commented Dec 11, 2017

Choose a reason for hiding this comment

DonJayamanne commented Dec 9, 2017 •

edited

Loading