repo: format Python files with ruff and enforce in CI

[pdgendt]

Moving forward I would like the Python formatting to be enforced in CI, this ensures there's little to no discussion needed on how code should be formatted in PRs.

In Zephyr, the ruff formatter is enforced in CI for new files. As there are a lot of existing Python files there, it made sense to not format in one go (350+ excluded files ATM).
In West there aren't a lot of files (16 need formatting changes), so that's why I propose to apply this once.

--preview is enabled to have Github Action annotations.

Can we just bite the bullet here, please?

EDIT cc:

• app: config: Add support for appending to the config string #768 # unrelated PR where a related discussion happened
• [RFC] scripts: ci: check_compliance: Add python lint/format check zephyr#81070

[codecov]

Codecov Report

❌ Patch coverage is 85.83333% with 51 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.52%. Comparing base (594a789) to head (241f897).
⚠️ Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
src/west/app/project.py	84.80%	19 Missing ⚠️
src/west/app/main.py	83.60%	10 Missing ⚠️
src/west/manifest.py	92.43%	9 Missing ⚠️
src/west/commands.py	68.75%	5 Missing ⚠️
src/west/log.py	0.00%	3 Missing ⚠️
src/west/app/config.py	85.71%	2 Missing ⚠️
src/west/configuration.py	88.88%	2 Missing ⚠️
src/west/util.py	75.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #865   +/-   ##
=======================================
  Coverage   84.52%   84.52%           
=======================================
  Files          11       11           
  Lines        3366     3366           
=======================================
  Hits         2845     2845           
  Misses        521      521           

Files with missing lines	Coverage Δ
src/west/util.py	87.87% <75.00%> (ø)
src/west/app/config.py	93.61% <85.71%> (ø)
src/west/configuration.py	84.91% <88.88%> (ø)
src/west/log.py	36.26% <0.00%> (ø)
src/west/commands.py	90.33% <68.75%> (ø)
src/west/manifest.py	95.33% <92.43%> (ø)
src/west/app/main.py	74.67% <83.60%> (ø)
src/west/app/project.py	80.36% <84.80%> (ø)

[thorsten-klein]

I was doing exactly the same 👍🏻
I have added poe format and poe format_check, with the difference that poe format really does the formatting.

And in which CI does this run?
I have added it in test.yml to run only on 1 combination (like lint / types)

Ref: thorsten-klein/west@main...thorsten-klein:west:run-ruff-format

[pdgendt]

I was doing exactly the same 👍🏻 I have added poe format and poe format_check, with the difference that poe format really does the formatting.

Sure

And in which CI does this run? I have added it in test.yml to run only on 1 combination (like lint / types)

Whoops, you are right

[pdgendt]

I know @marc-hb is probably opposed to doing this, but this repository isn't actually that large so the "churn" from my point of view is acceptable.

The introduction of the CI format check in #766 was a feeble attempt in trying to do it in smaller bits, but not enforcing it, is practically the same as not moving forward IMO.

[mbolivar]

no objections to the idea from me

[pdgendt]

no objections to the idea from me

But no approval either? 🥲

[carlescufi]

I must say I am not a fan of some of the formatting choices made by ruff, but it certainly beats leaving it to the implementer.

[mbolivar]

@pdgendt I basically agree with @carlescufi and since you are the maintainer I believe it's your call :)

[marc-hb]

In Zephyr, the ruff formatter is enforced in CI for new files. As there are a lot of existing Python files there [in Zephyr], it made sense to not format in one go (350+ excluded files ATM).

I wonder why the file by file approach is working in Zephyr but not here.

I took a quick look at the codestyle changes in this PR and I spotted none that made the code more readable. Unlike C or C++, Python already enforces indentation rules alone, that's probably why.

Moving forward I would like the Python formatting to be enforced in CI, this ensures there's little to no discussion needed on how code should be formatted in PRs.

I don't remember any past discussion on how the code should be formatted in this west repo. Maybe that has been happening in Zephyr but not here.

so that's why I propose to apply this once.

It's never "once" and that is always the problem with this type of enforcement. Massive git pollution "once" + continuous git pollution and conflicts going forward.

I gave this a quick try and what I feared happened exactly:

$ git fetch origin pull/865/head
git checkout FETCH_HEAD

$ uv run poe format
21 files left unchanged

$ uv run ruff format
5 files reformatted, 16 files left unchanged  # !!!

