xarray-contrib/xncml

ENH: Migrate to pyproject.toml and ruff

bzah opened this issue · 7 comments

bzah commented

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 commented

Ok for pyproject.toml.

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

huard commented

@Zeitsperre Thoughts?

bzah commented
  • 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 commented

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

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 commented

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.

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.).