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

Upgrade the project config to use PEP standards (build-system + pyproject.toml) and remove 6 useless config files from the root folder #1883

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

vemonet
Copy link
Contributor

@vemonet vemonet commented Jul 5, 2023

Why these changes? It was working fine before!

Indeed the development setup for cookiecutter is really good! But its configuration is stored in many different files, using different formats, making the repository harder to read and maintain.

So we propose to upgrade the project to use PEP standards (517, 518, 621) as much as possible: defining a build-system and putting everything in the pyproject.toml file, this will have the following benefits:

  • Greatly reduces the amount of config files (6 files deleted from the root folder, most of them just had 2 or 3 lines of config for a tool, now everything is standardized under the pyproject.toml file)
  • Greatly improve readability and maintainability of config files used for development: more consistent configs, they are all defined using the same format
  • Using PEP 621 standard metadata means effortless compatibility with all python dev/build tools (apart from poetry who are planning to support PEP 621 in the next major release, but we don't really care)
  • Clear, easy and standard management of optional dependencies bundle
  • While many files have disappeared, we keep a development setup 1:1 identical to before: based on tox to manage envs and not a single configuration field has been left out (afaik)

Whats is Hatch?

Hatch is project management tool for python, relying on PEP standards: https://hatch.pypa.io/latest/

It's doing a lot of things, and doing them well! (I have been looking for a good project management tool in python for years, and this one really blew my mind, so simple, so powerful, no surprises)

You probably never heard of it (because people only complains about bad things!), but it's already v1, officially recommended and maintained by the PyPA (python packaging authority), it is used as build-system by many modern, well-built and popular python projects such as FastAPI, pydantic, and others (see the long list of hatch users here: https://hatch.pypa.io/latest/community/users/)

It allows you to:

  • Define all your package metadata in the pyproject.toml following PEP 621, so you don't need to have 4 files just for packaging metadata (setup.cfg, setup.py, ). Most tools now supports config from the pyproject.toml, apart from flake8 (but we can use the additional Flake8-pyproject package)

  • Define your dependencies, with bundle of optional dependencies (to be installed with pip install cookiecutter[docs,test]). You can go over all your dependencies in a glance instead of jumping between 3 different files with different formats

  • Define scripts to run with hatch run my-script, which kind of replace Makefile, at the difference that:

  • Hatch will automatically setup and sync the virtual env when you are running a script with hatch run, so no need to figure out what to install, just hatch run test and the virtual env will be set automatically. It can also run post install scripts like pre-commit install

  • Hatch relies a lot on existing well established tools like pip to install, or virtualenv, all it does is to provide a sane user interface over it that can be fully configured from the pyproject.toml following the PEP recommendations.

  • Hatch gets out of your way. Don't want to use their scripts? The devs don't even need to install the hatch package, and hatch will just be used to put together your package when your run python -m build

What have I done?

  • I moved to add hatch as build-system (more exactly hatchling) and moved as many configuration as possible to the pyproject.toml, but kept tox and Make to run stuff

  • I removed the 2 requirements.txt files for test and docs, and defined those as optional dependencies in the pyproject.toml, then tox can install dependencies from the optional bundles instead of the requirements files

  • Removed 6 config files: .bandit, MANIFEST.in, docs/requirements.txt, setup.cfg, setup.py, test_requirements.txt. Everything is now in the pyproject.toml

  • The package version is now only defined at one place: in the cookiecutter/__init__.py file, there is no need to define it in pyproject.toml, hatch will resolve it auto

And everything is working as it was before for the devs: use make or tox for tests/docs/lint!

The GitHub actions workflow also should not need any changes (I would probably install tox with pipx instead of pip to prevent any weird dependency conflict but that's another question!). With the hatchling build-system python -m build will be able to do everything needed to publish the python package

Could we do more?

Currently we could also easily replace tox with hatch to define envs matrix and test run. I already tried it, it works really well and make the config easier to read and maintain imo. You can check an example of pyproject.toml using hatch scripts here: https://github.com/vemonet/rdflib-endpoint/blob/main/pyproject.toml#L80 then the scripts are run with hatch run my-script

But tox provide a summary of the results of each step at the end, which hatch does not! So the current setup with tox seems better adapted to the project imo (it was really nice to run your test suite, kudos!)

We could also replace Makefile to directly use hatch scripts. The advantage of hatch is that it will automatically setup the env for the user, on the other hand the major advantage of Make is that it provides terminal autocomplete for the scripts (which hatch does not, you cannot tab your way to find the script you want :( )

For linting we could replace flake8 by ruff, it is now much more advanced, with many more auto fixes, more actively developed, and also used by many big python projects. I am using it on other projects, and would never go back!

Conclusion

This PR closes:

We could also delete codecov.yml which only contains comment=false, it does not seems codecov support config through the pyproject.toml though, so I could not move this config, but it seems fairly useless, and removing it does not seems impact the test coverage

@ericof ericof added this to the 3.0.0 milestone Jul 5, 2023
@ericof
Copy link
Member

ericof commented Jul 5, 2023

@vemonet As this is a significant change. I invite you first to open an issue and let us discuss this.

@vemonet
Copy link
Contributor Author

vemonet commented Jul 6, 2023

Hi @ericof, as I mentioned in the original explanation there is already an issue about it opened here: #1556

The issue about issues

There has been already complaints from the main maintainers that the number of issues are piling up and that they don't have time to close them: #1828 so creating an issue that is a duplicate of an existing issue where a discussion has been started seems to be really counter productive for everyone (and especially the maintainers, me I can just copy paste the explanation I already did, for the maintainers it means twice more issue and PR to manage and close...)

My previous merged PR closed 4 issues, those 4 issues are still opened, not sure how long it will take to close them even if the issue is solved.

I don't want to add unnecessary work to the maintainers. I made a long post explaning the changes in this PR, maintainers can discuss the changes in this PR, it will be easier for everyone. Instead of splitting the conversation in 2 channels. If someone really wants to post something in an issue instead of a PR then they can use #1556

About the change

It is not a significant change as it is completely invisible for the end users (template creators and template consumers), and it has no impact on the developers workflow.

The built package will work exactly the same, it's just some clean up of old code! (small refactoring are necessary from time to time and not really considered as significant changes)

We are still using make and tox, the only difference is from where tox retrieve its dependencies, and from where bandit gets it's config.

Also all tests, docs gen, linting are all working 100% like they were before

Conclusion

Could we just tag / request review from maintainers that might want to say something about it directly in this PR? and discuss changes directly in this PR? It would be helpful for this repository maintainers, and release some pressure from their shoulder (they get in the PR, check the code, say yes or no. Instead of arriving in an issue, going to the PR, checking the code, going back to the issue to comment... That does not make sense)

If you really want me to create a duplicate issue I can still do it in 2min, I'll just copy paste the current explanation provided with the PR

btw, what is your opinion about these changes ? :D

@henryiii
Copy link
Contributor

henryiii commented Jul 7, 2023

FYI, I'd not consider changing backends a breaking change. A user getting a wheel will get an identical wheel as before - neither the setup.py nor the pyproject.toml is placed in the wheel. We've moved lots of packages over (like packaging, wheel, build, pipx, nox, etc.) and the process is usually quite smooth. It only affects third party packagers that are building from SDist without using isolated environments (conda and the like), and they know how to make this change quite well by now.

I would not recommend trying to use one of the flake8 pyproject.toml adaptors. The flake8 authors seem to go out of their way to try to break these when possible (I could be wrong, but I'm 100% they will never do even a slight change to help one of these projects exist, and I've seen them keep breaking them!). Instead, I'd move to Ruff (probably in a PR before or after this one). Ruff is 100x faster, has huge numbers of plugins built in, has fixes for plugins that have been languishing elsewhere, and supports toml config. Ruff is possibly the fastest growing project I've seen; it's hard to point out projects that haven't switched. Examples of ones that have include pip, mypy SciPy, FastAPI. Until then, you can put the flake8 config in tox.ini, we did that for a while in build.

If you'd like docs on modern configuration, see https://packaging.python.org/en/latest/tutorials/packaging-projects/, https://learn.scientific-python.org/development/guides/packaging-simple/, or https://intersect-training.org/packaging/. (Disclaimer: I was a primary author on all three). If you'd like to compare build systems, cookiecutter gh:scientific-python/cookie will let you choose between twelve of them.

@henryiii
Copy link
Contributor

henryiii commented Jul 7, 2023

FYI, in tox 4, you can add:

[testenv]
package = wheel
wheel_build_env = .pkg

which will cache the wheel and provide a nice speed boost to running multiple envs.

@vemonet
Copy link
Contributor Author

vemonet commented Jul 7, 2023

Thanks a lot for all the details on packaging @henryiii I learned a lot of things

Indeed it makes more sense to move the flake8 config to the tox.ini file, I did not know it was possible. I just made the changes

I would agree to replace flake8 with ruff, I am using it on other projects and it is really great. But it would have introduced too great changes in 1 PR, I will create a follow up PR with ruff if this one gets merged

Something I forgot to mention also, currently I used the following for authors and maintainers:

authors = [
    { name = "Audrey Feldroy", email = "audreyr@gmail.com" },
]
maintainers = [
    { name = "Audrey Feldroy", email = "audreyr@gmail.com" },
]

Let me know who I should add to the list

@danieleades
Copy link
Contributor

This will need a big ol' rebase

@vemonet
Copy link
Contributor Author

vemonet commented Apr 8, 2024

Why? There has been no expression of interest for improving the current build system. Who would go through a rebase for nothing?

@danieleades
Copy link
Contributor

Why? There has been no expression of interest for improving the current build system. Who would go through a rebase for nothing?

i'm personally very interested in this and think it would be a huge improvement. I don't speak for the maintainers though

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.

None yet

4 participants