improve python management for free-threaded python #10606
Merged
+286
−63
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's GuideThis pull request integrates free-threaded Python support throughout Poetry's environment management by introducing a dedicated Sequence diagram for Python installation with free-threaded supportsequenceDiagram
actor User
participant CLI as "Poetry CLI"
participant InstallCmd as "PythonInstallCommand"
participant PathProvider as "PoetryPythonPathProvider"
participant Installer as "PythonInstaller"
User->>CLI: poetry python install 3.12.0t
CLI->>InstallCmd: handle(request="3.12.0t")
InstallCmd->>InstallCmd: Detect trailing "t", set free_threaded=True
InstallCmd->>PathProvider: installation_dir(version, implementation, free_threaded)
InstallCmd->>Installer: Check if exists (with free_threaded)
alt Already installed
InstallCmd->>CLI: Error: Python version already installed
else Not installed
InstallCmd->>Installer: Download and install
Installer->>PathProvider: installation_dir(..., free_threaded)
Installer->>CLI: Success message
end
Sequence diagram for Python removal with free-threaded supportsequenceDiagram
actor User
participant CLI as "Poetry CLI"
participant RemoveCmd as "PythonRemoveCommand"
participant PathProvider as "PoetryPythonPathProvider"
User->>CLI: poetry python remove 3.12.0t
CLI->>RemoveCmd: handle(request="3.12.0t")
RemoveCmd->>RemoveCmd: Detect trailing "t", set free_threaded=True
RemoveCmd->>PathProvider: installation_dir(version, implementation, free_threaded)
RemoveCmd->>RemoveCmd: Remove installation directory
RemoveCmd->>CLI: Success/Error message
ER diagram for Python installation directory namingerDiagram
PYTHON_INSTALLATION_DIR ||--o| PYTHON_VERSION : contains
PYTHON_VERSION {
string implementation
string version
boolean free_threaded
}
PYTHON_INSTALLATION_DIR {
string path
}
Class diagram for updated PythonInfo and related classesclassDiagram
class PythonInfo {
+int major
+int minor
+int patch
+str implementation
+bool free_threaded
+Path|None executable
}
class Python {
+property major: int
+property minor: int
+property patch: int
+property implementation: str
+property free_threaded: bool
+property executable: Path
}
PythonInfo <|-- Python
Class diagram for updated PoetryPythonPathProvider methodsclassDiagram
class PoetryPythonPathProvider {
+installation_dir(version: Version, implementation: str, free_threaded: bool): Path
+installation_bin_paths(version: Version, implementation: str, free_threaded: bool = False): list[Path]
}
Flow diagram for CLI option handling for free-threaded Pythonflowchart TD
A["User enters CLI command"] --> B{"Command type?"}
B -->|install| C["Check for trailing 't' or --free-threaded"]
B -->|remove| C
B -->|list| C
C --> D["Set free_threaded flag"]
D --> E["Pass flag to logic and path provider"]
E --> F["Perform action (install/list/remove)"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Extract the trailing-’t’ suffix parsing logic from both the install and remove commands into a shared helper to DRY up the code and keep behavior consistent.
- Add validation to error out or warn when both a trailing ‘t’ in the request and the --free-threaded flag are provided so users aren’t sending redundant or conflicting options.
- Consider pinning an upper bound for the pbs-installer dependency (e.g. <2026.0) even when using calver to guard against future breaking changes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Extract the trailing-’t’ suffix parsing logic from both the install and remove commands into a shared helper to DRY up the code and keep behavior consistent.
- Add validation to error out or warn when both a trailing ‘t’ in the request and the --free-threaded flag are provided so users aren’t sending redundant or conflicting options.
- Consider pinning an upper bound for the pbs-installer dependency (e.g. <2026.0) even when using calver to guard against future breaking changes.
## Individual Comments
### Comment 1
<location> `src/poetry/console/commands/python/remove.py:100-110` </location>
<code_context>
def handle(self) -> int:
implementation = self.option("implementation").lower()
free_threaded = self.option("free-threaded")
result = 0
for request in self.argument("python"):
result += self.remove_python_installation(
request, implementation, free_threaded, self.io
)
return result
</code_context>
<issue_to_address>
**issue (code-quality):** We've found these issues:
- Convert for loop into call to sum() ([`sum-comprehension`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/sum-comprehension/))
- Inline variable that is immediately returned ([`inline-immediately-returned-variable`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/inline-immediately-returned-variable/))
</issue_to_address>
### Comment 2
<location> `tests/console/commands/python/test_python_remove.py:110-114` </location>
<code_context>
@pytest.mark.parametrize("free_threaded", [True, False])
@pytest.mark.parametrize("option", [True, False])
def test_remove_version_free_threaded(
tester: CommandTester,
config: Config,
mocked_poetry_managed_python_register: MockedPoetryPythonRegister,
free_threaded: bool,
option: bool,
) -> None:
standard_path = mocked_poetry_managed_python_register("3.14.0", "cpython")
free_threaded_path = mocked_poetry_managed_python_register(
"3.14.0", "cpython", free_threaded=True
)
args = "3.14.0"
if free_threaded:
if option:
args += " --free-threaded"
else:
args += "t"
assert tester.execute(args) == 0, tester.io.fetch_error()
details = "cpython"
if free_threaded:
details += ", free-threaded"
assert (
tester.io.fetch_output()
== f"Removing installation 3.14.0 ({details}) ... Done\n"
)
if free_threaded:
assert not free_threaded_path.exists()
assert standard_path.exists()
else:
assert not standard_path.exists()
assert free_threaded_path.exists()
</code_context>
<issue_to_address>
**suggestion (code-quality):** We've found these issues:
- Swap positions of nested conditionals ([`swap-nested-ifs`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/swap-nested-ifs/))
- Hoist nested repeated code outside conditional statements ([`hoist-similar-statement-from-if`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/hoist-similar-statement-from-if/))
- Replace if statement with if expression ([`assign-if-exp`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/assign-if-exp/))
```suggestion
args += " --free-threaded" if option else "t"
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
radoering
force-pushed
the
python-management-free-threaded
branch
from
073abd1 to
4a48a8a
Compare
* bump required pbs-installer version so that free-threaded Python uses a different directory for installation (cf frostming/pbs-installer#11) * do not use an upper bound for pbs-installer because it uses calver * do not complain that a python version is already installed when trying to install a free-threaded python with the same version * list free-threaded python versions with a trailing "t" * allow to request a free-threaded version with a trailing "t" instead of the command option * add support for removing free-threaded python versions * fix removing Python versions with patch version "0"
radoering
force-pushed
the
python-management-free-threaded
branch
from
4a48a8a to
a79a19f
Compare
|
Deploy preview for website ready! ✅ Preview Built with commit a79a19f. |
Open
|
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. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
version_dirfor free-threaded Python and standard Python frostming/pbs-installer#11)Pull Request Check List
Summary by Sourcery
Add comprehensive support for free-threaded Python versions by extending version tracking, commands, and providers, updating documentation, tests, and bumping the pbs-installer dependency.
New Features:
Bug Fixes:
Enhancements:
Build:
Documentation:
Tests: