Taichi: Enforcing Code Formats

[yuanming-hu]

First of all, inputs are welcome! Reformatting every source file is clearly a HUGE change. Although the benefits are clear, reformatting everything will lead to issues such as hard-to-track source line history (so git blame will no longer work for previous commits). This is likely our final chance to change source code at this scale. So I would like feedback from everyone, before the decision is made on March 17 23:59 EDT. Especially, I would like to know if everyone who contributed/will contribute agrees on the code format below: @k-ye @archibate @KLozes @masahi @xumingkuan @Psycho7

Code Formats

Low-level formats:

• For C++ we will use the a clang-format profile that mostly follows the Chromium code style
• For Python 3, we will simply follow PEP 8 and use yapf to enforce it. This means we are switching to 4-space identation instead of 2 (thanks to @masahi)

Chromium for C++ is picked for historical reasons. Most taichi C++ files already roughly follow these formats.

For all files (including e.g. rst and md), we will

• Enforce a new line ad the end of file.
• No tailing whitespace allowed
• No tab allowed (except for Makefiles in misc). Please use spaces.

We will follow the high-level code style guide from Google for C++ and Python.

Since we have reached a consensus (by March 17 23:59 EDT), code format will probably not be altered again in the future. For examples, for Python we could also use chromium (2-space indentation) or google, but we have made our decision to use PEP 8.

Reformatting everything

Automatic reformating using clang-format-6.0 and yapf 0.29.0 has been done on March 18, around 11:59 am EDT, as a pull request from @taichi-gardener.

Format issues that can't be directly addressed by the auto-formatters will be gradually fixed later.

Continuous format enforcement

There's no doubt that consistent code style is good, but we don't want the installation of formatting tools and cumbersome rules to stop people from contributing. So here's a solution that may make everyone happy:

• I wrote a minimalist format server which takes in PR id and commits with the reformatted version under the user @taichi-gardener. I have confirmed this works (test case: https://github.com/taichi-dev/taichi/pull/590). In the future, we can make it more automatic using something like GitHub actions. I'll deploy it at a machine at MIT and then PR mergers can enforce code format with one click.

• Starting March 19, frequent committers can use the ti format command to reformat modified code before committing. We might also configure a githook, but that's optional.

---

Thank you so much for your contributions and time reading this long post. I believe we are working together to create a very meaningful tool for people in the fields of computer graphics/computational physics/machine learning.

Again, inputs are welcome. For those whose life is affected by COVID-19, please take care!

[masahi]

For python, both chromium and google now seem to follow 4 space indent. So why stick to 2?

https://chromium.googlesource.com/chromium/src/+/master/styleguide/python/python.md#Our-Previous-Python-Style

They say they used to do 2 space and CamelCase, which seem very weird to me. I believe majority of py projects follow 4 space and snake_case.

[xumingkuan]

590 makes mpm99.py longer than 99 lines. Shall we exclude the examples from reformatting or allow a higher column limit?

[archibate]

So @taichi-gardener is our dummy user hooked on the repo PR and only make enforce code format commits? Cool! I want to make sure if this is only called right before merge/squash? Or might confuse style-breaking contributors..

[yuanming-hu]

590 makes mpm99.py longer than 99 lines. Shall we exclude the examples from reformatting or allow a higher column limit?

Good point. We should simply skip these files in examples with filename following letters + line numbers.

[yuanming-hu]

For python, both chromium and google now seem to follow 4 space indent. So why do we stick to 2?

https://chromium.googlesource.com/chromium/src/+/master/styleguide/python/python.md#Our-Previous-Python-Style

They say they used to do 2 space and CamelCase, which seem very weird to me. I believe majority of py projects follow 4 space and snake_case.

Oh, I didn't know they are adopting PEP8 and use 4 spaces and snake_case for new script files. I guess it's probably a painful decision for them, since they now have two Python styles in the same project.

I also assume this implies sticking to PEP8 has clear practical benefits otherwise they won't embrace such pain. Thank you so much for pointing this out.

Let's change the Python format to PEP 8.

[yuanming-hu]

So @taichi-gardener is our dummy user hooked on the repo PR and only make enforce code format commits? Cool! I want to make sure if this is only called right before merge/squash? Or might confuse style-breaking contributors..

Currently, the user/reviewer has to do an HTTP GET with URL http://server/PR_id to make the server do the job. This should happen before squashing/merging. I think maybe we can make it more automatic. GitHub Actions may be one option.

[yuanming-hu]

Done. Thank everyone for participating. Contributors can auto-format their PRs using the format server.

[yuanming-hu]

If you see nothing when accessing the format server, then we have probably hit the GitHub API querying limit. See #620