# RFC: Document and standardize Python code style

**URL:** <https://discourse.llvm.org/t/rfc-document-and-standardize-python-code-style/68257>\
**Category:** LLVM Project\
**Tags:** rfc\
**Created:** [February 7, 2023, 7:49am UTC](https://discourse.llvm.org/t/rfc-document-and-standardize-python-code-style/68257 "2023-02-07T07:49:23Z")\
**Posts on this page:** 20\
**Page:** 1

<div class="post-metadata">

**Author:** ![tobiashieta](https://sea1.discourse-cdn.com/flex021/user_avatar/discourse.llvm.org/tobiashieta/32/26681_2.png) [@tobiashieta](https://discourse.llvm.org/u/tobiashieta)\
**Post date:** [February 7, 2023, 7:49am UTC](https://discourse.llvm.org/t/rfc-document-and-standardize-python-code-style/68257/1 "2023-02-07T07:49:23Z")

</div>

In a recent [review](https://reviews.llvm.org/D143295), we discussed Python coding style. I realized that we hadn’t documented the Python coding style we want the scripts in our repo to use.

The most logical thing is to document that we should follow [PEP-8](https://peps.python.org/pep-0008/) in our python scripts.

We should also use [Black](https://github.com/psf/black) to format Python code. It doesn’t require any configuration, follows PEP-8 (except in [some cases](https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html#pragmatism)) and is officially “blessed” by the PSF.

In the future we could add automatic formatting with black for Python scripts in pre-commit checks, but for now I think it’s enough to recommend it.

If no one disagrees, I will add this to the CodingStandard document.

---

<div class="post-metadata">

**Author:** ![jh7370](https://sea1.discourse-cdn.com/flex021/user_avatar/discourse.llvm.org/jh7370/32/2079_2.png) [@jh7370](https://discourse.llvm.org/u/jh7370)\
**Post date:** [February 7, 2023, 9:02am UTC](https://discourse.llvm.org/t/rfc-document-and-standardize-python-code-style/68257/2 "2023-02-07T09:02:11Z")

</div>

+1 to having an official Python coding style. I don’t have any particular comment on what the style should be, but an official one seems as good a one as any (as long as our existing code doesn’t stray too far from it).

---

<div class="post-metadata">

**Author:** ![jrtc27](https://sea1.discourse-cdn.com/flex021/user_avatar/discourse.llvm.org/jrtc27/32/1748_2.png) [@jrtc27](https://discourse.llvm.org/u/jrtc27)\
**Post date:** [February 7, 2023, 9:46am UTC](https://discourse.llvm.org/t/rfc-document-and-standardize-python-code-style/68257/3 "2023-02-07T09:46:43Z")

</div>

UTC uses a 2-space indent like our C++ style

---

<div class="post-metadata">

**Author:** ![tobiashieta](https://sea1.discourse-cdn.com/flex021/user_avatar/discourse.llvm.org/tobiashieta/32/26681_2.png) [@tobiashieta](https://discourse.llvm.org/u/tobiashieta)\
**Post date:** [February 7, 2023, 10:01am UTC](https://discourse.llvm.org/t/rfc-document-and-standardize-python-code-style/68257/4 "2023-02-07T10:01:06Z")

</div>

What’s UTC in this context?

I have no strong opinion about 2 vs. 4 spaces in Python, I personally think 2 spaces are nicer, but all PEP-8 tools enforce 4 by default, so I usually just go with the flow here.

---

<div class="post-metadata">

**Author:** ![gregb](https://sea1.discourse-cdn.com/flex021/user_avatar/discourse.llvm.org/gregb/32/2413_2.png) [@gregb](https://discourse.llvm.org/u/gregb)\
**Post date:** [February 7, 2023, 10:06am UTC](https://discourse.llvm.org/t/rfc-document-and-standardize-python-code-style/68257/5 "2023-02-07T10:06:22Z")

</div>

I’d second the recommendation for black for newly added files at least. Internally we use it in conjunction with isort and just go with whatever formatting they impose and have never had a problem. In our case internally though we just reformatted our entire project when we switched to black. I’m not sure whether it allows partial formatting of selections or not. The LLVM project went through some quite large discussions for whether to reformat the entire LLVM codebase with clang-format when that became available but decided not to. Maybe the arguments are different for the more limited amount of python code in the project?

One question would be whether to [accept the default 88 columns or to configure it for 80 columns](https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html#line-length) to match the rest of the project. Beyond that it can’t really be configured (which I personally regard as a good thing).

While it’s still very early days, I’ve been having a really good time with [ruff](https://pypi.org/project/ruff/) for linting, so we might want to consider recommending that new code is run through that, although I don’t know if we’d want to go so far as to enforce it just yet.

---

<div class="post-metadata">

**Author:** ![jrtc27](https://sea1.discourse-cdn.com/flex021/user_avatar/discourse.llvm.org/jrtc27/32/1748_2.png) [@jrtc27](https://discourse.llvm.org/u/jrtc27)\
**Post date:** [February 7, 2023, 4:45pm UTC](https://discourse.llvm.org/t/rfc-document-and-standardize-python-code-style/68257/6 "2023-02-07T16:45:04Z")

</div>

UpdateTestChecks. Which isn’t to say that we shouldn’t go with 4 spaces, just pointing out we have a chunk of regularly-used code that doesn’t adhere to that.

---

<div class="post-metadata">

**Author:** ![pogo59](https://sea1.discourse-cdn.com/flex021/user_avatar/discourse.llvm.org/pogo59/32/2412_2.png) [@pogo59](https://discourse.llvm.org/u/pogo59)\
**Post date:** [February 7, 2023, 6:48pm UTC](https://discourse.llvm.org/t/rfc-document-and-standardize-python-code-style/68257/7 "2023-02-07T18:48:06Z")

</div>

As long as there’s a used-everywhere tool to enforce a style, I’m not too fussed about what the exact style is.

+1 to do a bulk update of everything. Mixed styles is worse than any specific style. I hate that LLVM has consistently failed to do bulk updates whenever we actually manage to agree on a style.

Don’t forget the lit.cfg files peppered all through the test directories. They’re all Python, even if they don’t all have a .py extension.

---

<div class="post-metadata">

**Author:** ![JDevlieghere](https://sea1.discourse-cdn.com/flex021/user_avatar/discourse.llvm.org/jdevlieghere/32/2048_2.png) [@JDevlieghere](https://discourse.llvm.org/u/JDevlieghere)\
**Post date:** [February 7, 2023, 7:06pm UTC](https://discourse.llvm.org/t/rfc-document-and-standardize-python-code-style/68257/8 "2023-02-07T19:06:18Z")

</div>

+1 on what’s been said here regarding consistency and having a tool to automate the process.

The majority of LLDB tests are written in Python. At some point in the past it was formatted but it was never maintained or enforced. I’m supportive of bulk formatting everything, but I’d like at least the opportunity to see if we can find a configuration that reduces the churn in our test suite. If we’re all using a tool (such as black or yapf) it shouldn’t matter if the configuration for LLDB is slightly different.

---

<div class="post-metadata">

**Author:** ![royjacobson](https://sea1.discourse-cdn.com/flex021/user_avatar/discourse.llvm.org/royjacobson/32/14059_2.png) [@royjacobson](https://discourse.llvm.org/u/royjacobson)\
**Post date:** [February 7, 2023, 7:57pm UTC](https://discourse.llvm.org/t/rfc-document-and-standardize-python-code-style/68257/9 "2023-02-07T19:57:14Z")

</div>

+1 for black, with one comment - black breaks compatibility between versions from different calendaric years: [The Black Code Style - Black 23.1.0 documentation](https://black.readthedocs.io/en/stable/the_black_code_style/index.html)

I’ve seen this cause some formatting ping-pong in practice.

With clang format that’s less of a problem because we just vendor our own version 🙂

Could we maybe vendor an official version of black through the existing CMake infrastructure to avoid this issue?

---

<div class="post-metadata">

**Author:** ![tobiashieta](https://sea1.discourse-cdn.com/flex021/user_avatar/discourse.llvm.org/tobiashieta/32/26681_2.png) [@tobiashieta](https://discourse.llvm.org/u/tobiashieta)\
**Post date:** [February 7, 2023, 8:46pm UTC](https://discourse.llvm.org/t/rfc-document-and-standardize-python-code-style/68257/10 "2023-02-07T20:46:11Z")

</div>

Did some checks with black - with default configuration (the only one that really counts in black) I ran:

```auto
black $(find . -name \*.py)
2035 files reformatted, 232 files left unchanged, 6 files failed to reformat.

```

git diffstat for that:

```auto
2035 files changed, 124430 insertions(+), 97959 deletions(-)

```

so that’s a lot of churn, let’s try black but with two spaces instead (there are no configuration options in black to set the indent, so you actually have to install another tool (black) that monkey patches that into black… sigh)

```auto
2078 files reformatted, 189 files left unchanged, 6 files failed to reformat.

```

diff stat:

```auto
2072 files changed, 221808 insertions(+), 201602 deletions(-)

```

so it would seem like 4 spaces are the more common indent style in our tree.

What about the files that couldn’t be parsed? One file seems to have out right parse error:

```auto
error: cannot format llvm/utils/lit/tests/shtest-encoding.py: 'utf-8' codec can't decode byte 0xc2 in position 69: invalid continuation byte

```

Aha, that file has an INTENTIONAL parse error 🙂

5 files are still using python 2 syntax:

```auto
error: cannot format polly/utils/pyscop/jscop2iscc.py: Cannot parse: 12:8: print "D :=",
Python 2 support was removed in version 22.0.
error: cannot format polly/utils/jscop2cloog.py: Cannot parse: 53:8: print template % (context, domains, schedules)
Python 2 support was removed in version 22.0.
error: cannot format compiler-rt/lib/dfsan/scripts/build-libc-list.py: Cannot parse: 92:8: print 'fun:%s=uninstrumented' % f
Python 2 support was removed in version 22.0.
error: cannot format polly/lib/External/isl/imath/tools/findthreshold.py: Cannot parse: 89:14: print "%d\t%d\t%.3f\t%.3f\t%.4f" % (prec, thresh, trec, tnorm,
Python 2 support was removed in version 22.0.
error: cannot format openmp/runtime/tools/summarizeStats.py: Cannot parse: 146:14: print "Cannot open " + fname
Python 2 support was removed in version 22.0.

```

So those needs to be fixed, in any case.

I also tried yapf with `--style=pep8` and it seems way less picky:

```auto
1861 files changed, 75453 insertions(+), 62234 deletions(-)

```

Quickly looking at the files I still prefer `black` over `yapf` - but I don’t care **that** much, the important part is that we have a standard tool and that we enforce it - similar to what people have said above.

---

<div class="post-metadata">

**Author:** ![tobiashieta](https://sea1.discourse-cdn.com/flex021/user_avatar/discourse.llvm.org/tobiashieta/32/26681_2.png) [@tobiashieta](https://discourse.llvm.org/u/tobiashieta)\
**Post date:** [February 7, 2023, 8:48pm UTC](https://discourse.llvm.org/t/rfc-document-and-standardize-python-code-style/68257/11 "2023-02-07T20:48:24Z")

</div>

Note that yapf allows configuration of the style, so we could have a yapf config file in our repo similar to a `.clang-format` file to potentially reduce the churn even more. But since there hasn’t been any enforced standard before I wouldn’t expect there to be a single style that would fit all of our files.

---

<div class="post-metadata">

**Author:** ![tobiashieta](https://sea1.discourse-cdn.com/flex021/user_avatar/discourse.llvm.org/tobiashieta/32/26681_2.png) [@tobiashieta](https://discourse.llvm.org/u/tobiashieta)\
**Post date:** [February 9, 2023, 7:50pm UTC](https://discourse.llvm.org/t/rfc-document-and-standardize-python-code-style/68257/12 "2023-02-09T19:50:15Z")

</div>

It sounds like we have consensus around adding A code-standard.

It seems like we have consensus around that coding standard should be PEP-8. The only argument against seems to be that some code is using 2 spaces and not 4. But it also didn’t sound like @jrtc27 was totally against switching, did I read that correct?

About mass-reformatting. From what I read most seem to be in favour - but the diff seems huge, especially considering tests in lldb for example. Not sure we have a consensus around this yet?

I propose a document patch that codifies PEP-8 as our python coding style and recommends formatting code with black. I feel this step can be taken without deciding on mass reformat.

I would love to hear what people think about my stats on the patch size above and more arguments for and against mass reformatting.

---

<div class="post-metadata">

**Author:** ![pogo59](https://sea1.discourse-cdn.com/flex021/user_avatar/discourse.llvm.org/pogo59/32/2412_2.png) [@pogo59](https://discourse.llvm.org/u/pogo59)\
**Post date:** [February 9, 2023, 8:04pm UTC](https://discourse.llvm.org/t/rfc-document-and-standardize-python-code-style/68257/13 "2023-02-09T20:04:53Z")

</div>

I’m curious what proportion of the changed files from your experiment are from the lldb tests? I count about 1200 \*.py files under lldb/test. If most of them need formatting, that’s like half the total number of files. In that case, a mass-reformat of everything except lldb/test feels viable. The lldb folks can catch up as they go, and nobody else is really affected.

---

<div class="post-metadata">

**Author:** ![JDevlieghere](https://sea1.discourse-cdn.com/flex021/user_avatar/discourse.llvm.org/jdevlieghere/32/2048_2.png) [@JDevlieghere](https://discourse.llvm.org/u/JDevlieghere)\
**Post date:** [February 9, 2023, 9:52pm UTC](https://discourse.llvm.org/t/rfc-document-and-standardize-python-code-style/68257/14 "2023-02-09T21:52:28Z")

</div>

Yup, that was my original suggestion. My suspicion is that it’s exactly like that, but it would be good to confirm.

@tobiashieta Could you collect those numbers?

---

<div class="post-metadata">

**Author:** ![tobiashieta](https://sea1.discourse-cdn.com/flex021/user_avatar/discourse.llvm.org/tobiashieta/32/26681_2.png) [@tobiashieta](https://discourse.llvm.org/u/tobiashieta)\
**Post date:** [February 10, 2023, 7:40am UTC](https://discourse.llvm.org/t/rfc-document-and-standardize-python-code-style/68257/15 "2023-02-10T07:40:17Z")

</div>

Running black on everything and then resetting the `lldb` subdirectory gives the following stats:

```auto
776 files changed, 68835 insertions(+), 55888 deletions(-)

```

So yeah it’s safe to say that more than half the files formatted was in lldb subdirectory.

I have posted three phabricator diffs to fix the syntax problems I found when running `black`, reviews of those would be great:

- compiler-rt - [⚙ D143701 [NFC] convert compiler-rt/lib/dfsan/scripts/build-libc-list.py to python3 syntax](https://reviews.llvm.org/D143701)
- OpenMP - [⚙ D143702 [NFC] Convert openmp/runtime/tools/summarizeStats.py to python 3 syntax](https://reviews.llvm.org/D143702)
- Polly - [⚙ D143703 [NFC][polly] Convert some python scripts to Python 3 syntax](https://reviews.llvm.org/D143703)

---

<div class="post-metadata">

**Author:** ![OCHyams](https://sea1.discourse-cdn.com/flex021/user_avatar/discourse.llvm.org/ochyams/32/1968_2.png) [@OCHyams](https://discourse.llvm.org/u/OCHyams)\
**Post date:** [February 10, 2023, 3:20pm UTC](https://discourse.llvm.org/t/rfc-document-and-standardize-python-code-style/68257/16 "2023-02-10T15:20:07Z")

</div>

I’m happy to bulk update the `cross-project-tests/debuginfo-tests/dexter` directory, if it is useful to help break it up? FWIW using black version `22.8.0` that’s:

```auto
 79 files changed, 3621 insertions(+), 2994 deletions(-)

```

How does the decision become official? It looks like there’s one outstanding concern still:

> [@royjacobson](#):
>
> black breaks compatibility between versions from different calendaric years: [The Black Code Style - Black 23.1.0 documentation](https://black.readthedocs.io/en/stable/the_black_code_style/index.html)

---

<div class="post-metadata">

**Author:** ![tobiashieta](https://sea1.discourse-cdn.com/flex021/user_avatar/discourse.llvm.org/tobiashieta/32/26681_2.png) [@tobiashieta](https://discourse.llvm.org/u/tobiashieta)\
**Post date:** [February 10, 2023, 3:21pm UTC](https://discourse.llvm.org/t/rfc-document-and-standardize-python-code-style/68257/17 "2023-02-10T15:21:21Z")

</div>

I think the easiest is that we recommend black 23.x for now and maybe bump that next year if we feel there is a need for it.

---

<div class="post-metadata">

**Author:** ![tobiashieta](https://sea1.discourse-cdn.com/flex021/user_avatar/discourse.llvm.org/tobiashieta/32/26681_2.png) [@tobiashieta](https://discourse.llvm.org/u/tobiashieta)\
**Post date:** [February 10, 2023, 3:22pm UTC](https://discourse.llvm.org/t/rfc-document-and-standardize-python-code-style/68257/18 "2023-02-10T15:22:26Z")

</div>

I think we’ll start with landing the Policy.

I am planning to submit that to Phab this weekend.

---

<div class="post-metadata">

**Author:** ![tobiashieta](https://sea1.discourse-cdn.com/flex021/user_avatar/discourse.llvm.org/tobiashieta/32/26681_2.png) [@tobiashieta](https://discourse.llvm.org/u/tobiashieta)\
**Post date:** [February 12, 2023, 8:02pm UTC](https://discourse.llvm.org/t/rfc-document-and-standardize-python-code-style/68257/19 "2023-02-12T20:02:00Z")

</div>

I have pushed an initial draft of the change to the documentation here: [⚙ D143852 [docs] Add Python coding standard to documentation (llvm.org)](https://reviews.llvm.org/D143852)

Not sure who I should add as reviewers, so please feel free to add yourselves.

---

<div class="post-metadata">

**Author:** ![pogo59](https://sea1.discourse-cdn.com/flex021/user_avatar/discourse.llvm.org/pogo59/32/2412_2.png) [@pogo59](https://discourse.llvm.org/u/pogo59)\
**Post date:** [February 13, 2023, 4:37pm UTC](https://discourse.llvm.org/t/rfc-document-and-standardize-python-code-style/68257/20 "2023-02-13T16:37:08Z")

</div>

I don’t think we came to a conclusion about 80-char vs 88-char lines. The reset of the project uses 80 (for code, anyway) and it seems a little odd to make that different for Python; the only justification being “that’s black’s default.”

[Next page](https://discourse.llvm.org/t/rfc-document-and-standardize-python-code-style/68257.md?page=2)