$ git diff --stat
 src/west/app/project.py       |  16 +++++-----
 tests/test_config.py          |  18 ++++++-----
 tests/test_manifest.py        |   6 ++--
 tests/test_project.py         | 138 +++++++++++++++++++++++++++++++++++++++++++++-------------------------------------
 tests/test_project_caching.py | 124 +++++++++++++++++++++++++++++++++++++++++--------------------------------
 5 files changed, 168 insertions(+), 134 deletions(-)

So, this looks exactly like the mistake it would be.

[marc-hb]

How is this related to ruff?

[marc-hb]

Just like for any other PR: what problem does this solve?

• Time-consuming and pointless codestyle discussions? Not in this particular repo.
• Better readability? Not in this particular repo.

So far, the only problem solved by this PR seems to be the inability to run uv run poe format without thinking. Except:

• Running uv run poe format without thinking does not seem to help readability much.
• Running that command does not seem to work without thinking, see above. It does not work because - like with every other formatting tool - the output depends on the version, configuration, environment, phases of the moon and what not. It took me less than 3 minutes to demonstrate that - imagine how much more difference I would have found if I had tried harder.

Speaking of configuration, is there maybe some codestyle ugliness(es) that harm readability and that we can agree to forbid? That would be defining a specific problem that needs solving. I would not mind some specific, well defined and more stable subset of formatting rules. Enforcing a smaller subset would reduce both initial and ongoing git pollution dramatically. That would be a sweet spot and actual trade-off between readability and git pollution.

Unrelated PR where that discussion already happened (it was off-topic there)

• app: config: Add support for appending to the config string #768

[pdgendt]

In Zephyr, the ruff formatter is enforced in CI for new files. As there are a lot of existing Python files there [in Zephyr], it made sense to not format in one go (350+ excluded files ATM).

I wonder why the file by file approach is working in Zephyr but not here.

It doesn't, most of the initial files are still excluded, documentation was updated recently so let's hope that changes. But without enforcement, I doubt it.

I took a quick look at the codestyle changes in this PR and I spotted none that made the code more readable. Unlike C or C++, Python already enforces indentation rules alone, that's probably why.

Some changes are subtle, some aren't. I think is is more readable, especially arguments split over newlines.

Moving forward I would like the Python formatting to be enforced in CI, this ensures there's little to no discussion needed on how code should be formatted in PRs.

I don't remember any past discussion on how the code should be formatted in this west repo. Maybe that has been happening in Zephyr but not here.

Because, for my PRs, I manually tweaked styling to be consistent with surrounding code, which I hate to do. And I assume other west contributors did the same.
I created this PR right after a PR discussion from @thorsten-klein. He's an active member here, and I think also prefers enforcement of styling.

Besides C and Python, I work with Go and dabble with some rust and Zig, these all have formatters in the language tooling itself. You might not agree with some of the style choices, but there's no debate, and I prefer that.

so that's why I propose to apply this once.

It's never "once" and that is always the problem with this type of enforcement. Massive git pollution "once" + continuous git pollution and conflicts going forward.

I gave this a quick try and what I feared happened exactly:

It did change because preview was enabled for Github Action annotations in CI. I will make that enabled for the project rather than for some CI tasks.

Just like for any other PR: what problem does this solve?

• Time-consuming and pointless codestyle discussions? Not in this particular repo.

Yes, in this PR, there just isn't/wasn't that much traffic in west. This PR itself, for example, and I'll keep advocating for making this change.

• Better readability? Not in this particular repo.

Agree to disagree

So far, the only problem solved by this PR seems to be the inability to run uv run poe format without thinking. Except:

Yes, exactly that, I don't want to spend time and brainpower on formatting.

• Running uv run poe format without thinking does not seem to help readability much.
• Running that command does not seem to work without thinking, see above. It does not work because - like with every other formatting tool - the output depends on the version, configuration, environment, phases of the moon and what not. It took me less than 3 minutes to demonstrate that - imagine how much more difference I would have found if I had tried harder.

How is that a bad thing, things evolve, tools improve.

Speaking of configuration, is there maybe some codestyle ugliness(es) that harm readability and that we can agree to forbid? That would be defining a specific problem that needs solving. I would not mind some specific, well defined and more stable subset of formatting rules. Enforcing a smaller subset would reduce both initial and ongoing git pollution dramatically. That would be a sweet spot and actual trade-off between readability and git pollution.

Readability is not the only concern here, ease of development and spending your time where it matters: the content instead of the styling. Git is just another tool, and I don't consider styling changes to be git pollution. We've had this argument before, the git history is just that, a log of what happened. It isn't sacred.

[pdgendt]

Unrelated PR where that discussion already happened (it was off-topic there)

• app: config: Add support for appending to the config string #768

Yes, but my arguments were the same as they are now.

[marc-hb]

Because, for my PRs, I manually tweaked styling to be consistent with surrounding code, which I hate to do. And I assume other west contributors did the same.

Don't tweak and don't assume?

Yes, exactly that, I don't want to spend time and brainpower on formatting.

Then just don't? Just stop obsessing about whitespace. Let it go. We already have a relatively strict linter, that's plenty enough codestyle.

Besides C and Python, I work with Go and dabble with some rust and Zig, these all have formatters in the language tooling itself.

Python is very different because it already enforces indentation.

You might not agree with some of the style choices, but there's no debate, and I prefer that.

I don't really care about whitespace in Python; only you do. The paradox is: you started this debate which I never heard of before you became maintainer, and now it must be fixed. This is really inventing both the problem and the solution.

Just like for any other PR: what problem does this solve?
Time-consuming and pointless codestyle discussions? Not in this particular repo.

Yes, in this PR, there just isn't/wasn't that much traffic in west.

Very ironically, not even in this PR has there been any discussion about the style itself. We're arguing about the enforcement of whitespace rules no one seems to even have looked at. Among others, this means no one knows how often the rules will change. PS: futile codestyle discussions do not necessitate significant traffic.

How is that a bad thing, things evolve, tools improve.

As mentioned many times before, the bad thing is a regular stream of whitespace commits that constantly break git for a very low value.

Readability is not the only concern here, ease of development and spending your time where it matters: the content instead of the styling

Then just stop obsessing about whitespace in Python and problem fixed. We already have relatively strict linter rules, that's plenty enough codestyle already.

Git is just another tool, and I don't consider styling changes to be git pollution.

Agree to disagree.

We've had #800, the git history is just that, a log of what happened. It isn't sacred.

You apparently never had to perform complex backports, bisects or other branch maintenance in a very large project. Neither to git blame far back in time across multiple refactorings.

As far as repo sizes and number of branches go, west is just a "toy" git repo. So these problems won't be too bad. But they will happen; that took me only 3 min to demonstrate.

It did change because preview was enabled for Github Action annotations in CI.

I don't understand. I ran two slightly different formatting commands locally and the output was different already. How was that CI or github related?

[thorsten-klein]

I’m happy to learn from you! Maybe I’m being too naive.
Where does this caution and rejection come from?

Just like for any other PR: what problem does this solve?

From my experience as a contributor, it’s very helpful when a project enforces code formatting in its CI pipeline.
This simplifies things — contributors don’t have to worry about each project’s individual formatting preferences.
Locally they just run a format task using the project’s configuration, and that’s it.

It also benefits you, the maintainers:
PR reviews can focus on actual content rather than formatting details (as Pieter did on my recent PR).

Of course, introducing enforced formatting will create a large initial PR, but as Pieter mentioned, that’s a necessary bullet to bite.

I see the risk as quite low for this project:

• there are only a few open PRs that would need rebasing
• a small number of active feature branches that might face conflicts
• and no feature backports to older west LTS versions

Running that command does not seem to work without thinking

I’d say: it depends.
A project needs to maintain its configuration for each tool it enforces in CI — that’s some overhead, yes.
But once that’s in place, contributors can run the tool locally (in any version) with the shared config.
In the end, a CI pipeline enforces consistency (if tool versions differ), since the formatting task must pass with a specific tool version.

[henrikbrixandersen]

Where does this caution and rejection come from?

I believe the main issue with introducing tree-wide style changes (and keeping up with new style changes along the way) is what was described as "Git pollution" above. Each line changed only for "style" ends up with a "polluted" Git history, making it harder to find the history of any functional changes in that particular line (i.e. using git blame will just show a lot of lines being changed for style, not functionality).

That being said, I don't have a strong opinion on this particular PR, mostly since I rarely look at the West source code.

[pdgendt]

I don't understand. I ran two slightly different formatting commands locally and the output was different already. How was that CI or github related?

Yes, this has been updated, and shouldn't happen anymore.

All comments on ignoring styling as we already have indentation

Put a tab instead of spaces, all tests will succeed and CI won't complain. But depending on your editor settings, this will look bad.
This likely never happens, because most editors will already expand to tabs, so tooling helps 👍.

I don't really care about whitespace in Python; only you do. The paradox is: you started this debate which I never heard of before you became maintainer, and now it must be fixed. This is really inventing both the problem and the solution.

