[format] Using Black

[john-bodley]

CATEGORY

Choose one

• Bug Fix
• Enhancement (new features, refinement)
• Refactor
• Add tests
• Build / Development Environment
• Documentation

SUMMARY

Though for Python we currently use flake8 and pylint for linting and error checking these don't auto-format the code and thus it's still relatively hard to try to enforce a common code style/format.

There seems to be three main auto-formatters:

• autopep8
• YAPF
• Black

Here's a fairly good comparison of the three tools. Note that YAPF supports a number of styles including "pep8" (which is what autopep8 uses). The "Facebook" style is similar to Black.

Though I don't necessarily agree with all of Black's style formatting (including the 88 character line length, non-trailing commas, double-quotes, etc.) it seems this has gained popularity(it currently has over 11,000 GitHub stars) over the likes of YAPF mostly because it isn't configurable (and thus ensures consistency) which is a plus if the goal is to align on a style/format across numerous repos.

This PR performs the following:

1. Formats all the code using the Black format.
2. Adds a pre-commit hook to auto-format the diff using the Black format on every local commit.
3. Adds a CI check to ensure that the code is Black format compliant.

Note the flake8 and pylint CI remains as flake8 includes checks for import ordering as well as type checking. pylint additionally checks for numerous errors. In the future there would be merit in further cleaning up the code and reducing the number of flake8 codes we ignore.

TEST PLAN

CI.

ADDITIONAL INFORMATION

• Has associated issue:
• Changes UI
• Requires DB Migration.
• Confirm DB Migration upgrade and downgrade tested.
• Introduces new feature or API
• Removes existing feature or API

REVIEWERS

to: @betodealmeida @michellethomas @mistercrunch @villebro

[mistercrunch]

I've been meaning to do this for a while. Black is the way to go.

We could do a 2 phase rollout with black -S (skipping the single/vs/double quote issue) first.

Happy to just byte the bullet and go for it.

+1

[john-bodley]

@mistercrunch I can try the two phase rollout to minimize the change. Regarding pre-commit I'm using the suggestion in the doc per here.

[codecov-io]

Codecov Report

Merging #7769 into master will decrease coverage by <.01%.
The diff coverage is 59.95%.

@@            Coverage Diff             @@
##           master    #7769      +/-   ##
==========================================
- Coverage   65.71%   65.71%   -0.01%     
==========================================
  Files         459      459              
  Lines       21981    21980       -1     
  Branches     2415     2415              
==========================================
- Hits        14445    14444       -1     
  Misses       7415     7415              
  Partials      121      121

Impacted Files	Coverage Δ
superset/models/sql_types/presto_sql_types.py	72.41% <ø> (ø)	⬆️
superset/models/sql_lab.py	94.73% <ø> (ø)	⬆️
...migrations/versions/fb13d49b72f9_better_filters.py	53.44% <ø> (ø)	⬆️
superset/views/datasource.py	95.12% <ø> (ø)	⬆️
superset/utils/import_datasource.py	100% <ø> (ø)	⬆️
superset/models/tags.py	90.09% <ø> (ø)	⬆️
superset/sql_validators/__init__.py	83.33% <ø> (ø)	⬆️
superset/sql_lab.py	76.88% <ø> (ø)	⬆️
superset/views/__init__.py	100% <ø> (ø)	⬆️
superset/views/utils.py	87.77% <ø> (ø)	⬆️
... and 125 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0c9e6d0...0f249a4. Read the comment docs.

[mistercrunch]

Now thinking about it I'm thinking making it straight black is best, two phases would just be more confusing... There will be lots of conflicts, but resolving them is easy: just run black.

[john-bodley]

@mistercrunch are you ok with me merging this? I realize that the PR will cause a number of conflicts in existing PRs, though as you pointed out, resolving these are trivial.

[mistercrunch]

LGTM - deterministic code style FTW!

[mistercrunch]