Move project configuration to pyproject.toml

[singingwolfboy]

The pyproject.toml file is the new standard for project configuration in Python. It is the preferred configuration file for pip, and many other tools support it as well. This pull request moves as much of the project configuration as possible into this file, deleting the now-unnecessary files that they used to live in.

Note that this pull request also deletes setup.cfg and setup.py! For more information about how this project can be packaged and uploaded to PyPI, see the flit documentation. flit handles the heavy lifting for interfacing with PyPI. (Users do not need flit installed on their computer in order to install this project! As I said, pip supports this build system, and has for many years now.)

[codecov]

Codecov Report

Base: 94.19% // Head: 94.19% // No change to project coverage 👍

Coverage data is based on head (6c23431) compared to base (664ddf2).
Patch has no changes to coverable lines.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1382   +/-   ##
=======================================
  Coverage   94.19%   94.19%           
=======================================
  Files          28       28           
  Lines        5085     5085           
  Branches      968      968           
=======================================
  Hits         4790     4790           
  Misses        176      176           
  Partials      119      119           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[singingwolfboy]

Hi @MartinThoma, could you please review/merge this pull request?

[MartinThoma]

I'm hesitating to merge this one. The current workflow works and I don't see any issues with that.

I'm open to move isort (docs) and pytest (docs).

I'm way more hesitant with anything that could break packaging.

[MartinThoma]

Moving the coverage config is also not a big deal for me

[singingwolfboy]

I understand your hesitation, but I'm trying to move the Python developer community towards the new standard. I'll mention again that pyproject.toml is the preferred packaging format for pip, and setup.py is now considered "legacy".

Of course, this is your project, and it's your decision. But is there anything I can do to relieve your fears and help you migrate to the new standard?

[MasterOdin]

I'm way more hesitant with anything that could break packaging.

Perhaps it'd help to list out your workflow for doing packaging and releasing here? I think you use build in combination with twine to handle publishing?

By listing out the explicit steps, can then show what that'd look like via flit (or whatever) instead with the pyproject.toml file.

[MartinThoma]

When I release a new version, I do this:

python make_changelog.py
# manually adjust the changelog
# manually bump the version in _version.py
make upload
git add CHANGELOG.md
git add PyPDF2/_version.py
git commit
# Add the changelog as a commit message with the subject line 'REL: x.y.z'
git push
git tag -s x.y.z
# Add the changelog as a tag message
git push --tags
# Create a release version on Github

The only thing that would change is the command behind make upload. How would that change?

setup.py is now considered "legacy" [by pip]

Huh, that is interesting. I didn't know that. Thanks for sharing!

[MartinThoma]

@MasterOdin If I understand your comment right, you would also be in favor of merging this PR?

[singingwolfboy]

The only thing that would change is the command behind make upload. How would that change?