Then just stop obsessing about whitespace in Python and problem fixed. We already have relatively strict linter rules, that's plenty enough codestyle already.

Yes, I care, and yes I want to have this debate, because I work on features/bugfixes/improvements for west. Why is that an argument for not having this discussion? I also take responsibility in maintaining it.

Very ironically, not even in this PR has there been any discussion about the style itself. We're arguing about the enforcement of whitespace rules no one seems to even have looked at. Among others, this means no one knows how often the rules will change. PS: futile codestyle discussions do not necessitate significant traffic.

The ruff formatting style is already "part of" the repository, from #766. And I do have a preference here, and really like the ruff formatting Philosophy.

You apparently never had to perform complex backports, bisects or other branch maintenance in a very large project. Neither to git blame far back in time across multiple refactorings.

As far as repo sizes and number of branches go, west is just a "toy" git repo. So these problems won't be too bad. But they will happen; that took me only 3 min to demonstrate.

Please don't make assumptions you can't prove, the contrary is true. In the long run it's actually easier on a codebase with consistent formatting.
This repository is small indeed, exactly part of the reason why I think we should do this now.
The difference you generated and found has been dealt with, won't happen again.

[marc-hb]

[replying to different people at once, sorry]

This simplifies things — contributors don’t have to worry about each project’s individual formatting preferences.

The even simpler thing is not to run any format tool at all. That's how west started and everything was fine for years. I'm really running out of way to rephrase this basic fact; this is getting a bit crazy. And again: Python is different. Not all languages are the same / one size does not fit all., there is already a linter, etc. etc. See above if you're interested.

Locally they just run a format task using the project’s configuration, and that’s it.

"That's it" only when the formatting rules and implementation never change. Otherwise, a small rule or policy change or small fix in the formatter can cause thousands of lines of git pollution. I'm running out of ways to rephrase that other, basic fact too.

Very long story very short, this is a trade-off that depends on two things:

1. How much git is required in the repo
2. How stable the formatter is.

Now that is "it"!

I agree git has never been needed much in this repo; funny enough. I don't know how stable ruff formatting is but I like https://docs.astral.sh/ruff/formatter/#philosophy too, thanks for the link (and sorry if I missed it earlier)

This repository is small indeed, exactly part of the reason why I think we should do this now.
The difference you generated and found has been dealt with, won't happen again.

Thanks, I would have expressed nowhere near as much reservation if this has been only presented as such a calibrated trade-off. But what I found baffling is the extensive repetition of "We've always done it like this." dogma and "Whitespace matters, git does not." in the git repo of a tool managing... git repos!

So these problems won't be too bad. But they will happen; that took me only 3 min to demonstrate.

Please don't make assumptions you can't prove, the contrary is true.
The difference you generated and found has been dealt with,

Come on, I just ran something format and git diff... I still don't understand what happened there (and I confirm your magic somehow fixed it), but please admit this really looked like evidence.

it won't happen again.

I am marking your words! :-)

[thorsten-klein]

Each line changed only for "style" ends up with a "polluted" Git history, making it harder to find the history of any functional changes in that particular line.

Thank you! This really helped me understand the scepsis 👍🏻

The even simpler thing is not to run any format tool at all.

That’s exactly how I prefer to develop software in a small team.
However, as a project grows, these tools usually become necessary — that’s at least my experience (no matter if C, C++, and Python projects).

I hope, that in future contributors like me will continue to open PRs.
If yes, it’s unrealistic for you to manually comment on all formatting issues in every PR — and you probably wouldn’t want to accept inconsistent formatting either. That’s why many projects introduce tools for such simple tasks that be automated.

Maintainers don't want to annoy contributors with this, but they want to make their own life easier.
Even if those tools may not bring short-term benefits, but in the mid- to long-term they help maintain consistency and reduce review overhead.

It seems that the same has happened in Zephyr, where ruff runs as part of the CI compliance check script.
Not long time ago ruff was introduced for some existing directory and many files were re-formatted:
zephyrproject-rtos/zephyr#81092

I can’t judge whether now is the right time to introduce a code formatter in west, but to me it feels like a reasonable step forward.

1. How much git is required in the repo
2. How stable the formatter is.

I’m still relatively new to the Zephyr community, but from what I’ve seen in their scripts/ci/check_compliance.py, a number of tools are already integrated.
@marc-hb Do you have any insights or feedback from Zephyr maintainers about how often tool updates lead to git pollutions?
Would be interesting to know, especially since they use so many different tools and code checkers.