-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
PR: Support activation of Pixi environments #23919
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
Conversation
…kernels.git --branch=spyder_23558 --update --force external-deps/spyder-kernels subrepo: subdir: "external-deps/spyder-kernels" merged: "a8245d991" upstream: origin: "https://github.com/dalthviz/spyder-kernels.git" branch: "spyder_23558" commit: "a8245d991" git-subrepo: version: "0.4.3" origin: "???" commit: "???"
7d28c6d to
9852f81
Compare
--branch=spyder_23558 --update --force external-deps/spyder-kernels subrepo: subdir: "external-deps/spyder-kernels" merged: "eb2ab0afb" upstream: origin: "https://github.com/dalthviz/spyder-kernels.git" branch: "spyder_23558" commit: "eb2ab0afb" git-subrepo: version: "0.4.3" origin: "???" commit: "???"
…ble path to get pixi info
--branch=spyder_23558 --update --force external-deps/spyder-kernels subrepo: subdir: "external-deps/spyder-kernels" merged: "59bd75824" upstream: origin: "https://github.com/dalthviz/spyder-kernels.git" branch: "spyder_23558" commit: "59bd75824" git-subrepo: version: "0.4.3" origin: "???" commit: "???"
--branch=spyder_23558 --update --force external-deps/spyder-kernels subrepo: subdir: "external-deps/spyder-kernels" merged: "edc25c0ea" upstream: origin: "https://github.com/dalthviz/spyder-kernels.git" branch: "spyder_23558" commit: "edc25c0ea" git-subrepo: version: "0.4.3" origin: "???" commit: "???"
--branch=spyder_23558 --update --force external-deps/spyder-kernels subrepo: subdir: "external-deps/spyder-kernels" merged: "d145d69e5" upstream: origin: "https://github.com/dalthviz/spyder-kernels.git" branch: "spyder_23558" commit: "d145d69e5" git-subrepo: version: "0.4.3" origin: "???" commit: "???"
ccordoba12
left a comment
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 @dalthviz for your work on this!
Co-authored-by: Carlos Cordoba <ccordoba12@gmail.com>
…r-kernels.git --branch=master --update --force external-deps/spyder-kernels subrepo: subdir: "external-deps/spyder-kernels" merged: "957e1fde7" upstream: origin: "https://github.com/spyder-ide/spyder-kernels.git" branch: "master" commit: "957e1fde7" git-subrepo: version: "0.4.3" origin: "???" commit: "???"
ccordoba12
left a comment
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.
Last comments for you @dalthviz, then this should be ready.
| elif env_info["env_type"] == PythonEnvType.PyEnv: | ||
| env_type = "Pyenv" | ||
| elif env_info["env_type"] == PythonEnvType.Pixi: | ||
| env_type = "Pixi" |
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've been thinking about this and in my opinion we should show "Conda" for Pixi envs. That's because:
- Pixi envs are basically conda envs, but managed by Pixi (so, not a different kind of env, like Pyenv). In a way, the case is similar to Mamba and Micromamba.
- Pixi is not well known yet, but Conda is widely recognized at this point. So, newbies wouldn't have to wonder what Pixi is.
- This would also make our in-progress support for env management easier for users (sure, we'll use Pixi in the backend but they we'll see a Conda env in the statusbar).
So, please add this validation above like this
if env_info["env_type"] in [PythonEnvType.Conda, PythonEnvType.Pixi]:
env_type = "Conda"unless you think it's not a 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.
I would say that being explicit about the envs being managed by Pixi is better. In fact, I think showing conda would actually make things confusing since I don't think everyone wanting to use a Pixi created env knows about the relationship between Pixi and conda. For example, for someone unware of this relationship and that has a setup with only Pixi, seeing a conda label could make them wonder why Spyder is showing things being labeled as conda when they don't even have conda installed. Also, not doing the distinction between Pixi and conda could give the idea to users that somehow Spyder supports managing Pixi created envs via conda which I think is not correct
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.
Ok, those are good points too. Let's go with Pixi then.
Co-authored-by: Carlos Cordoba <ccordoba12@gmail.com>
ccordoba12
left a comment
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 good to me now, thanks @dalthviz!
Description of Changes
Adds base support to run kernels from Pixi created environments. The command to run the kernel uses the Pixi executable. This also adds logic to show
Pixiwhen a Pixi env is selected (status bar). For more specific implementation details you can see #23558 (comment)A preview:
Issue(s) Resolved
Fixes #23558
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: dalthviz