all: GitHub Action to lint Python code with ruff.

[cclauss]

Ruff supports over 500 lint rules including
flake8, isort, pylint, and pyupgrade and is written in Rust for speed.

The Action uses minimal steps to run in 7 seconds, rapidly providing contributors with intuitive
GitHub Annotations.

[github-actions]

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:    +0 +0.000% PYBV10
        rp2:    +0 +0.000% PICO

[stinos]

Idea is ok, but just like for #10911 this should have a separate configuration file so users can run the check locally (manually, or as part of a pre-commit hook).

Perhaps it's time to adopt pyproject.toml? codespell, ruff, ... all support it

[cclauss]

Added pyproject.toml for both codespell and ruff

I used ini2toml to convert codespell.cfg to toml and then validate-pyproject to verify the results.

[stinos]

That's pretty neat.

@dpgeorge what do you think, go for pyproject.toml or not?

[dpgeorge]

what do you think, go for pyproject.toml or not?

Yes that looks good, to keep config all in one file. But:

• does pyproject.toml work with IDEs, ie can they find it and use it?
• does pyproject.toml need to be placed in the root of the repo, or could it be moved to tools/? I don't mind having it in the root if that makes it work with IDEs, just wondering if we have options here or not.

[dpgeorge]

Will these ignores gradually be removed once the problems are fixed?

[cclauss]

✅

[cclauss]

Yes, #10979, #10991, #10992, #10993, #10994, ... Small moves.

[dpgeorge]

Small is good. As long as they are forward steps!

[cclauss]

pyproject.toml is the recommended direction from the Python Packaging Authority to replace setup.cfg + setup.py + requirements.txt, etc. All the IDEs and other Python tools (except flake8) are moving towards it. I have never seen one in a non-root directory so my recommendation would be to keep it in the root until we can find a counterexample.
https://pip.pypa.io/en/stable/reference/build-system/pyproject-toml Lists the PEPs and history.

[stinos]

does pyproject.toml work with IDEs, ie can they find it and use it?

I'm not sure what 'works with IDEs' entails. Maybe because I've never used full-blown Python IDEs, but for everything else the way it works is that the 'host aplication' i.e. text editor just invokes the tools (ruff/isort/black/pylint/...) with the working directory set appropriately and by passing the path to a directory or file to process, and then the tool itself takes care of finding pyproject.toml when it's in a directory up from that path. And if it's not, the host application normally has a way to configure the tool so the user can pass it --config ${projectroot}/tools/pyproject.toml or something like that. So in essence there's no difference of calling the tool from the commandline or fom another application, ime.

That being said it is obviously a lot more convenient and expected to have pyproject.toml (and others like .editorconfig etc) in the project root.

[cclauss]

What is the correct Commit message format for this kind of utility PR that may touch multiple parts of the codebase?

[codecov-commenter]

Codecov Report

Merging #10977 (0dce0ad) into master (4376c96) will not change coverage.
The diff coverage is n/a.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@           Coverage Diff           @@
##           master   #10977   +/-   ##
=======================================
  Coverage   98.50%   98.50%           
=======================================
  Files         155      155           
  Lines       20549    20549           
=======================================
  Hits        20241    20241           
  Misses        308      308           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[dpgeorge]

What is the correct Commit message format for this kind of utility PR that may touch multiple parts of the codebase?

Best thing to do is check existing commit messages. Maybe "top: Add linter ..." or "all: Reformat ...".

[cclauss]

I just spotted this https://beta.ruff.rs/docs/configuration/#pyprojecttoml-discovery

[Josverl]

I did a quick check om my IDE of choice : VScode + Pylance

• There is a ruff add-in
• It does read the ruff settings from pyproject.yml
• uses an LSP , and coexists with other LSPs , although it 'barges in' and presents fixes for warnings generated by other LSPs
• 🪳The ruff add-in did not* manage to find Python installed in the path on Windows, and thus failed to run.
• In some cases the quick fixes offered in the IDE seem different than the ones the command line makes (adding a \ versus a r-string )

For code quality : I have not seen any thing yet that was flagged by ruff, and not flagged by pyright/ pylance

Personally I find the configuration quite hard to read for a tool that aims to improve code quality :

extend-select = ["C9", "PLC","W605"]

[cclauss]

@Josverl I believe that the place for ruff-vscode issues is at https://github.com/charliermarsh/ruff-vscode

Perhaps the raw string vs. \\ fixer thing is caused by two different versions of ruff on your machine?

• Add autofix for W605 [InvalidEscapeSequence] astral-sh/ruff#1361

[Josverl]

@cclauss , I don't think I have multiple versions (bare container image with one install of ruff).
There may be a version / behavior's difference between the version used in the Action (The extension ships with ruff==0.0.252) and the latest version installed in the ruff workflow. Happens to be ruff-0.0.254 - but can be different tomorrow.

