improve python management for free-threaded python#10606
improve python management for free-threaded python#10606radoering merged 1 commit intopython-poetry:mainfrom
Conversation
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
|
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.
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"
4a48a8a to
a79a19f
Compare
|
Deploy preview for website ready! ✅ Preview Built with commit a79a19f. |
|
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. |
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: