Skip to content

fix: remove dependencies declared in [project.optional-dependencies] - #11111

Open
kokokoXUY wants to merge 1 commit into
python-poetry:mainfrom
kokokoXUY:fix/remove-optional-dependencies
Open

kokokoXUY wants to merge 1 commit into
python-poetry:mainfrom
kokokoXUY:fix/remove-optional-dependencies

Conversation

@kokokoXUY

@kokokoXUY kokokoXUY commented Sep 29, 2026 •

Copy link
Copy Markdown

Summary

poetry remove <package> never looked at [project.optional-dependencies], so a dependency added with poetry add --optional <extra> <package> (a PEP 621 project) could not be removed:

$ poetry add --optional linting "pylint^3"
$ poetry remove pylint
ValueError: The following packages were not found: pylint

Without --group, remove collected its candidates from [project.dependencies] and [tool.poetry.dependencies] only. add --optional writes the dependency into [project.optional-dependencies].<extra> when the project declares a [project] table, and that table was never searched, so remove concluded the package was not a dependency at all.

Changes

  • src/poetry/console/commands/remove.py: search the [project.optional-dependencies] tables as well, match on the canonicalized PEP 508 name, drop the extras that become empty, and remove the whole optional-dependencies table when its last entry is gone. collections.abc.Mapping is imported at runtime because isinstance is now used in the module.
  • tests/console/commands/test_remove.py: two regression tests — one removes foo from the linting extra while leaving the typechecking extra untouched, the other removes the last entry and asserts the emptied table is gone.

Validation

  • pytest tests/console/commands/test_remove.py -q -n0 → 27 passed.
  • Reverting only src/poetry/console/commands/remove.py makes both new tests fail with the reported ValueError: The following packages were not found: foo, so the tests exercise the bug rather than the implementation.
  • ruff check and ruff format --check are clean on both files.

Fixes #10703.

Disclosure: this change was prepared with an AI coding assistant (DeepSeek) under my direction; I reviewed the diff and the commands above before pushing.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - I've reviewed your changes and they look great!

Sourcery assessment

Needs a human reviewer. If the matching logic is wrong, poetry remove could delete a dependency declaration from pyproject.toml, changing future environment resolution. Reverting the code would not restore declarations already removed, but the bounded configuration change can be repaired by restoring the entries.


Sourcery is free for open source - if you like our reviews please consider sharing them ✨

@kokokoXUY
kokokoXUY force-pushed the fix/remove-optional-dependencies branch from 86d9a04 to da49e91 Compare September 29, 2026 17:18
@dimbleby

Copy link
Copy Markdown
Contributor

Explain why not #10734. See also #10726 (comment)

Without `--group`, `remove` only looked at `[project.dependencies]` and
`[tool.poetry.dependencies]`. A dependency that `add --optional <extra>` wrote
into `[project.optional-dependencies]` was therefore never found, and removing
it failed with:

    ValueError: The following packages were not found: foo

Search the `[project.optional-dependencies]` tables too, drop the extras that
become empty and remove the whole table when its last entry is gone.

Fixes python-poetry#10703.
@kokokoXUY
kokokoXUY force-pushed the fix/remove-optional-dependencies branch from da49e91 to 516e2f8 Compare October 5, 2026 05:40
@kokokoXUY

Copy link
Copy Markdown
Author

Thanks for looking at this, and for the pointer to #10726 — that comment is exactly the design question I hit while writing the patch.

Why not #10734 / #10726

Both fix the same bug (#10703) in a different place than this PR does:

  • fix: handle removal of optional dependencies #10726 (closed) added _remove_optional_packages() covering [project.optional-dependencies] and legacy [tool.poetry.extras].
  • fix: support removing packages from project.optional-dependencies #10734 (open since February, last activity March; its author offered to close it in favour of fix: handle removal of optional dependencies #10726) makes [project.optional-dependencies] behave like a dependency group: synthetic groups named optional:<group> plus a skip_group_update flag threaded through _remove_packages() so the in-memory DependencyGroup is left alone.
  • This PR stays inside the existing "no --group means remove from everywhere" branch of handle() and adds one helper, _remove_packages_from_optional_dependencies(), that edits the TOML document which is written back. No synthetic groups, no extra parameter on the shared _remove_packages() signature, no change to DependencyGroup.remove_dependency() semantics. Packages are matched by canonicalized PEP 508 name (canonicalize_name(Dependency.create_from_pep_508(req).name)), so pylint (>=3,<4) and Pylint match the same package.

If you would rather have this inside #10734, say so and I will close this one and post the patch there — the behaviour matters more than whose PR carries it.

On the semantics you raised in #10726

What this PR implements: poetry remove <name> removes the name from every place it is declared as a dependency — [project.dependencies], [dependency-groups], [tool.poetry.*] groups, and now every extra in [project.optional-dependencies]. Extras that become empty are dropped, and the table goes away when no extras remain.

You are right that two cases stay ambiguous, because remove has no syntax for naming an extra:

  1. remove from one extra but not another;
  2. remove from [project.dependencies] but keep it in the extras that reference it.

I did not invent syntax for them. The options as I see them:

  • (a) keep "remove everywhere" and document it, which is what this PR does;
  • (b) make the target explicit, e.g. poetry remove --extra linting pylint, and keep removing from all extras only when no selector is given;
  • (c) don't support extras in remove at all and tell users to edit pyproject.toml, as you suggested.

I picked (a) because with no --group the command already means "remove from every group", so leaving optional dependencies behind is the part that looks inconsistent. I am happy to turn this PR into (b) — it is a small delta on top of this diff — or to withdraw it for (c) if that is the preference. The one thing I would argue against is making extra-scoped removal implicit.

Two smaller notes:

  • [tool.poetry.extras] is untouched here: those entries are lists of names of dependencies declared with optional = true in [tool.poetry.dependencies], so remove already deletes them at their only declaration site. It can leave a stale name in the extras list; I can clean that up in this PR or in a follow-up, whichever you prefer.
  • The branch is rebased onto main (3ea141d) as of now, and tests/console/commands/test_remove.py passes (27 passed).

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cannot remove optional dependency

2 participants