Skip to content

Better formatting for cirq code #3292

@maffoo

Description

@maffoo

I find the autoformatting that we currently use in cirq to be less than ideal. There are three main issues:

  • yapf formatting is often bad, particular with things like nested tuples but also due to mixing hanging versus 4-space indentation styles (here's an example of mixed indentation)
  • 80 character lines seem to be too short, particularly with heavy use of type annotations as we have in our code.
  • The incremental formatting is line-based, so we have lots of old code that has never been formatted.

Describe the solution you'd like

  • Switch formatter from yapf to black. Black's formatting algorithm is simpler and the resulting formatting is more predictable and regular than yapf's, IME.
  • Increase the line length to something like 100 characters. I think this would cut down on a lot of unnecessary line breaking and improve readability of the code.
  • Switch the incremental autoformatting to be file-based instead of line-based. As part of doing this, we would have a one-time change to format the entire code base, which would be a massive diff but entirely automated. There's also a bit of work to update PRs that are in-flight at the time of the big formatting change, but this too is manageable.

Describe alternatives/workarounds you've considered

  • Could increase line length without switching from yapf to black.
  • Could tweak various yapf knobs to possibly improve the formatting (I prefer black's approach of having fewer knobs in the first place so there's less to argue about, but YMMV).
  • Could switch to file-based incremental formatting and pay the one-time cost of formatting all old code without switching to black.

What is the urgency from your perspective for this issue? Is it blocking important work?

P3 - I'm not really blocked by it, it is an idea I'd like to discuss / suggestion based on principle

Metadata

Metadata

Assignees

Labels

area/formattingkind/feature-requestDescribes new functionalitykind/healthFor CI/testing/release process/refactoring/technical debt itemstriage/acceptedA consensus emerged that this bug report, feature request, or other action should be worked on

Type

No type

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions