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

Move the metadata into PEP 621-compliant pyproject.toml. #627

Closed
wants to merge 2 commits into from

Conversation

KOLANICH
Copy link

No description provided.

@amoffat
Copy link
Owner

amoffat commented Oct 18, 2022 via email

@KOLANICH
Copy link
Author

2.0 branch

  1. uses poetry, that doesn't support PEP 621
  2. is stale.

@amoffat
Copy link
Owner

amoffat commented Oct 18, 2022

Poetry will eventually support PEP 621, and I am not currently interested in switching from poetry on the 2.0 branch unless there is a very compelling reason. I think that makes this PR premature, do you agree?

@KOLANICH
Copy link
Author

I think that makes this PR premature, do you agree?

I disaggree. I'm interested in getting the metadata in PEP 621 format (or at least in having PEP 517-compliant pyproject.toml + setup.cfg or any other declarative metadata format that can be easily and reliably parsed) right in the default branch because I am creating a package manager for python replacing pip for some use cases (it installs bleeding-edge packages from GitHub repos of projects), and that package manager relies on PEP 517 for detection of build backend and for building packages and on machine-parseable metainformation being available within source distribution (and setup.py is not considered a machine-parseable format, well, it can be parsed for some cases, but it is additional complexity on the tool side and noone can guarantee that the parsing is correct, a better alternative is to switch the packages to the state-of-the-art declarative metadata format). So I move dependencies of my packages, that are not already using the declarative metadata format, to PEP 621, one-by-one.

@amoffat
Copy link
Owner

amoffat commented Oct 19, 2022

What do you propose we do with poetry on the 2.0 branch? since it will eventually be merged into default branch, it will undo any of your proposed changes.

@pquentin
Copy link

I came here because I received similar pull requests in projects I maintain (urllib3/urllib3#2299, python-trio/trio#2448, python-trio/trio#2449) and while I understand your goals @KOLANICH, you're not very reasonable because you don't care about the goals of the projects you're contributing to. Also, opening a pull request is one thing, but making sure that the resulting sdist and wheels are the same and fixing any issues that come up are another thing.

I'm sure you'll find this story very educational: https://nitter.it/di_codes/status/1542489151630712832

@KOLANICH
Copy link
Author

What do you propose we do with poetry on the 2.0 branch?

This decision is not up to me, but up to the maintainers.

since it will eventually be merged into default branch

That branch is stale:

Updated Oct 13, 2021 by ecederstrand

, so there is no certainty (for me) that it will be merged in near future and, TBH, that it will be merged at all.

There is a choice:

  1. keep PEP 621 metadata, keep setuptools
  2. change the backend to something else (flit_core, hatchling), keep PEP 621 metadata
  3. change the backend to poetry, keep the PEP 621 metadata - only makes sense if poetry implements PEP 621
  4. replace the file with the one from release/v2.0.0 branch, switching to poetry and converting the metadata into poetry-specific format

I'm OK witn any of them: in all of them the metadata format is declarative and my tool has metadata parsing backends implemented for all flit, poetry, hatchling and setuptools.

I consider the options 1 and 2 the most beneficial since the metadata format used in them is the standardized PEP 621. I consider the option 1 the most beneficial since recently speed of setuptools has improved (I have not measured, just got surprized how fast it has started to work after I have updated it).

it will undo any of your proposed changes.

No problem. Closing a PR will also "undo" them. The most of conversion was automatic using setuptools_py2cfg and ini2toml tools, the manual part of work was to check whether everything looks OK after the conversion, to restore the info stripped by the tools and to fix CI scripts.

@pquentin

I'm sure you'll find this story very educational: https://nitter.it/di_codes/status/1542489151630712832

Thanks for the story (I also use libredirect BTW), I enjoyed it. Well, I have considered some of the points described there. For example it could be possible to create a bot mass-converting packages and mass-sending PRs, but it would be very hard to do it with enough quality fully automatically, that's why I do it semi-automatically - the conversion is done automatically, then human-vetted and postprocessed.

while I understand your goals @KOLANICH, you're not very reasonable because you don't care about the goals of the projects you're contributing to.

Everyone cares about own issues, but it turnes out that in a lot of cases solving an issue of one person solves an issue of other ones too. Yes, I have implemented those PRs out of self-interest, but I consider them beneficial to the projects they are sent to because the PRs improve the projects' compatibility to the tools and because it is a step to improve security of the ones installing them (in setuptools repo one can find an issue (https://github.com/pypa/setuptools/issues/2901) about making setup.py-based package building an explicit opt-in and about making package building using PEP 517 backends secure). Purpose of projects is to solve issues of their users that are within scope of projects, and if a project has files for packaging, then packaging-related issues are within the scope of the project.

Also, opening a pull request is one thing, but making sure that the resulting sdist and wheels are the same and fixing any issues that come up are another thing.

I will check it (and I usually currently rely on CI for detecting issues). Though please keep in mind, I don't think the wheels and sdists should be exactly the same to the ones generated by the old variant with setup.py. For example in the most of the PRs I switch to using setuptools-scm for fetching versions automatically, and it adds git commit tag to versions (it can be disabled, but I think it is a beneficial feature). Or when auto-generating version file I strip the unneeded minor parts: the template has to go into pyproject.toml and adding the comment there is just unneeded bloat. But if, for example, tests go into the wheel, if they didn't (or just shouldn't go there), then it should be solved.

@amoffat
Copy link
Owner

amoffat commented Oct 19, 2022

Thanks for the context. After careful consideration, I think we will continue down the poetry path. Please contact the poetry team if you wish to advance PEP-621 there, as I believe it will be a better use of everyone's energy. Here's a good starting point python-poetry/roadmap#3

@amoffat amoffat closed this Oct 19, 2022
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.

3 participants