python-poetry / poetry

Python packaging and dependency management made easy
https://python-poetry.org
MIT License
31.05k stars 2.25k forks source link

Use the [project] section in pyproject.toml according to PEP-621 #3332

Open MartinThoma opened 3 years ago

MartinThoma commented 3 years ago

Feature Request

PEP621 (Storing project metadata in pyproject.toml) and PEP631 (Dependency specification in pyproject.toml based on PEP 508) define a clear way how metadata and dependencies should be stored. Both of them have the status "accepted". It would be good if poetry would use that.

For example, I've noticed that poetry init generates a pretty similar pyproject.toml, but puts the metadata in a poetry-specific section.

layday commented 1 year ago

How did it become partially incompatible? Poetry doesn't use the project table at all.

dimaqq commented 1 year ago

Looking at the docs, Ruff insisted on certain fields under the [project] key, and maybe it’s not happy if fields are duplicated, while poetry requires certain field under the tool.poetry key

zmievsa commented 1 year ago

@dimaqq It's not happy if the [project] is not there and it's right: https://packaging.python.org/en/latest/specifications/declaring-project-metadata/#declaring-project-metadata

Name is the only required field. We should move name from tool.poetry to project so that poetry is compatible with the standards.

dimaqq commented 1 year ago

Uhm, should is a rather strong word... I'd say, it would be nice if poetry supported both legacy and new layout; as long as some name if given, it should be good enough.

dimaqq commented 1 year ago

P.S. there's https://github.com/python-poetry/poetry-core/pull/567 let's try the branch out and add our reviews there 🙏🏻

henryiii commented 1 year ago

Poetry would not be compatible if you moved only name. Any unspecified field is guaranteed to be empty if you don't list it in dynamic. So you'd have to list dynamic with every known field as well.

Which you don't need to do! It's perfectly valid to not use [project] and always will be. If Ruff 200 is unhappy about it, Ruff is wrong - though I'd be surprised if it was, there are more pyproject.toml's without [project] than there are with, due to setuptools. You should only validate the project table if the project table is present. If it is present, you should implement everything it supports.

(It's also perfectly valid to support a [tool.*] table + [project], especially for non-metadata things like dev tools stuff, as long as you fully implement [project])

@charliermarsh, would it make sense to implement @abravalheri's validate-pyproject instead of a custom validator code? I'm planning on working on writing at least one plugin for validate-pyproject (to validate scikit-build-core's config).

henryiii commented 1 year ago

@Ovsyanka83 is it possible you are adding something to [project]? It's not valid to use anything in [project] without using all of it (anything unspecified is guaranteed to be blank). I've seen people try to implement only [project] requires-python which is incorrect. Ruff would be correct telling you it's incorrect if that's the case.

(Technically, you could add [project] name = to the same thing as Poetry then set dynamic = [<everything else>] and it would be correct again, even with Poetry ignoring it :) )

layday commented 1 year ago

It's not happy if the [project] is not there and it's right: [...]

It's not right about that at all (assuming it does actually work that way[^1]). The project table is not required to be present in pyproject.toml.

[^1]: I ran ruff on a Poetry project of mine with the RUF ruleset enabled and it's not emitting any errors.

zmievsa commented 1 year ago

Sorry, I was wrong: we did have another field in project which caused the error with name to occur. My bad. Sorry for wasting your time :)

konstin commented 1 year ago

Coming here to say that this issue is very important to us and that ruff has recently become partially incompatible with poetry due to RUF200 code.

In case someone has a specific case where RUF200 is raised on a valid poetry config, please open an issue at https://github.com/astral-sh/ruff/issues

eli-schwartz commented 1 year ago

Perhaps for now poetry could detect a [project] section and unconditionally raise:

ERROR: [project] section specified in pyproject.toml but poetry does not support this

Since

henryiii commented 1 year ago

I don't think that would help much, that's really the job of a validation tool (like Ruff).