Just to be clear, I think :

• both fixes provide an improvement over the original code, so either is good
• there is value to use ruff in the the inner dev-loop (IDE , pre-commit hooks) as it is fast and does the job
• all tools in CI/CI must be pinned to a specific version and configuration that is the same to the inner loop( to simplify review / merging and to avoid frustrations for all involved). Even better if that version is in a standard requirements-.txt format so it can be re-used outside that workflow.
• the configuration should be documented, especially for a tool such a ruff which by nature has an overlap with many other tools. I suggest adding to the code conventions
• Pyproject.toml is a good place to store the config, but some of the configuration will need to be repeated ( e.g. [toolblack] has its own line-length setting,
• we should be careful with automatic fixes/refactoring on sections of code that have no automated unit or integration tests to avoid being caught with beautiful but non-working code.

[cclauss]

all tools in CI/CI must be pinned to a specific version

I do not sense this is currently the case for uncrustify and psf/black. I was unable to find the usual dependency config files like requirements.txt, setup.cfg, setup.py, or pyproject.toml, so I am unclear how the versions of dev dependencies are currently pinned. It is interesting that these tools are not run in pre-commit which would be the natural place for them to be pinned and run both locally and in CI.

[Ruff] has an overlap with many other tools.

I am unclear which other tools this refers to. Ruff was designed to be highly compatible with psf/black.

How about this approach:

• Do not encourage the use of ruff-vscode until it simultaneously releases with ruff.
• Agree that line-length is currently different under psf/black (best effort formatter) and ruff (rules-based linter) but our goal is to make them the same.
  • The difference because the former is guidance for a tool that formats code but not comments, docstrings, etc., and is deliberately timid about reformatting strings while the latter can be used as a trigger for one or more good-first-issues for interested contributors to shorten long lines that psf/black does not autofix.
• Let's not use ruff --fix in automated pipelines until we have more experience with those fixers.
• Modify pre-commit to run psf/black and ruff both locally and in CI.

[Josverl]

Claus, I'm not trying to make this a battle between one or the other, but in this PR you are proposing a change.

My thoughts as I outlined above , are just that , my thoughts on that proposed change. I'm no more a contributor than you are , I we both trying to help things along.
While I do have a few years of experience building software, reviewing docs and code, and deployments, but still I just know a little. So please only take from my comments what you like, and feel free to lay aside what doesn't make sense. I'd be happy if you try to explain what or why something does not make sense, as that way I could learn a little.

I am unclear which other tools this refers to.

• You started this PR with a reference that ruff is a rewrite of rules implemented by other tools. flake8, isort, pylint, and pyupgrade . So from a functional level there is overlap. Overlap is good , it makes roofs watertight. But is software tooling this may need to be address specifically , especially as it is common ( almost a certainty) that different tools will interpret / implement a principle or rule slightly different.

Do not encourage the use of ruff-vscode until it simultaneously releases with ruff.

• I fail to see what that would accomplish.
• If you prefer unspecified dependencies compared to pinned dependencies , then just document explicitly that ruff@latest will be used. That allows IDE users to find / configure an extension/add-in with the same config or switch to/install a specific version.
  For black just using @latest works, as the behavior's is stably reproducible , and the change rate is not high
  With ruff there seem to be multiple changes in the last 2 patches (3 weeks) , one of which i happened to run into in my first test. perhaps that is just a fluke. I really do not know.

Line length

Thanks , would make sense documenting that that as a comment or otherwise for the less ruff-knowledgable such as myself.

pre-commit.

Black is already in the pre-commit hooks, but still could benefit from a [tools.black] config in pyproject.toml to make it run more consistently.

repos:
  - repo: local
    hooks:
      - id: codeformat
        name: MicroPython codeformat.py for changed files
        entry: tools/codeformat.py -v -f
        language: python
...

[dpgeorge]

@cclauss can you please rebase this on latest master, which now includes pyproject.toml?

[dpgeorge]

I think these comments can be removed. Also, the main branch here is called master.

Maybe just:

on: [push, pull_request]

[cclauss]

I find that on: [push, pull_request] will make duplicate runs only for people who have write permissions to the repo (i.e. maintainers) so I tend to use

on:
  push: [master]
  pull_request: [master]

but let's keep it simple given the speed of these runs.

[dpgeorge]

I'd suggest a more descriptive title, eg Python code lint with ruff

[dpgeorge]

Can these exceptions be removed now that those files are fixed?

[stinos]

Shouldn't this match black's line length (99) ?

[cclauss]

The 99 is aspirational whereas the 337 is actual.

psf/black is unwilling to wrap all lines (comments? docstrings? strings?) so those lines need to be wrapped manually.

Ideally, both should be 99 once some manual work is done.

% ruff --select=E501 --line-length=99 --show-source .

[ ... ]
Found 96 errors.

[stinos]

Hmm then why ruff says "Ruff is compatible with Black out-of-the-box, as long as the line-length setting is consistent between the two." Can you point to an example where ruff complains? There's no way to make it ignore comments for instance (should that be the culprit)?

[cclauss]

@charliermarsh, @dhruvmanila This question has come up a few times in my travels...

Do you have a better explanation? Should I create a PR to the ruff docs to improve the explanation there?

I am OK with the behavior of the two tools as they are because I believe that the goals of a formatter are different than the goals of a linter.

[charliermarsh]

Hmm, yeah, I'd explain it as: Black makes a best-effort attempt to wrap lines under the specified line length, but doesn't guarantee that all lines will be under the specified line length. But Ruff's rule doesn't discriminate, it just flags all lines over the limit.

You're right that in this particular case, we do arguably "disagree" with Black, depending on how you look at it. Projects with Black enabled could choose to disable this rule entirely if they're happy with Black's line-length handling, though in my personal opinion it still makes sense to enable it. A few examples of code that Black won't wrap, but Ruff will flag:

# Here's a really long comment. Black doesn't wrap comments at all. It wraps docstrings, but it
# doesn't wrap comments.


x = here_is_a_really_really_really_really_really_really_really_really_really_really_long_function_name(
    1, 2, 3
)

[dpgeorge]

Thanks @charliermarsh for the explanation.

Sounds like we need to keep this at 337, for now. Maybe in the future we can tidy up those scripts that go over the 99 length Black limit, so this number here can be reduced.

[stinos]

Ok clear.

[andrewleech]

I've only recently started using ruff with micropython projects but am definitely a big fan!

You might also want to add the setting builtins = ["const"], I'm surprised it's not already failing tests due to this?

[cclauss]

Good one! We are currently ignoring F821 undefined names which is a shame because it is a great way to find bugs.

% ruff --select=F821 --statistics .
Before builtins = ["const"] --> 479 F821 Undefined name Pin
After builtins = ["const"] --> 448 F821 Undefined name Pin

Looking at the output of ruff --select=F821 ., should more of these be added to builtins?!?

[andrewleech]

Ah, yes as an aside I dislike the old "error number" approach of most linters, ruff included. I saw you had excluded error codes but didn't know what they were! I've been working with mypy too in my project and it has descriptive errors instead which is much more readable.

But yes I'm sure there likely are more builtins worth registering... that being said most things come from imports, not new "untraceable" built-in terms like const (which technically you can import from micropython, but don't have to)

