Skip to content

PR: Fix conda script selection for custom interpreter env activation on Windows #20575

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

Merged

Conversation

dalthviz
Copy link
Member

@dalthviz dalthviz commented Feb 21, 2023

Description of Changes

  • Wrote at least one-line docstrings (for any new functions)
  • Added unit test(s) covering the changes (if testable)
  • Included a screenshot or animation (if affecting the UI, see Licecap)

Seems like the custom interpreters env activation was not working (at least on Windows). When selecting a custom interpreter running latest master (8dc6d18) on Windows I got the following message:

imagen

With the changes here I was able to not show the error message, but seems like the console gets stuck loading the kernel 🤔 :

imagen

Not sure if there is something else that I'm missing to setup on my side to prevent this but creating this PR just in case.

Also the validation for the spyder-kernels version was missing (as well as updating the min max version of spyder-kernels supported constants). Using an interpreter from an env with the incorrect spyder-kernels version shows now this:

image

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

@ccordoba12
Copy link
Member

@dalthviz, do you have master of Spyder-kernels installed in the custom interpreter you're trying to use? That's not a requirement because now Spyder should be able to detect an older version and show a message about it. But it'd help to check that we're actually able to start consoles for custom interpreters.

@impact27, could you check what's happening on Windows with custom interpreters?

@dalthviz
Copy link
Member Author

dalthviz commented Feb 21, 2023

do you have master of Spyder-kernels installed in the custom interpreter you're trying to use?

I had spyder-kernels 2.4.2 installed in the env. By installing spyder-kernels 3.0.0.dev and the changes in this PR the console was able to load 👍 so I guess there is something happening with the logic to show the missing spyder-kernels installation/correct version message in the master branch?

Also, trying to debug this while I still had spyder-kernels 2.4.2 installed, seems like the client.kernel_handler.connection_state was in an error state but the sig_stderr was not being triggered (?). Maybe that is the cause of the console just getting stuck with the loading page?

Edit: Basically the problem is that the spyder-kernels validation on master only validates if spyder-kernels is installed but not its version (>=3.0.0.dev0;<4.0.0)

@dalthviz dalthviz added this to the v6.0alpha1 milestone Feb 21, 2023
@dalthviz dalthviz changed the title [WIP] PR: Fix conda script selection for custom interpreter env activation on Windows PR: Fix conda script selection for custom interpreter env activation on Windows Feb 21, 2023
@dalthviz dalthviz requested a review from ccordoba12 February 21, 2023 23:44
Copy link
Member

@ccordoba12 ccordoba12 left a 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 help to solve this problem! Small suggestion for you, the rest looks good to me.

Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

Thanks @dalthviz!

@ccordoba12 ccordoba12 merged commit 7512977 into spyder-ide:master Feb 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants