-
Notifications
You must be signed in to change notification settings - Fork 2.4k
fix: take virtualenvs.prefer-active-python into account on EnvManager.get() #6986
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
fix: take virtualenvs.prefer-active-python into account on EnvManager.get() #6986
Conversation
efab161 to
a42859d
Compare
src/poetry/utils/env.py
Outdated
| def _get_python_version(self) -> list[int]: | ||
| version_info = list(sys.version_info[:3]) | ||
|
|
||
| if self._poetry.config.get("virtualenvs.prefer-active-python"): | ||
| executable = self._detect_active_python() | ||
|
|
||
| if executable: | ||
| python_patch = decode( | ||
| subprocess.check_output( | ||
| list_to_shell_command( | ||
| [executable, "-c", GET_PYTHON_VERSION_ONELINER] | ||
| ), | ||
| shell=True, | ||
| ).strip() | ||
| ) | ||
|
|
||
| version_info = [int(v) for v in python_patch.split(".")[:3]] | ||
|
|
||
| return version_info |
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.
| def _get_python_version(self) -> list[int]: | |
| version_info = list(sys.version_info[:3]) | |
| if self._poetry.config.get("virtualenvs.prefer-active-python"): | |
| executable = self._detect_active_python() | |
| if executable: | |
| python_patch = decode( | |
| subprocess.check_output( | |
| list_to_shell_command( | |
| [executable, "-c", GET_PYTHON_VERSION_ONELINER] | |
| ), | |
| shell=True, | |
| ).strip() | |
| ) | |
| version_info = [int(v) for v in python_patch.split(".")[:3]] | |
| return version_info | |
| def _get_python_version(self) -> tuple[int, int, int]: | |
| if self._poetry.config.get("virtualenvs.prefer-active-python"): | |
| executable = self._detect_active_python() | |
| if executable: | |
| python_patch = decode( | |
| subprocess.check_output( | |
| list_to_shell_command( | |
| [executable, "-c", GET_PYTHON_VERSION_ONELINER] | |
| ), | |
| shell=True, | |
| ).strip() | |
| ) | |
| return tuple(int(v) for v in python_patch.split(".")[:3]) | |
| return sys.version_info[:3] |
Using a tuple would more clearly indicate that we expect 3 items.
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 implemented a version that returns a tuple. mypy was a little bit in the way here ...
a42859d to
28ea70b
Compare
28ea70b to
3be157c
Compare
neersighted
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.
I'd rather we had one source of truth for this/it feels like the abstraction is leaking even more, but this is a fix for a real issue and it doesn't make the amount of work needed to refactor this file greater.
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Until now
virtualenvs.prefer-active-pythonwasn't respected by theEnvManagerwhen it tries to get the venv. This is fixed by the PR.I took the chance to refactor
EnvManagerin that way, thatIOis now set during initializing a new object. Thus it is not necessary to pass it in into the several methods.Pull Request Check List
Resolves: #6893
Resolves: #5947