Recommended way to format python code?

, , ,
1

I’m looking for a one-stop solution to format-on-save python code, in such a way that it passes some recommended linter checks. I used ament_cmake_flake8 for format checking during test and vscode + “black” for formatting, but that fails once flake8_quotes is installed, because of black using double quotes, and flake8 insisting on single quotes.
What’s the best solution? This has been discussed about 2 years ago but I did not see a resolution. I’ve tried using ament_cmake_black instead of ament_cmake_flake8 but it’s not available on rolling.
Any suggestions?

2

There is ruff format, it is still in beta, but tires to be a black replacement, it has single quotes support.

3

Thanks, I’ll give it a try!

4

I’m using ament_black in pre-commit. It works great.

Here’s the PR to add support.

2 Likes
5

I tried that and it worked great, but failed for rolling because ament_black is not supported there (yet?). I’m trying out ruff at the moment and the first impression is very good. It formats basically like black, except that it’s somewhat more configurable. Have yet to test if the formatting can pass the flake8 tests.

6

You can also just use the normal black pre-commit hook; the ament one does not seem to behave any different.

7

Indeed ruff format works great and auto-formats in a way that looks almost like black but passes the flake8 tests, including single-quotes:

Here are my vscode settings:

"[python]": {
        "editor.defaultFormatter": "charliermarsh.ruff",
        "editor.formatOnSave": true,
        "editor.codeActionsOnSave": {
            "source.organizeImports": true,
            "python.sortImports": true
        }
    },
"ruff.format.args": [
        "--config=/home/your_home_dir/.config/ruff/ruff.toml",
        "--line-length=100"
    ],

And here the ruff.toml file:

[format]
quote-style = "single"
2 Likes
8

you also might be interesting in pre-commit setup we use in ros2_control project. We use rather all the tools directly without ament because they are then more portable and flexible to execute.

Also, executing linters in test was very, very annoying for us — so we separated linting and testing stages.

We decided to go with black with double quotes.

9

The honeymoon with the ruff formatter did not last too long: I can’t get it to order the imports the way that flake8 wants them, and there are some other cases (like one line vs two lines after class definitions) that flake8 does not pass.
So back to ament-cmake-black, for which there is a source entry in the rosdistro, and there is also a release branch in the release repository but it is currently unavailable on Ubuntu 22.04.

10

While on this topic, any idea why ament_black or ament-cmake-black in not part of of ament_lint?

1 Like
11

Hello there, I’m the current maintainer of ament_black.

This Python formatting situation is something that drove me nuts in the past and I still could not find interior peace in terms of ROS 2 requirements + tooling.

A fact is that black will NOT be adapted by ROS 2 repositories. See a failed try, but in general, makes not sense to reformat 100% of the Python code base now. black was not early adapted, and now it’s too late. Especially with these conditions

Extra info:

But something we might want to try is to combine ruff +isort to “blackify” the codebase with minimal changes, and if we manage to, then this tool would be more likely to get adapted in ROS repos…

I do dream of an automatic tool that does the whole reformatting without anyone changing double quotes to single quotes, ever again.

@Bernd_Pfrommer It seems you already had some ruff-fun, would you have some time in the future to support me with this effort?

@andrejpan : Above you can find the reason why ament_black is not part of the mainstream tooling of ament, it’s simply not the standard in ROS codebases

1 Like
12

@nachovizzo I can certainly help as a tester as I have done before. Alas I had to abandon ruff for now because I could not get the import ordering to work.

I still can’t believe there is a ROS formatting standard for python (flake8 tests) and no formatting tool for it that formats the code for you. We need to either find a formatting tool, or adjust the formatting standard. Ideally somebody would come out of the wood works and tell the rest of us how to auto-format their code to pass the flake8 tests (preferably in vscode/emacs/vi).

Changing formatting standards is painful, and I think reformatting existing code is a means of last resort. If there is indeed no other solution than moving to “black”, then why can’t we change the standard to “black” and just recommend newly introduced packages to use the “black” standard, leaving the older packages on flake8?

13

I’d say the main reason is quotes… haha believe it or not. Black does not support single-quotes (compared to others, like ruff). That means a massive amount of line changes, in all the repos, merge conflicts, pain, and more pain, just because of single/doble quotes. I think using black in ROS codebase is now a dream that will never come true.

I already used ruff to sort imports+format the ament_black repo (yes, this couldn’t be funkier, I know) and it’s making the flake8 tests happy. But this is a rather small python package (1 real python file), so I still need to try in other scenarios if that combination will work

PS: At some point ruff formatter will support automatically sorting the imports

