-
Notifications
You must be signed in to change notification settings - Fork 22
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
feat(poetry): support PEP 621 in Poetry 2.0+ #1003
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1003 +/- ##
=====================================
Coverage 93.0% 93.0%
=====================================
Files 37 37
Lines 994 994
Branches 98 99 +1
=====================================
Hits 925 925
Misses 55 55
Partials 14 14 ☔ View full report in Codecov by Sentry. |
75b1ab8
to
2a778bd
Compare
The failing tests are unrelated to the changes, and are likely caused by the functional tests using Poetry 2.0, and failing somehow. Getting the same failure in #1004. Funny how when random failures appear, it's often only on Windows 💀 |
2a778bd
to
fe712d9
Compare
Alright, it took a bit of time, but CI is now fixed, so this is definitely ready for review now 😄 |
fe712d9
to
28e101a
Compare
28e101a
to
3dda700
Compare
Closes #1002.
Blocked by #1007.PR Checklist
docs
is updatedDescription of changes
Support PEP 621 syntax that Poetry 2.0 supports by implementing the suggested solution from #1002.
PoetryDependencyGetter
now extendsPEP621DependencyGetter
, which makes it possible to parse PEP 621 dependencies even though a[tool.poetry]
section is defined.Whether projects actually use PEP 621 syntax or not, or only partially, allows parsing Poetry specific sections, similarly to other managers that define their own syntax for defining development dependencies, on top of PEP 621.
Specificities of the implementation
Poetry 2.0 is a bit more different than other PEP 621 managers, as per documented here, it is still possible to define dependencies under
[project.dependencies]
and have some specific parts of the data in[tool.poetry.dependencies]
that enrich the dependencies that are defined in[project.dependencies]
, while disallowing defining new dependencies in[tool.poetry.dependencies]
.For instance, given:
httpx
from[tool.poetry.dependencies]
enriches the data inhttpx
from[project.dependencies]
mypy
is NOT installed as a dependency at all by Poetry and simply skippedThis is handled in the changes by not extracting
[tool.poetry.dependencies]
section at all if there is at least one dependency defined under[poetry.dependencies]
, since in that case, we can assume that keys under[tool.poetry.dependencies]
will only be duplicate of keys under[project.dependencies]
, expect for new dependencies, which are skipped by Poetry anyway, so we do not want to extract them.Note that the documentation mentions that
dynamic = ["dependencies"]
could be used, but on my local tests, it did not change anything to set it or not, whether using Poetry as an environment manager or as a build system for building packages throughpoetry build
.Test plan
Additionally to the added unit and functional tests, I've also did some local tests on a dummy project to check Poetry's behaviour.
Documentation changes
It wasn't easy to update the documentation to highlight the new behaviour. I wasn't sure at first if we should separate Poetry into 2 "sections" in the page that lists supported package managers, one for "PEP 621" syntax and one for the legacy one, but since it's possible to mix both, I think this is less confusing overall to have everything as a subsection of "PEP 621".