FYI, listing all metadata values (exempt name) in [project] dynamic and listing the same [project] name as [tool.poetry] name is valid via the PEP and by Poetry. It's not actually doing anything with the [project] items, but it's a valid config. Someone might even use this to dual support backends (you'd want to validate that information listed in two places was identical, but it could be done). Another reason could be to list information for tools that read pyproject (like ones that look for requries-python) that aren't aware of Poetry's custom config.

IMO effort should go into allowing Poetry to optionally support [project], that's a great start (#567). Anything listed in dynamic could be specified the classic way.

eli-schwartz commented 1 year ago

I don't think that would help much, that's really the job of a validation tool (like Ruff).

I don't see how ruff could possibly expect to perform any sort of validation here at all, except by acting as a build frontend and invoking the optional prepare_metadata_for_build_wheel pep517 hook, then comparing the results and asserting that the prepared metadata does not deviate.

FYI, listing all metadata values (exempt name) in [project] dynamic and listing the same [project] name as [tool.poetry] name is valid via the PEP and by Poetry. It's not actually doing anything with the [project] items, but it's a valid config.

It is technically valid but in practice not, because the expectation in the poetry ecosystem is to use both upper and lower bounds for all dependencies, and then to do it in a non-PEP440 compliant manner, and I have severe doubts that people will manually get this correct. If they don't match, then the failure to match makes the [project] section invalid.

Someone might even use this to dual support backends (you'd want to validate that information listed in two places was identical, but it could be done).

A slight challenge here is that some backends consider the idea of having frontends to be bloaty or idk, and don't include command-line entrypoints (e.g. setuptools). So using them means patching the build backend in pyproject.toml and then running pip. That being said, poetry does have a CLI. :)

Having successfully built a wheel with each backend you could then compare the resulting .dist-info and validate that they match, but the standard doesn't actually require them to match if they would prefer to diverge in their handling of dynamic metadata. So that would be a Quality of Life check that users don't end up with gunky and unhelpful installation results, not a check that their PEP621 conformance is any good.

henryiii commented 1 year ago

I don't see how ruff could possibly expect to perform any sort of validation

Ruff can validate that the [project] table is valid according to existing PEPs, just like it did with @Ovsyanka83 & caught a real issue. PEPs are allowed to change the format in the future, though, so a build backend should not validate, only linters. For example, I'm hoping to propose supporting a table for dynamic eventually; due to the way Python iterates over keys, it should naturally be backward compatible by most backends, but will require updating most validators.

pyproject-validate, a pyproject.toml linter, supports adding schemas for backends, by the way, so it could learn to validate a tool.poetry table as well. I'm planning to write a schema for tool.scikit-build. It already has one for project and tool.setuptools.

It is technically valid but in practice not

You don't need to list anything twice except the name. You'd have dynamic = [..., "dependencies", ...], and then any static analysis tool knows that there are dependencies, and they are coming from some other source. (Likewise, having a [project] but no dependencies tells static analysis tools like GitHub's depgraph you have 0 dependencies).

I don't know if anyone would want to do this, but I don't think arbitrarily ruling this out is a good idea. The only way you'd get an undetectable user error with this is if someone listed both [tool.poetry] name and [project] name and didn't expect to have dual configurations.

expectation in the poetry ecosystem is to use both upper and lower bounds for all dependencies

There is no such thing as a "poetry ecosystem". An ecosystem needs to have its own hosting. Conda has an ecosystem. There's a Python ecosystem hosted on PyPI, and the social expectation there is nothing has upper caps. If you publish a Poetry package there, that is, you are making a library, then you should follow the social expectations for PyPI, not Poetry. If you are working on an "app", you don't "publish" it, and [project] (which is about filling out wheel metadata for PyPI) doesn't really matter one way or anther. And you use a lock file, so upper bounds are still silly IMO, since you control updates with poetry update.

johnthagen commented 10 months ago

I've seen Poetry's lack of PEP-621 support brought up more and more on forums, podcasts, within work, etc as a reason to not use Poetry. Poetry isn't mentioned as prominently on PyPA tutorials due to this as well. IMO, the lack of this feature is beginning to become an existential risk to Poetry's long term health.

Is there a way the community can help test/reach consensus/ move this along? ❤️

radoering commented 10 months ago

I can only speak for myself, but what is currently stopping me from investing more time in it is the (afaik) missing concept for multiple-constraints dependencies with additional information. In https://github.com/python-poetry/roadmap/issues/3#issuecomment-1186273566, the migration for single-constraints dependencies looks like:

[tool.poetry.dependencies]
tomlkit = { version = ">=1", develop = true, source = "foo" }

becomes

[project]
dependencies = [
    "tomlkit>=1",
]

[tool.poetry.dependency-options]
tomlkit = { develop = true, source = "foo" }

which feels ok to me.

However, what about?

torch = [
    {version = ">=2.0.1", markers = "sys_platform == 'darwin'", source = "pypi" },
    {version = ">=2.0.1", markers = "sys_platform == 'linux'", source = "torch" },
 ]

Maybe?

[project]
dependencies = [
    "torch>=2.0.1 ; sys_platform == 'darwin'",
    "torch>=2.0.1 ; sys_platform == 'linux'",
]

[tool.poetry.dependency-options]
torch = [
    { markers = "sys_platform == 'darwin'", source = "pypi" },
    { markers = "sys_platform == 'linux'", source = "torch" },
]

(We have to match dependencies and options via environment markers.) That feels quiet cumbersome compared to the existing solution. If I was a user who did not care about standard compliance but about usability, I would see this as a step in the wrong direction.

Personally, I have the feeling that there are (at least) two opposing groups of users:

  1. Users who don't need fancy stuff like multiple-constraints dependencies with explicit sources and want standard compliant dependency specifications.
  2. Users who need fancy stuff like multiple-constraints dependencies with explicit sources and care more about a simple way to configure this stuff than about standard compliance.

I wonder, if we should keep [tool.poetry.dependencies] in addition to [project.dependencies] (and internally merge both) - or just allow specifying dependencies with version constraints in [tool.poetry.dependency-options] that are not in [project.dependencies] - so that users who don't care about standard compliance and need special features can keep specifying their dependencies in a simpler way?

rmarquis commented 10 months ago

so that users who don't care about standard compliance

The more crucial question is imho the actual preference of the maintainers: whether you guys prioritize a simpler method for configuring dependencies in specific scenarios, or the value of adherence to standards.

I acknowledge my understanding of the full implications might be limited, but I feel that enforcing standard compliance is the only sane approach going forward.

dimbleby commented 10 months ago

Speaking only for myself: I've no objection to poetry supporting PEP621 but I don't work on this because it isn't important to me - certainly not important enough to justify the amount of effort that I expect it would take.

I suppose that this issue has been open for three years because the same is true for all of the other eight billion people on the planet too.

https://github.com/python-poetry/poetry-core/pull/567 looks like the most progressed try, but seems to have fizzled. Probably it would be a good starting point for anyone who does care about this.

Is there a way the community can help test/reach consensus/ move this along?

The same way as for every other open issue in the repository: be the change you want to see! Mostly I expect this "just" needs someone to take on the work...

sinoroc commented 10 months ago

However, what about?

torch = [
    {version = ">=2.0.1", markers = "sys_platform == 'darwin'", source = "pypi" },
    {version = ">=2.0.1", markers = "sys_platform == 'linux'", source = "torch" },
 ]

This is inconvenient that this feature made it into Poetry in this form. This crosses the boundary between abstract dependencies and concrete dependencies. Same for "develop". Packaging metadata (listing abstract dependencies) should be kept separate from data about "how should this actually be installed" (source indexes, editable or not, and so on).

MartinThoma commented 10 months ago

Wouldn't it be possible to define a configuration hirarchy?

project.dependencies is overwritten by tool.poetry.dependencies. That would mean that people who need the feature described by @sinoroc can use it, and the rest can use standard pyproject.toml files.

radoering commented 10 months ago

This crosses the boundary between abstract dependencies and concrete dependencies.

project.dependencies is overwritten by tool.poetry.dependencies.

I think I like this idea:

sinoroc commented 10 months ago

@radoering

* `project.dependencies` are the abstract dependencies, which are used for packaging metadata

* `tool.poetry.dependencies` are the concrete dependencies, which are used for locking

There is still at least one remaining issue...

In some cases having concrete dependencies in the pyproject.toml file that is shared by all contributors can be an issue. For example with the package index URLs being hard-coded. I do not have the link to the issues right now, but there were problems (with Poetry and other tools/workflows as well) were some contributors do not have access to the same package indexes as others. For example because access to some servers is either too slow or in some cases forbidden/heavily restricted. If I recall correctly even PyPI itself is problematic in some regions and mirrors have to be used instead.

I know it is something that can and probably should be handled separately, but I thought I would mention it anyways since we are kind of on the topic.

If not possible to store concrete dependencies separately (in some cases ideally it should be a different file that does not get committed to git), at least it should be easy to override (the index URLs for example).

NeilGirdhar commented 10 months ago

If not possible to store concrete dependencies separately (in some cases ideally it should be a different file that does not get committed to git), at least it should be easy to override (the index URLs for example).

I totally agree. I think I proposed this exact idea somewhere else, but I can't find it. Essentially, some kind of pyproject_override.toml, which would be able to override most settings for the local machine and not be committed. This would be useful for all kinds of things including:

Maybe we should make a separate issue for this idea?

sinoroc commented 10 months ago

@NeilGirdhar Aside: I have this: https://discuss.python.org/t/proposal-overrides-for-installers/23666 -- but I guess we are off-topic now.

And otherwise I know there are already some open issue tickets about the index URL issue. Here is one: https://github.com/python-poetry/poetry/issues/5958.

samuelcolvin commented 10 months ago

Just to chime in, we in the Pydantic team are keen to adopt poetry, but we would need poetry to use the standard [project] layout to do so.

dimaqq commented 10 months ago

I think that pragmatic approach would be best — someone contributes top-level fields only, leaving dependency resolution for later.

johnthagen commented 8 months ago

An example of a project migrating from Poetry to PDM in part of the missing PEP 621 support

tigerhawkvok commented 7 months ago

I think Poetry's syntax is better and more powerful, but it's important to support PEP621.

I propose that:

The upshot of my suggestion is to not try to support some complicated blend. Let people pick one or the other, and raise an error if it's not just one or the other. Should be much easier to develop.

eli-schwartz commented 7 months ago

The upshot of my suggestion is to not try to support some complicated blend. Let people pick one or the other, and raise an error if it's not just one or the other. Should be much easier to develop.

I think I can summarize your fairly lengthy description.

This has already been suggested but I believe poetry didn't want to do that.

abravalheri commented 7 months ago

[project] may always exist\nrequires-python may exist in either tool.poetry.dependencies or project, but only in one.\nIf tool.poetry.dependencies exists, neither dependencies or optional-dependencies are allowed as keys in project. Having both raises an error.

I don't know if I understood this proposal correctly, but if the idea is allow [project] and tool.poetry.dependencies to be defined simultaneously, then according to the PEP, project.dynamic must be validated in such scenarios.

It should:

  1. Be necessarily defined by the user.
  2. Contain "dependencies".
Turakar commented 7 months ago

Have we already discussed the following option: Whenever Poetry performs dependency resolution, i.e., when it makes changes to poetry.lock, it could also write a PEP 621 compliant set of dependencies to [project], effectively overwriting the contents there with a big comment warning # These dependencies are automatically generated by Poetry based on the contents of [tool.poetry.dependencies]..

If all other Metadata is equally well specifiable in [project], we might want to deprecate writing it to Poetry-specific tables, otherwise we could rely on overwriting here as well. Most likely, a feature flag would be desirable to turn this behavior on or off.

This proposal becomes difficult if there are Poetry dependency expressions which are inexpressible in PEP 621. I do not really know a solution to this if these cases have actual usefulness and are not just weird corner cases.

merwok commented 7 months ago

pyproject is not meant to be a lock file – dependencies in pyproject are not a full list of pinned dependencies.

flying-sheep commented 7 months ago

I think Poetry's syntax is better and more powerful

That’s subjective, and false, respectively.

Regarding “better”, I think it’s worse since ^ encourages specifying upper bounds, which you shouldn’t do in almost all cases.

Regarding “more powerful”, see above the distinction between abstract and concrete dependencies.

NeilGirdhar commented 7 months ago
  • If you want to specify how to resolve dependencies, project.dependencies is not the place for that.

You may want to watch this proposal (or its descendants when it grows them) so that you can participate in it.

flying-sheep commented 7 months ago

Thanks. I personally don’t need this that much. I think people who do should contribute there, so they can get all the Poetry resolution niceness in a standardized way, everywhere.

eli-schwartz commented 7 months ago

Regarding “more powerful”, see above the distinction between abstract and concrete dependencies.

Huh, and I thought you were going to say something about how poetry's dependency syntax cannot be more powerful than the PEP 621 syntax since the poetry syntax is, quite literally, compiled by poetry into the PEP 621 syntax (PEP 508 strings) in the process of being written into the wheel metadata. This was, after all, a major factor in poetry's PEP 633 being rejected.

flying-sheep commented 7 months ago

You’re right. The metadata part (abstract dependencies) is just as powerful. Poetry mixes that with package sources and path dependencies, which are resolution features (concrete dependencies), and therefore shouldn’t be part of the dependency array.

Of course, package sources and path dependencies are super useful, but extricating them from the dependencies array doesn’t make anything less powerful, just differently structured.

radoering commented 5 months ago

9135 should be in a testable state now. (Consider it an alpha version.) Please try it out and report issues you encounter in the PR.