1 Like
14

Cool, tnx for the explanation. Legacy is a big problem and it is hard to change a big chunk of code.

Do you have hints on where could “inexperienced” developers start helping on this topic?

15

Sure, it’s straightforward. We are debating format issues, which means no code should be changed, the impact of these changes is merely aesthetical.

I was planning to gather some “pure Python” repos that we know comply with the ROS standards (since for now, they are not enforced) and see what ruff can do there. It already helped with ament_black but more investigation is needed. If we can make sure ruff can format ROS code while making all flake8 linter happy, then I’d say that we could scale up this to turn it to be the default python formatted for ROS code, similar to what happened with uncrustify

16

@Bernd_Pfrommer good and bad news.

I managed to find a setup that makes ROS code happy but it’s suing ruff+isort, not ruff alone. This is mainly because ament_flake8 imposes google style for sorting imports, which is not the default in isort, ruff, and many tools. Luckily isort allows us to configure this, but not yet ruff

This is the setup that worked in this case

pyproject.toml

[tool.ruff]
line-length = 99

[tool.ruff.format]
quote-style = "single"

[tool.isort]
profile = "google"

and then run

ruff format .
isort --profile=google .

I also added a pre-commit hook that works so far:

repos:
  - repo: https://github.com/astral-sh/ruff-pre-commit
    rev: v0.1.13
    hooks:
      - id: ruff-format

  - repo: https://github.com/pycqa/isort
    rev: 5.13.2
    hooks:
      - id: isort
        args: [--profile, google]

I’d say with the current status of ruff we could either wait for better customization or create a new ament_ruff tool that runs those 2 checks sequentially aiming to automatically have ROS-PEP8 standardized code, with single quotes, google imports, and all that ugly stuff

1 Like
17

For some reason or the other I cannot get vscode to change the isort profile. I’m not even sure which one it is using right now. It looks like I’m passing the flake8 tests, so keeping fingers crossed.

"[python]": {
    "editor.defaultFormatter": "charliermarsh.ruff",
    //"editor.defaultFormatter": "ms-python.black-formatter",
    "editor.formatOnSave": true,
    "editor.codeActionsOnSave": {
        "source.organizeImports": "explicit",
    }
},
"isort.args": [
    "--profile",
    "google"
],
"ruff.format.args": [
    "--config=/home/pfrommer/.config/ruff/ruff_ros2.toml",
    "--line-length=99"
],
18

I have to disagree, but I’m using black to format about 15 different repositories. 4 are open source. The ament_black repo is awesome for this purpose, and pre-commit works well too!

I got tried of flake8 easly on. Spending time manually formatting code is a complete waste of time, but I understand why this practice is enforced in the ROS core.

Anyways, I propose using automated tools for new code, and would be happy to lead a proposal to change this requirement for ROS packages:
https://docs.ros.org/en/rolling/The-ROS2-Project/Contributing/Developer-Guide.html#linters-and-static-analysis

To me, if a NEW package has all of its linting tool requirements specified in the package.xml, they are documented how to run them automatically in CI, and you can run them all automatically with pre-commit, why does it have to be the same style for the every repo in the ecosystem? Isn’t the point to be consistent within a repo and obvious for users on how to run the tools?

Personally, I have zero preference whether to write python strings with single or double quotes; it’s much more important to me that the tooling just fixes whatever I type and gives new contributors an easy way to auto-format the code. Having to maintain packages with manually formatted code and remind contributors every PR that they failed the style checks and have to go manually add some whitespace before a class definition is not the best use of time IMHO.

Also, regarding having to deal with noise in the commit history, there’s a solution: git-blame-ignore-revs:

  1. Decide the formatting tools
  2. Run them on the code, and merge a single commit to the repo with those automated changes
  3. Add a PR to add that commit hash to the .git-blame-ignore-revs file
  4. Now, your editor and Gitlab ignore those commits when looking at blame!

Note - mass-formatting a codebase between releases is hard if you maintain multiple LTS branches. Depending on the size of the repo, and whether it’s forked or used as-is, it might make sense to do a mass-format. Some people go as far as re-writing all the history in the repo with the new format from the beginning of time, however that’s quite aggressive and breaks any companies forking internally who aren’t superb git users. Internally, we had 3 teams working on the same codebase, and set a date a year out, about a month after a big release, when everyone was doing architecture work, and ran the tool. It was pretty low impact for us because you can have pre-commit run the formatter on each commit when you rebase a long-standing branch that had the format change under its feet.

19

black has a --skip-string-normalization flag to avoid automatically formatting single quotes to double.

1 Like