-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
PR: Show conda and pyenv environments in Python interpreter (Preferences) #13950
Conversation
/show binder |
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.
Thanks @steff456! This looks pretty good!
I only left than some minor suggestions and comments for you.
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.
Thanks @steff456 for your hard work on this one! I think it's almost ready.
spyder/utils/misc.py
Outdated
def get_list_pyenv_envs(): | ||
"""Return the list of all pyenv envs found in the system.""" | ||
try: | ||
out, err = subprocess.Popen( | ||
['pyenv', 'versions', '--bare', '--skip-aliases'], |
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.
@steff456 , this will not work for Spyder.app for the same reason as above in the conda case. To my knowledge, on macOS pyenv will only ever be installed at /usr/local/bin
def get_list_pyenv_envs(): | |
"""Return the list of all pyenv envs found in the system.""" | |
try: | |
out, err = subprocess.Popen( | |
['pyenv', 'versions', '--bare', '--skip-aliases'], | |
def get_list_pyenv_envs(): | |
"""Return the list of all pyenv envs found in the system.""" | |
pyenv = 'pyenv' | |
if running_in_mac_app(): | |
old_path = os.environ['PATH'] | |
os.environ['PATH'] = os.pathsep.join([old_path, '/usr/local/bin']) | |
pyenv_path = find_program('pyenv') | |
os.environ['PATH'] = old_path # restore PATH | |
if pyenv_path: | |
pyenv = pyenv_path | |
try: | |
out, err = subprocess.Popen( | |
[pyenv, 'versions', '--bare', '--skip-aliases'], | |
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.
@steff456 @ccordoba12, I wonder if we could simplify this by instead enhancing is_program_installed
(called by find_program
). What if is_program_installed
checked that certain paths were already in PATH
and temporarily added them if not present, based on platform. This could do 2 things
- Other developers could use
find_program
without having to consider the macOS application case. - Even outside of the macOS application, when searching for a program on the system, we don't have to assume that user profiles have been sourced (e.g. ~/.bash_Profile, etc)
What do you think?
spyder.utils.programs.py
def is_program_installed(basename):
"""
Return program absolute path if installed in PATH.
Otherwise, return None
On macOS systems, a .app is considered installed if
it exists.
"""
if (sys.platform == 'darwin' and basename.endswith('.app') and
osp.exists(basename)):
return basename
home = get_home_dir()
req_paths = []
if sys.platform == 'darwin':
req_paths.extend([
osp.join('/usr', 'local', 'bin'),
osp.join(home, 'opt', 'anaconda3', 'condabin'),
osp.join(home, 'opt', 'miniconda3', 'condabin'),
osp.join('/opt', 'anaconda3', 'condabin'),
osp.join('/opt', 'miniconda3', 'condabin')
])
elif sys.platform.startswith('linux'):
pass # Set required linux paths
elif os.name == 'nt':
pass # Set required Windows paths
old_path = os.environ['PATH']
for path in req_paths:
if path not in old_path:
os.environ['PATH'] += os.pathsep + path
abspath = None
for path in os.environ["PATH"].split(os.pathsep):
abspath = osp.join(path, basename)
if osp.isfile(abspath):
break
os.environ['PATH'] = old_path
return abspath
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 think it is a great idea! In this way we can guarantee this type of tooling to work inside the macOS application. @ccordoba12 do you think we need to add this in a different PR or in this one?
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 think we should leave this for another PR. But I agree it's a really good idea.
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.
Also, it'd be great if you could give us a hand with that @mrclary.
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.
Thanks @steff456 for all your hard work on this one! Great improvement for 4.2!
Description of Changes
Add two miscellaneous methods that are able to detect conda and pyenv environments. This is the first PR for enhancing Spyder's environment handling.
Issue(s) Resolved
Fixes #13903
Fixes #12200
Affirmation
By submitting this Pull Request or typing my (user)name below,
I affirm the Developer Certificate of Origin
with respect to all commits and content included in this PR,
and understand I am releasing the same under Spyder's MIT (Expat) license.
I certify the above statement is true and correct: Steff456