Add formatter?

[abelsiqueira]

Should there be a formatter with an appropriate configuration file?

[bouweandela]

Yes, we recommend using those in the guide: https://guide.esciencecenter.nl/#/best_practices/language_guides/python?id=coding-style-conventions

Given the popularity of black, that would probably be the best choice.

It would be nice to add a pre-commit hook for it too #222.

[egpbos]

My two cents: I personally hate black with a passion ;) If it must be added, I would personally appreciate it if it were an optional part of the template, otherwise it would be a strong disincentive for me to use the template. Perhaps I'm a minority, though, so don't let me discourage you if it turns out many people want this.

Btw, does it make sense to use an autoformatter while also maintaining a linter config? Or can autoformatter and linter configs be unified? Just thinking about maintainability of the template.

[bouweandela]

Linting does a lot more than just checking formatting

[abelsiqueira]

@egpbos, only black, or formatters in general? I saw some packages using yapf.

[egpbos]

I personally prefer yapf or actually rather no autoformatter at all, but I don't want to impose my preference :) Just pitching in from a random dev perspective.

To be honest, I've already started having doubts about using the template for some of my projects in the past, because it feels too bloated to me. I looked into making components optional, but that's not easy with cookiecutter. I also thought about creating a separate minimalist version, but that would just create more confusion and discussion and maintenance burden that I don't think we want.

Anyway, rather than follow my philosophical tangent, let's keep the discussion practical: if you think it's useful for you, let's just add it :) People can take it out again from their projects if they prefer. At least then it's consistent with the Guide as @bouweandela pointed out.

[abelsiqueira]

On "The bird project", AKA "Where The Flock", AKA "The one @lyashevska is leading", we decided to go with black. yapf looks great for detailing how you want it formatted, and that means that I would spend a lot of time trying to customize exactly how I want it.

[carschno]

Github implicitly recommends ruff for linting in their Python CI instructions.

Ruff can be used to replace Flake8 (plus dozens of plugins), isort, pydocstyle, yesqa, eradicate, pyupgrade, and autoflake, all while executing tens or hundreds of times faster than any individual tool.

I have been using ruff for a while now, and I find it preferable over Black because of its speed and because it also has an isort feature for sorting imports.

[egpbos]

Ah, I wasn't aware that ruff can do autoformatting as well (I see it calls it "autofix"...). In lilio it's also used for linting. Not sure whether they also use the autofix functionality.

What do people think about replacing everything (i.e. linting and autoformatting) with ruff?

@abelsiqueira, @lyashevska, how have you integrated black into the repo you mentioned? Do you automate things in CI or with precommit hooks? Or just run manually?

[abelsiqueira]

We use pre-commit for everything. pre-commit run -a in the CI or locally, pre-commit install to install as a pre-commit hook, and we use https://pre-commit.ci to auto-fix PRs that don't use the pre-commit hook.

Currently, our pre-commit calls black, prospector, and isort. On this project I have more hooks, including linter for markdown, and link-checker.

I am also considering moving to ruff for linting and dropping prospector (because of https://github.com/landscapeio/prospector/issues/594, for instance). I think it calls it "autofix" because it is not formatting like black does, simply fixing the linting issue. They use black themselves: https://github.com/charliermarsh/ruff/blob/main/.pre-commit-config.yaml

[BSchilperoort]

@egpbos Ruff's autofix is for automatically fixing linting issues. Note that not all warnings/errors have an autofix available. Ruff does not reformat code like black would. It does however, sort imports like isort.

[egpbos]

Ok, so ruff is not a replacement for autoformatting, although maybe partially? Do you have a clear view on which parts can get autofixed @BSchilperoort?

I guess having config for both might be nice then.

Replacing prospector with ruff sounds like a good plan @abelsiqueira, it seems like the support level for prospector is significantly lower than ruff currently. If you do that on your project, please ping us here as well, and we can try to get it integrated in the template.

I think especially autoformatting makes sense only in pre-commit, not so much in CI where it is basically already too late. From @abelsiqueira's example, it seems very simple to include black, so let's just do that. Of course, it would be nice if we also add a test to the template's test suite that makes sure it works.

Anyone feel up to making a PR for this? :)

[BSchilperoort]

Ok, so ruff is not a replacement for autoformatting, although maybe partially? Do you have a clear view on which parts can get autofixed @BSchilperoort?

It's mostly simpler stuff. For example "none-comparison" (Comparison to None should be cond is None) has an autofix. However, rules like "line-too-long" or "mixed-spaces-and-tabs" do not have an autofix. The Ruff documentation has an overview of all rules, and if autofixes are available for them.

@egpbos @abelsiqueira do consider that when moving from prospector to Ruff, you have to choose which rules to use. Ruff has implemented many from different packages. The rulesets we use can be found here and here. By default, Ruff only enables Flake8's E and F rules

[egpbos]

Ruff now also includes auto-formatting capabilities and is as good as feature complete compared to Black: https://astral.sh/blog/the-ruff-formatter (article from October, several updated releases since then). I think that for simplicity, in the template we should just use that. It seems like people agree that it's best to add this to a pre-commit config file. Let's move that part of this issue there (#222), since I think once pre-commit is added, adding ruff formatting, black formatting and basically anything else (perhaps commented out so that people can opt-in to particular choices, see also #361) is quite trivial, it seems.