If the rest of those errors are from imports that ruff can't introspect, then we might be stuck ignoring them until there are type stubs officially available for all/most built-in libraries like machine etc.

[dpgeorge]

All uses of const should be doing from micropython import const at the start of the file. So I think we need stubs for C-based modules like machine, as @andrewleech says.

[andrewleech]

All uses of const should be doing from micropython import const at the start of the file.

Oh, I've often seen/used const without an import and assumed that was normal, just like other builtins such as print.

In my current project I'm using one of the stubs packages from https://micropython-stubs.readthedocs.io/en/main/ which is probably why I'm not seeing to many other missing import errors.

[dpgeorge]

I added an exclusion of the lib/ directory, because that contains only 3rd-party code which we have no control over. Having submodules checked out there (btstack and tinyusb) leads to many ruff errors (which aren't seen in the CI because the CI is not checking out submodules).

[cclauss]

What about something like:

builtins = [
  "add",
  "bgt",
  "const",
  "delay_off",
  "delay_on",
  "freeze",
  "include",
  "jmp",
  "label",
  "micropython",
  "module",
  "mov",
  "movt",
  "movw",
  "movwt",
  "nop",
  "pin",
  "pins",  
  "require",
  "stm",
  "strh",
  "sub",
  "wait",
]

That would drop undefined names from 479 down to 66. Are there_builtins_ to add or remove?
% ruff --select=F821 . | sort -k5

Personally, I like requiring imports because linters won't be the only ones confused about where these names are defined.

[andrewleech]

Many of those terms you've listed come from viper or inline assembly, I wonder if there's a way to exclude functions decorated as such... probably not.

Others are from manifest files, which could be excluded by file pattern perhaps?

[dpgeorge]

That's quite an ad-hoc list of built-ins, and still doesn't solve F821 completely. I would much rather use stubs.

[dpgeorge]

I've merged this because it's a good step forward, adding ruff to the CI.

Moving forward, we can fix individual errors one by one.

Also, would be great to add ruff to micropython-lib (hint, hint!).