Skip to content

Reformat dantro using black

Utopia Developers requested to merge 125-use-formatter-and-pre-commit-hooks into master

This MR reformats dantro using black, the uncompromising code formatter, and integrates pre-commit into the development workflow to automate future code formatting.

Details

This is quite a large MR due to all the formatting changes. Let me walk you through it:

Why black?

It turns out that we can still use black 🎉 It seems they have introduced the possibility to exclude parts of the code from formatting by adding # fmt: on/off fences around them. As this was our only reason for choosing yapf over black, I was happy that we could simply use black (also because it is much much easier to configure).

How was the code re-formatted?

My workflow was like this:

  1. Go through each python file we have in the repo.
  2. Look at the git blame:
    • Module dominated by single author? If yes:
      • Apply formatting to that file
    • Distinct parts dominated by single author? If yes:
      • Suppress formatting in those parts by adding # fmt: on/off fences
      • Then apply formatting for the rest of that file
  3. Go through the changed files and ...
    • ... replace .format with f-strings
    • ... in a few cases, fix the formatting to be easier to read
  4. Commit these changes in the name of the original author using git commit --author="Name <Mail>"
  5. Repeat for the next author or left-over files

Note:

  • While all files are changed in some way, the diffs were not as large as feared: many parts remain as they are, thus the original authors also are retained for many lines.
  • With black formatting, all strings consistently use double-quoted strings. Many of the diffs are simply ' -> ".
  • For string formatting, black respects the manual formatting in most cases and does not apply formatting on top... so in some places, this leads to overlong strings or weirdly wrapped ones. Therefore, during this initial reformatting, I used the --experimental-string-processing feature of black, which helped wrapping long strings into multi-line strings. In some places this might have lead to some weird string wrapping that I might have overlooked; shouldn't be too bad, though. (If you spot such a place, lmk.)

What else was done?

  • Applied isort import sorting:
    • Alphabetically sorts import statements
    • Put standard-library, third-party, and first-party imports into separate blocks
  • Added configuration files for isort and black (see pyproject.toml)
  • Added pre-commit to manage automatic application of isort and black
    • ... installs some further hooks, see .pre-commit-config.yaml
  • Used pre-commit hooks to ...
    • ... remove trailing whitespace from lines
    • ... add new line characters to the end of files
    • ... ensure double backticks are used in RST files

Anything to double-check?

  • After testing pre-commit locally: everything working as expected? (Feel free to make a test commit, but don't push ;))
  • Updated README and CONTRIBUTING helpful in that respect?
  • Do you see other useful commit hooks that we could use?
    • I tried out one to check YAML files, but they get confused and fail with all the custom YAML tags we are using

Can this MR be accepted?

  • Formatting done
  • Integration done
  • Documentation updated
  • Badges added to GitLab project
  • Checked test code coverage
    • Is slightly reduced (by 0.02%) because of some uncovered error messages now including an additional string formatting statement; nothing to worry about.
  • Ready for merging
    • Pipeline passes without warnings
    • History cleaned-up or squash option set
    • Changelog entry added
    • Version number bumped
    • Reviewed & approved by all maintainers

Related issues

Closes #125 (closed)

Edited by Utopia Developers

Merge request reports