[Feat] Replace Black, Flake8, Pylint with Ruff

[gregorywaynepower]

Is your feature request related to a problem? Please describe.

Not related to a specific problem. I wanted to see if maintainers were open to a pull request replacing the following Github Actions with an implementation with the code formatter and linter Ruff:

• Python Black Formatting
• Python Flake8 Code Quality
• Python Pylint Code Quality
• Python Code Quality

Describe the solution you'd like

I was planning on reading through the yml files and re-implementing those Github Actions with Ruff to decrease runtime of Python-related Github Actions by at least one order of magnitude.

If this sort of change would be too disruptive to how y'all do your work, I'm fine with closing this.

[echoix]

I'm all-in for this. As a matter of fact, I'm already using it and have the pyproject.toml config done, and I've been slowly introducing it to the other members here. In the last weeks or so, I've been doing PRs to improve the current state of the Python code since we have way too many errors with it (we are too far off to integrate it right now).

It is obviously on my list!

You can make a PR with the changes that you think must be made, but it will probably be hanging a little bit more than usual before merging. I'll gladly merge it once the repo is ready (things I'm working on).

[echoix]

I think that the way things are aligned, it will probably end up running in the pre-commit.ci workflow.

[echoix]

The repo isn't ready for it to replace Black yet :( The diff is still a bit big.

[wenzeslaus]

Is there a way to introduce Ruff in stages, e.g., replace either Pylint or Flake8 first to test the waters or in CI only? If it is as fast as they claim, I would be be open to even add another CI action just to have a comparison first and then remove the other action(s) when we see it works well.

Did you try to run Ruff on the code? On thing is replacing the actions or pre-commit configuration, but another is configuration for the specific issues checked. The worst case here is Pylint where we have three different .pylintrc files because of the differences between the different parts of the code base (./gui/wxpython/.pylintrc, ./.pylintrc, ./python/.pylintrc).

Even with pre-commit, I end up running the tools individually and I would be happy to replace it with one tool. Maybe just using pre-commit in a better way would address my issue here. On the other hand, if it is hidden behind pre-commit, then it is the same except that everyone will appreciate having a faster tool.

The repo isn't ready for it to replace Black yet :( The diff is still a bit big.

That's sort of worrying. The code is Black compliant and Ruff claims high compatibility. So where is the issue?

[echoix]

Rereading myself, it wasn't black specifically, but more the addition of the other linters that exist but aren't applied to to our code yet. But this week, formatting with ruff (their formatting feature is only a couple months old) has some differences. I'm wondering if it's because they are a bit in advance or late for the style version. I didn't try with their "preview" setting either.
For their Black formatting, they indicate their differences and I their their justifications are pretty solid. https://docs.astral.sh/ruff/formatter/black/
I observed for example some differences like this one https://docs.astral.sh/ruff/formatter/black/#implicit-string-concatenations-in-attribute-accesses
I was telling myself to try again I'm at the end of January for the Black's 2024 style, or try with black preview (as it is pretty much finished for 2024 now) to see what is missing.

Overall, their testing is quite impressive, all with the rust attitude of correctness, and it shows. And them having full time employees working on it meant it progressed so much in the last months, like since this summer.

For using it progressively: It's quite trivial to switch, especially if the config can be set in pyproject.toml. We can for example, either only enable the rules of a certain tool, or ignore all the rules from another tool (they have different prefixes, it isn't mixed all together, they stay separated as different tools). For flake8, there was a tool that in an instant converted the flake8 config file to a ruff section in pyproject.toml. See my temporary experiments here https://github.com/echoix/grass/blob/abfb48e2cbae3e0890533e561c24b82a386f31b6/pyproject.toml. For (pure) Flake8, the rules numbers are the same (they don't compete with other linters), so all ignores work the same, and all inline ignores work the same. They don't support flake8 plugins, but they implemented rules from at least 53 most popular flake8 plugins, all without adding nothing else.

Seriously, running ALL the tools (even some we don't use now), on ALL the code base (without ignoring any files), is instant. No apparent delay. It's really crazy. And installing from pip is also real fast, probably since it is a single pre-compiled binary (it's all written in rust).

[echoix]

I imagine that adding a ruff job in the python code quality could be integrated and required (or not) in less than an hour with the setup suggested of #3320. We'd see how faster one completes compared to the other.

[gregorywaynepower]

Since you're already on the case @echoix , want me to close this or leave this open?

[echoix]

Leave it, the PR that would implement it would mark that it closes this issue.

[wenzeslaus]

For their Black formatting, they indicate their differences and I their their justifications are pretty solid.

Yes, that's pretty good (at least on paper). I was concerned about the experience you had.

[gregorywaynepower]

I want to add that if we do end up changing the workflow to using Ruff, we can change the Submitting Python Code documentation so new contributors are made aware of the new workflow.

[wenzeslaus]

I agree, but even more than that. We can throw away big part of the submitting guidelines even now and simply replace it by an instruction to use the appropriate tool because that takes care of the guidelines in a better way. The new guidelines can then focus on more advanced things and things specific to GRASS GIS (interfaces, formats, messages, ...). Not directly related, but somewhat relevant here, we would like to move the guidelines from Trac wiki to a Markdown file in the repo.

Do you want to open a PR for adding a Ruff GitHub Action workflow to see how Ruff would work?

[gregorywaynepower]

I agree, but even more than that. We can throw away big part of the submitting guidelines even now and simply replace it by an instruction to use the appropriate tool because that takes care of the guidelines in a better way. The new guidelines can then focus on more advanced things and things specific to GRASS GIS (interfaces, formats, messages, ...). Not directly related, but somewhat relevant here, we would like to move the guidelines from Trac wiki to a Markdown file in the repo.

Do you want to open a PR for adding a Ruff GitHub Action workflow to see how Ruff would work?

@wenzeslaus I've been a bit strapped for time as of late, so it may be a while before I look at this.

@echoix What's your bandwidth looking like? I also don't want to step on your toes if you've already got a PR in the works.

[echoix]

Nope, I've got nothing started, you have all the opportunities to do it! I'm stuck with an other bigger set of changes, and this was a step too much.

If you are working on it right now, you are free to completely replace black with ruff format. I'd review what it takes. Our black is already outdated, and updating it will cause churn, so we might as well skip it.

Probably a PR with only the code formatted and another one for the actual configuration might be easier to review and we would be able to ignore only the commit of the code formatting.

[wenzeslaus]

Probably a PR with only the code formatted and another one for the actual configuration might be easier to review and we would be able to ignore only the commit of the code formatting.

That requires disabling the CI (in one way or another), so in the past, we (I?) did the changes in one PR (Black check version increase and code update). The ignore revisions file warns about this and I think you see a note about ignore revs being applied in GitHub interface. The value is put on well-working blames in the code and no workarounds for CI over straightforward blame for workflow files. Let me know if this doesn't make sense to you.

[echoix]

Ruff is now configured and checked in CI and pre-commit. All the rules that weren't already passing were added to the ignore list individually. So, fixes to one rule can be made once and remove the exclusion. This way, we can't regress more than what we have.

Ruff formatting isn't used for now. It is not possible to use both ruff format and black at the same time, they have little differences that fight against each other.

I still think this can be closed.