Currently, make upload runs python setup.py sdist bdist_wheel && twine upload -s dist/*. That would change to just flit publish, which takes care of both building and uploading the artifacts to PyPI.

I see that you have twine listed as a dev dependency via requirements/dev.txt. Let me make some changes to this pull request so that you can install the dev dependencies by running pip install -e .[dev], as well. I'll also rebase to resolve the conflicts.

[MasterOdin]

My comment was more that I think on merging this, it would ideally be that your workflow would ideally not change, or become simpler, and that @singingwolfboy PR would include this. So from the above, the question would be if make upload would continue to work as-is, or if that'd need to be changed. Perhaps other bits of the makefile (or gh action files) need changing as well, especially with the mention of flit.

As for whether this should be done, I would classify myself as largely "neutral" in merging this. I think that the goal of consolidating around pyproject.toml in the ecosystem will be good and at some point you'll probably have to, but also that the rollout of it across the python ecosystem has been a huge dumpster fire. The adoption of this format has been over a half-decade in the process at this point, and some aspects of it still aren't really standardized in a satisfying way, and then you've got poetry which doesn't follow PEP-621 and probably won't for a while yet.

Then you've got stuff like the fact that pip recommends everyone uses pyproject.toml, but still uses the legacy setup.py setup (amongst other very popular projects) and so doesn't necessarily inspire confidence that the format won't change in the future requiring maintenance overhead...

[MartinThoma]

Currently, make upload runs python setup.py sdist bdist_wheel && twine upload -s dist/*. That would change to just flit publish

How can I inspect the artifact flit creates before / without uploading it?

[singingwolfboy]

How can I inspect the artifact flit creates before / without uploading it?

You can use flit build to build the artifact, and inspect it at that point. Once you're satisfied, you can run flit publish and it will upload that same artifact -- or if you want to be paranoid extra sure, there's nothing wrong with continuing to use twine to upload the built artifact, just as before. I assumed that you would want fewer tools in your workflow, so I removed twine from the requirements/dev.in, but I'm happy to revert that change if my assumption was wrong!

[singingwolfboy]

My comment was more that I think on merging this, it would ideally be that your workflow would ideally not change, or become simpler, and that @singingwolfboy PR would include this. So from the above, the question would be if make upload would continue to work as-is, or if that'd need to be changed. Perhaps other bits of the makefile (or gh action files) need changing as well, especially with the mention of flit.

Indeed! Thank you for pointing that out -- I believe I've made the necessary changes in this PR so that the workflow should be unchanged.

As for whether this should be done, I would classify myself as largely "neutral" in merging this. I think that the goal of consolidating around pyproject.toml in the ecosystem will be good and at some point you'll probably have to, but also that the rollout of it across the python ecosystem has been a huge dumpster fire. The adoption of this format has been over a half-decade in the process at this point, and some aspects of it still aren't really standardized in a satisfying way, and then you've got poetry which doesn't follow PEP-621 and probably won't for a while yet.

Then you've got stuff like the fact that pip recommends everyone uses pyproject.toml, but still uses the legacy setup.py setup (amongst other very popular projects) and so doesn't necessarily inspire confidence that the format won't change in the future requiring maintenance overhead...

True. To be fair, packaging in the Python ecosystem has been more-or-less a dumpster fire for roughly as long as the Python ecosystem has existed. To me, pyproject.toml feels like a step in the right direction.

[MasterOdin]

True. To be fair, packaging in the Python ecosystem has been more-or-less a dumpster fire for roughly as long as the Python ecosystem has existed. To me, pyproject.toml feels like a step in the right direction.

Agreed on both points.

[MartinThoma]

I'm currently checking the distribution files. Left is old, right is new.

Wheel

I changed the order of entries on the left so that the diff is easier to read

• ✔️ Author and maintainer name/email is different: OK
• ❌ License is no longer included ⚠️ Can we fix that? Did the Python community find another way to standardize how to communicate licenses?
• ✔️ Platform: UNKNOWN was removed: OK. I have no idea why it was there in the first place
• ✔️ Requires-Dist: Spaces and quotes changed: OK
• ✔️ Requires-Dist: dev and docs was added. Nice!
• ✔️ Home-page: changed to a project-url: OK

The file top_level.txt which contains just PyPDF2 is no longer there. Seems to be ok: python-poetry/poetry#3093 (comment)

[MartinThoma]

⚠️ Blocker for merging: The sdist distribution file is empty

edit: I just needed to make sure the repo is clean. I leave this here as it might help other people making their first steps with flit

$ python -m venv venv
$ source venv/bin/activate
$ pip install flit
$ flit build --format sdist
/home/moose/.pyenv/versions/3.10.2/lib/python3.10/site-packages/requests/__init__.py:102: RequestsDependencyWarning: urllib3 (1.26.12) or chardet (5.0.0)/charset_normalizer (2.0.11) doesn't match a supported version!
  warnings.warn("urllib3 ({}) or chardet ({})/charset_normalizer ({}) doesn't match a supported "
Untracked or deleted files in the source directory. Commit, undo or ignore these files in your VCS.
$ ls -lh dist
total 4,0K
-rw-rw-r-- 1 moose moose 63 Okt 22 10:23 PyPDF2-2.11.1.tar.gz

[MartinThoma]

sdist: Needs adjustment

❌ The new sdist should look more similar to the old one. That means the following files/folders should not be added:
.github, docs, requirements, resources, sample-files, tests, .gitmodules, .gitignore, .git-blame-ignore-revs, .pre-commit-config.yml, .flake8, .pylintrc, Makefile, make_changelog.py, mutmut-test.sh, mutmut_config.py, tox.ini

That should be possible: https://flit.pypa.io/en/stable/pyproject_toml.html#sdist-section

@singingwolfboy Would you mind to add this?

[MartinThoma]

For the license, I see https://flit.pypa.io/en/stable/flit_ini.html?highlight=license#metadata-section

License: The name of a license, if you’re using one for which there isn’t a Trove classifier. It’s recommended to use Trove classifiers instead of this in most cases.

PEP 639 (draft status) seems very relevant. I need to read that. However, if the generated distributions (sdist / wheel) would be the same for the License topic, we could probably merge this PR sooner. Only if the format changes we need to have the discussion now.

[MartinThoma]

@singingwolfboy Thank you for all the work you already put into this 🙏 If you want to focus on other topics, I would understand that. In this case I could very likely figure out the rest by myself (in a follow-up change). Just let me know how you would like to proceed :-)

[MartinThoma]

@singingwolfboy If you add the two suggested changes, the PR would be fine and could be merged as-is :-)

[MartinThoma]

It's merged! Thank you @singingwolfboy for your work and for helping the Python community to move forward ❤️

[singingwolfboy]

It's merged! Thank you @singingwolfboy for your work and for helping the Python community to move forward ❤️

You're very welcome! And thank you for being so thorough with your review, and for pushing this PR the last few steps over the finish line. 😄