Sitelet https://github.com/microsoft/vscode-python-devicesimulator/pull/338
Skip to content
This repository was archived by the owner on Dec 23, 2021. It is now read-only.

Properly resolve Python path if not set - #338

Merged
Kevin Nguyen (xnkevinnguyen) merged 6 commits into
devfrom
users/t-anmah/unix-nopythonpath-fix
Apr 17, 2020
Merged

Properly resolve Python path if not set#338
Kevin Nguyen (xnkevinnguyen) merged 6 commits into
devfrom
users/t-anmah/unix-nopythonpath-fix

Conversation

@andreamah

@andreamah Andrea Mah (andreamah) commented Apr 17, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description:

In response to #163

Fixes to dependency process:

  • When no interpreter is set, it used to ask you to choose an interpreter. Now, it will look at your system version and see if it's sufficient. If there is no system version under "python" or if that version is too low, it will try using "python3". The follow-up use of "python3" helps UNIX systems get started, since "python" usually points to Python 2.7.
  • Setup now properly checks version number & dependency checker works on more versions of Python.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Limitations:

People who were using a version older that Python 3.7 will need to switch. Previously, they weren't properly told to switch off.

Testing:

Tested on VS Code Online, which runs on Linux. Also tested on local windows. In both, tested the following cases:

  • no defined pythonPath in settings
  • python.pythonPath defined in settings (relative path, absolute path, and invalid path)
  • python version too low
  • no pip

vandyliu
Vandy Liu (vandyliu) reviewed Apr 17, 2020
View reviewed changes
Comment thread src/service/setupService.ts
xnkevinnguyen
Kevin Nguyen (xnkevinnguyen) suggested changes Apr 17, 2020
View reviewed changes

@xnkevinnguyen Kevin Nguyen (xnkevinnguyen) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Amazing work! Just had minor comments

Comment thread src/service/setupService.ts Outdated
Comment thread src/service/setupService.ts Outdated
@vandyliu

Vandy Liu (vandyliu) commented Apr 17, 2020

Copy link
Copy Markdown
Contributor

Running python 2.7 as selected interpreter, I get
Code_33cyfTPDcI
Is the red one supposed to appear?

xnkevinnguyen
Kevin Nguyen (xnkevinnguyen) approved these changes Apr 17, 2020
View reviewed changes

@xnkevinnguyen Kevin Nguyen (xnkevinnguyen) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Amazing as always Andrea! I have tested without env path to python on my global, it works for me and uses a python version 3.7 and up to create venv

@xnkevinnguyen

Kevin Nguyen (xnkevinnguyen) commented Apr 17, 2020

Copy link
Copy Markdown
Contributor

Tested on MAC OS

nasadigital
David Serafimov (nasadigital) reviewed Apr 17, 2020
View reviewed changes
Comment thread src/service/setupService.ts
Comment thread src/service/setupService.ts
@andreamah

Andrea Mah (andreamah) commented Apr 17, 2020

Copy link
Copy Markdown
Contributor Author

Running python 2.7 as selected interpreter, I get
Code_33cyfTPDcI
Is the red one supposed to appear?

I think I fixed it. Can you pull and try again?

@vandyliu

Vandy Liu (vandyliu) commented Apr 17, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

FIXED AWESOME STUFF ANDREA :D

vandyliu
Vandy Liu (vandyliu) approved these changes Apr 17, 2020
View reviewed changes

@vandyliu Vandy Liu (vandyliu) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

amazing as always AAA

@andreamah Andrea Mah (andreamah) mentioned this pull request Apr 18, 2020
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@abmahdy Ahmed Mahdy (abmahdy) Awaiting requested review from abmahdy abmahdy is a code owner
@adclements Andrew Clements (adclements) Awaiting requested review from adclements adclements is a code owner
@iennae Jennifer Davis (iennae) Awaiting requested review from iennae
@isadorasophia Isa (isadorasophia) Awaiting requested review from isadorasophia
@markAtMicrosoft Mark Schramm (markAtMicrosoft) Awaiting requested review from markAtMicrosoft
@sagarmanchanda Sagar Manchanda (sagarmanchanda) Awaiting requested review from sagarmanchanda sagarmanchanda is a code owner
@smmatte smmatte Awaiting requested review from smmatte smmatte is a code owner
3 more reviewers
@nasadigital David Serafimov (nasadigital) nasadigital approved these changes
@xnkevinnguyen Kevin Nguyen (xnkevinnguyen) xnkevinnguyen approved these changes
@vandyliu Vandy Liu (vandyliu) vandyliu approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@andreamah @vandyliu @xnkevinnguyen @nasadigital