-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Make sure to search for the correct python during finding commands #3916
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3916 +/- ##
=======================================
- Coverage 81% 81% -<1%
=======================================
Files 401 401
Lines 18516 18561 +45
Branches 2975 2988 +13
=======================================
+ Hits 14978 14988 +10
- Misses 3535 3570 +35
Partials 3 3
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great @rchiodo, please ping me if my comments need discussing.
package.json
Outdated
"python.dataScience.forceJupyterExactMatch": { | ||
"type": "boolean", | ||
"default": false, | ||
"description": "Force the Python Interactive window to use the python selected and don't search for a close match", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to add and don't search for a close match
? If we force the Python Interactive window to use a specific interpreter, no further information is required here I think?
Also, Python is capitalized.
"description": "Force the Python Interactive window to use the python selected and don't search for a close match", | |
"description": "Force the Python Interactive window to use the Python interpreter selected.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is python not supposed to be capitalized? It's the name of a window.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'and don't search for a close match' is necessary because without this flag it does use the actual python selected first but if that fails it falls back to searching through all other available pythons. I think if it just says Force it to use the selected python, user's will think it doesn't use the selected python at all without this check.
Maybe I invert it and make it
'Search all installed pythons for a Jupyter installation when starting the Python Interactive window' and the default would then be true.
Turn off linting tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
I found this problem while investigating pipenv virtual environments. We were always returning the first other environment that might match a python path. We also required a ipykernel as the 'usable' python when that's not necessary. The 'usable' python should only need to start a notebook.
package-lock.json
has been regenerated by runningnpm install
(if dependencies have changed)