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

ENH: Migrate to pyproject.toml and ruff #69

Open
bzah opened this issue Feb 7, 2024 · 7 comments · May be fixed by #76
Open

ENH: Migrate to pyproject.toml and ruff #69

bzah opened this issue Feb 7, 2024 · 7 comments · May be fixed by #76

Comments

@bzah
Copy link
Collaborator

bzah commented Feb 7, 2024

It would be nice if xncml would use:

  • pyproject.toml instead of setup.py.
  • ruff format instead of black
  • ruff check instead of flake8 and isort

Optionally, it would also be nice (IMHO) to use a src layout structure instead of a flat layout: https://packaging.python.org/en/latest/discussions/src-layout-vs-flat-layout/

[x] I'm willing to make a PR to make these changes.

@huard
Copy link
Collaborator

huard commented Feb 7, 2024

Ok for pyproject.toml.

For the other points, could you provide a rationale for the changes?

@huard
Copy link
Collaborator

huard commented Feb 7, 2024

@Zeitsperre Thoughts?

@bzah
Copy link
Collaborator Author

bzah commented Feb 7, 2024

  • ruff have gained a lot of popularity and both the linter (ruff check) and the formatter (ruff format) are being used in project like pandas now
  • ruff is one tool to rule them all (and in clean code bind them):
    • It does not re-invent the wheel, the ruff formater output would be certainly identical to black outputs and the lint rules were picked up from flake, flake plugins and other linters.
    • having one tool eases the configuration and helps avoiding conflicts between different tools.
    • This also eases the maintenance burden by having to update only one.
    • It's much faster than any of the other linter or formatter.
  • I also had a good experience migrating from black/flake/other to ruff on a another project and did not face any major issue.

As for the src layout, it's main purpose is to avoid importing the xncml python module from the sources instead of using the one installed in the python environment.
The Python Packaging Authority recommends using this layout.

@huard
Copy link
Collaborator

huard commented Feb 7, 2024

Thanks, good for me, but I'd like to have Trevor's thoughts on this.

@Zeitsperre
Copy link
Collaborator

Good idea @bzah

We currently use a mixed implementation of ruff with a few other tools (ruff for all major linting checks, flake8 exclusively for checks not implemented in ruff, isort and black as formatters, mypy for some typing checks). I'm not sure how best to migrate more things to ruff. Maybe I could get a handle on that here.

For the layout proposition, I'm all for it. It's just a safer way of organizing the code. Not much is needed when it comes to configuration.

If you're interested in starting from an existing template, I've put a fair amount of work into our cookiecutter (https://github.com/Ouranosinc/cookiecutter-pypackage). This includes pyproject.toml support, tox, GitHub Workflows, documentation translations (optional), etc. If you generate it with cruft, we'll even be able to update the boilerplate code here from time to time (like other Ouranos projects).

@bzah
Copy link
Collaborator Author

bzah commented Feb 7, 2024

Thanks @Zeitsperre.
Out of curiosity, which checks are not yet in ruff that justify keeping flake8 ?

Thanks for the cookiecutter. One thing I see that diverges from it in xncml is the use of setuptools_scm. It generates a version for the package using git (the latest tag is used I think) and may require setuptool.
We could get rid of it, but this would change the release process of xncml.

@Zeitsperre
Copy link
Collaborator

As far as I can tell, there is no ruff implementation for flake8-alphabetize (not very important, but nice to have) nor for flake8-rst-docstrings (very useful for checking docstrings). The flake8 configuration for the cookiecutter is entirely found in .flake8 (note that all other flake8 checks are disabled), and I'm waiting to drop that file once those features are implemented.

There's a modified implementation of this cookiecutter to support setuptools and babel in https://github.com/Ouranosinc/xscen. We could modify it to support the existing setuptools_scm. Alternatively, we could do some shopping around for a relatively simpler cookiecutter recipe with all that we currently want and add the bells and whistles we're looking for (e.g. GitHub Workflows, Security scans, etc.).

@bzah bzah linked a pull request Apr 12, 2024 that will close this issue
6 tasks
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 a pull request may close this issue.

3 participants