You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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.
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.
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.
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.
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?
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.
@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.
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.
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
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.
Activity
bouweandela commentedon Feb 14, 2023
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 commentedon Feb 14, 2023
My two cents: I personally hate
blackwith 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 commentedon Feb 14, 2023
Linting does a lot more than just checking formatting
abelsiqueira commentedon Feb 14, 2023
@egpbos, only
black, or formatters in general? I saw some packages usingyapf.egpbos commentedon Feb 14, 2023
I personally prefer
yapfor 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 commentedon Feb 20, 2023
On "The bird project", AKA "Where The Flock", AKA "The one @lyashevska is leading", we decided to go with
black.yapflooks 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 commentedon May 31, 2023
Github implicitly recommends
rufffor linting in their Python CI instructions.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
isortfeature for sorting imports.egpbos commentedon May 31, 2023
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
blackinto the repo you mentioned? Do you automate things in CI or with precommit hooks? Or just run manually?abelsiqueira commentedon May 31, 2023
We use
pre-commitfor everything.pre-commit run -ain the CI or locally,pre-commit installto 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-commitcallsblack,prospector, andisort. On this project I have more hooks, including linter for markdown, and link-checker.I am also considering moving to
rufffor linting and droppingprospector(because of prospector-dev/prospector#594, for instance). I think it calls it "autofix" because it is not formatting likeblackdoes, simply fixing the linting issue. They useblackthemselves: https://github.com/charliermarsh/ruff/blob/main/.pre-commit-config.yamlBSchilperoort commentedon May 31, 2023
@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
blackwould. It does however, sort imports likeisort.egpbos commentedon May 31, 2023
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 commentedon May 31, 2023
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
use ruff instead of prospector and isort (Refs #336)
bring template code and ruff config in sync (Refs #336)
add ruff test and remove isort and prospector tests (Refs #336))
use ruff instead of prospector and isort (Refs #336)
bring template code and ruff config in sync (Refs #336)
Merge pull request #347 from NLeSC/336-use-ruff-instead-of-prospector…
use ruff instead of prospector and isort (Refs #336)
bring template code and ruff config in sync (Refs #336)
egpbos commentedon Dec 20, 2023
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.