Skip to content

AI-generated possible solution for #10703 (A) #10924

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.

Sign up for GitHub

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

Open
hugnia wants to merge 1 commit into
base: main
Choose a base branch
from
+33 −8
Open

AI-generated possible solution for #10703 (A) #10924

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 33 additions & 8 deletions src/poetry/console/commands/remove.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,11 @@ def handle(self) -> int:
for group_name, group_section in poetry_groups_content.items()
if group_name not in groups_content and group_name != MAIN_GROUP
)
optional_dependencies = project_content.get("optional-dependencies", {})
group_sections.extend(
(group_name, dependencies, [])
Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot May 26, 2026

Choose a reason for hiding this comment

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

issue (bug_risk): Use an empty dict instead of a list for the poetry_section placeholder when adding optional dependencies.

group_sections.extend passes its third element as poetry_section into _remove_packages, where this parameter is treated as a mapping. Using [] here changes its type compared to other call sites and could break membership checks or updates. Use {} instead to keep the type consistent and avoid subtle bugs.

for group_name, dependencies in optional_dependencies.items()
)

for group_name, standard_section, poetry_section in group_sections:
removed |= self._remove_packages(
Expand All @@ -111,11 +116,15 @@ def handle(self) -> int:
del poetry_content["group"][group_name]
if not standard_section and group_name in groups_content:
del groups_content[group_name]
if (
group_name not in groups_content
and group_name not in poetry_groups_content
):
self._remove_references_to_group(group_name, content)
if not standard_section and group_name in optional_dependencies:
del optional_dependencies[group_name]

if "group" in poetry_content and not poetry_content["group"]:
del poetry_content["group"]
if "dependency-groups" in content and not content["dependency-groups"]:
del content["dependency-groups"]
if "optional-dependencies" in project_content and not project_content["optional-dependencies"]:
del project_content["optional-dependencies"]

elif group == "dev" and "dev-dependencies" in poetry_content:
# We need to account for the old `dev-dependencies` section
Expand Down Expand Up @@ -152,7 +161,19 @@ def handle(self) -> int:
)
if not groups_content[group]:
del groups_content[group]
if group not in groups_content and group not in poetry_groups_content:
optional_dependencies = project_content.get("optional-dependencies", {})
if group in optional_dependencies:
removed.update(
self._remove_packages(
packages=packages,
standard_section=optional_dependencies[group],
poetry_section={},
group_name=group,
)
)
if not optional_dependencies[group]:
del optional_dependencies[group]
if group not in groups_content and group not in poetry_groups_content and group not in optional_dependencies:
self._remove_references_to_group(group, content)

if "group" in poetry_content and not poetry_content["group"]:
Expand Down Expand Up @@ -192,7 +213,10 @@ def _remove_packages(
group_name: str,
) -> set[str]:
removed = set()
group = self.poetry.package.dependency_group(group_name)
try:
group = self.poetry.package.dependency_group(group_name)
except Exception:
Comment on lines +216 to +218
Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot May 26, 2026

Choose a reason for hiding this comment

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

suggestion (bug_risk): Catching Exception around dependency_group is broad and may hide real errors.

This blanket except Exception will also hide programming errors (e.g. TypeError, AttributeError), making failures harder to detect. If you only want to ignore missing groups, please catch the specific exception raised when a group is absent (e.g. a Poetry-specific error or KeyError) and let other exceptions bubble up. You could also return early from _remove_packages when the group is missing instead of using group = None and checking later.

Suggested implementation:

        removed = set()
        try:
            group = self.poetry.package.dependency_group(group_name)
        except (KeyError, ValueError):
            # The requested dependency group does not exist; nothing to remove.
            return removed

If the rest of _remove_packages currently relies on group possibly being None (e.g. if group is None: ...), those branches can now be removed because the early return removed handles the “group missing” case. Adjust any such checks accordingly.

group = None

for package in packages:
normalized_name = canonicalize_name(package)
Expand All @@ -208,7 +232,8 @@ def _remove_packages(
removed.add(package)

for package in removed:
group.remove_dependency(package)
if group:
group.remove_dependency(package)

return removed

Expand Down
Loading