Skip to content
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

Incompatibility with PEP 621 #748

Closed
1 task done
abravalheri opened this issue Nov 19, 2021 · 12 comments · Fixed by #759
Closed
1 task done

Incompatibility with PEP 621 #748

abravalheri opened this issue Nov 19, 2021 · 12 comments · Fixed by #759
Labels
🐛 bug Something isn't working

Comments

@abravalheri
Copy link

Hi guys I am under the impression that pdm is not compatible with PEP 621 (I have been working in a pyproject.toml validator and this problem showed up when running it against some projects created using pdm).

In particular the problems lie in the following parts of the spec:

  1. version - Format: string
  2. Build back-ends MUST raise an error if the metadata specifies a field statically as well as being listed in dynamic.

PDM documentation indicate that users can do version = {use_scm = true} or version = {from = "pdm/__init__.py"}, which violates the spec in (1).

Regarding item (2), I understand that passing version = {use_scm = true} or version = {from = "pdm/__init__.py"} doesn't really is specifying a field statically, but this is still a very controversial usage/interpretation of the standard.

  • I have searched the issue tracker and believe that this is not a duplicate.

Make sure you run commands with -v flag before pasting the output.

Steps to reproduce

  1. Check a PDM's powered project that uses dynamic versions for its pyproject.toml
  2. Validate the pyproject.toml file against PEP 621

Actual behavior

PDM accepts a inline-table version.

Expected behavior

PDM should error when version is not a string.
Additional tool-specific information needed for dynamic version discovery, should be stored in tool.pdm

@abravalheri abravalheri added the 🐛 bug Something isn't working label Nov 19, 2021
@pawamoy
Copy link
Contributor

pawamoy commented Nov 19, 2021

I guess there's an issue with classifiers as well then. Too bad, it means we cannot specify some classifiers statically and let the build backend add some more dynamically...

@abravalheri
Copy link
Author

Yeap, the 2 things cannot exist at the same time unless using a tool-specific table.

The PEP goes even further and says:

If the metadata does not list a field in dynamic, then a build back-end CANNOT fill in the requisite metadata on behalf of the user (i.e. dynamic is the only way to allow a tool to fill in metadata and the user must opt into the filling in).

So it is "not allowed" to implicitly add classifiers to the user-given list (i.e. add extra classifiers to PKG-INFO/METADATA without the field being listed in dynamic which in turn requires the user to leave the field completely unspecified).

@frostming
Copy link
Collaborator

frostming commented Nov 22, 2021

Thanks, I've also noticed that and planned to change someday. version will be moved to tool table automatically, but classifiers, I think it is not intuitive to defined it in tool.pdm just for auto-filling, so maybe it is time to remove it from the dynamic fields and make it pure static... too bad, the same feeling as @pawamoy

@abravalheri
Copy link
Author

Should we start a discussion on discord about this, at least for the classifiers? It is a very handy feature...

I don't know how the community would feel about creating another PEP to amend this one, though... It might be difficult to get people on board.

@frostming
Copy link
Collaborator

Sure, join the PDM discord channel which can be found in the README

@abravalheri
Copy link
Author

Sorry, I mean a discussion with the packaging community about amending the PEP...

@frostming
Copy link
Collaborator

Okay, then https://discuss.python.org/c/packaging/14 should be the correct place, would you like to create a topic there?

@frostming
Copy link
Collaborator

frostming commented Nov 22, 2021

But IMO according to the spirit of dynamic of PEP 665: dynamic fields shouldn't be listed as a static field at the same time, there is no space for the autofilling classifiers, unless users are okay(at least I am not) to have them written in the tool table. Or do you have better approach?

@frostming frostming mentioned this issue Nov 22, 2021
2 tasks
@abravalheri
Copy link
Author

I understande @frostming, and I don't have a better approach. Maybe the best is to align with the PEP.

CharString pushed a commit to CharString/affen that referenced this issue Dec 1, 2021
@oprypin
Copy link

oprypin commented Jan 3, 2022

So the ability to automatically specify Python version classifiers has been fully removed:

I feel like that was a huge overreaction. Surely there are ways to keep this functionality under a different config key. And I wasn't specifying any extra static classifiers anyway.

I'm once again in a situation where I'm lacking features compared to Poetry
https://python-poetry.org/docs/pyproject/#classifiers

This, along with the fact that features are being removed like this just on a whim, is again a showstopper for me.

@pawamoy
Copy link
Contributor

pawamoy commented Jan 3, 2022

You mean marking classifiers as dynamic, and specifying classifiers in [tool.pdm] (or not), so that PDM can also add dynamic ones?

@pawamoy
Copy link
Contributor

pawamoy commented Jan 4, 2022

Well in the end, I like declaring my classifiers as static, because it forces me to actually add some meaningful ones 😁
Not sure if classifiers other than supported Python versions are really that useful though 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants