Use black for code auto formatting

[terrytangyuan]

It would be good to use auto formatting for our current Python codebase so the style is consistent among developers without having to manually fixing lint, etc. I've had good experience using black for other projects and it looks like both tensorflow/addons and TensorBoard are already using it.

Let's use this issue to discuss this and I am happy to work on this once we reach a consensus.

[yongtang]

I prefer black as well, we want to align with the overall tensorflow community so that common issues might be resolved together.

[BryanCutler]

It will be good to be on the same page as the rest of the community, so sounds good to me. Thanks @terrytangyuan !

[gabrieldemarmiesse]

If you guys are interested, in Addons, we managed to do a progressive (graceful) migration to black without git conflicts and without stopping anyone from working. See tensorflow/addons#988 and tensorflow/addons#932

[terrytangyuan]

@gabrieldemarmiesse Thanks! That’s a good idea. I’ll take a closer look on the details when I get a chance.

[terrytangyuan]

Now that @yongtang has added black check in our bazel scripts #837. It might be better for individual developers to run black check on their new PRs instead of doing everything at once (like in #812) to avoid conflicts.

If everyone agrees, I'll close #812 and then everyone can contribute towards this. WDYT?

[BryanCutler]

SGTM, thanks @terrytangyuan !

[yongtang]

Thanks sounds good to me. @terrytangyuan @BryanCutler I think what we could do, is in build.yml's lint section, to invoke black on individual files that has been touched by:

git diff --name-only master

Then we could run black (and pyupgrade) for those python files from the above list.

[terrytangyuan]

@yongtang Good idea.

[yongtang]

Looks like pylint from Google requires 2 space while Black needs 4 spaces. This is causing issues and we may need to find a way to get around.

[gabrieldemarmiesse]

4 spaces FTW

[yongtang]

It looks like pylint disagree with black in quite a few places. The biggest issue is the bad-continuation which more or less like a pylint bug pylint-dev/pylint#289

Another issue is that black will format line exceeding 80 in certain situations with causes Line too long (86/80) from pylint. In this case we probably could consider both pylint and black are correct.

Google's pylint rc file also uses 2 space, though we could override this behavior with:

pylint --indent-string='    ' --rcfile=googe.pylint.rc file.py

However, other than 2 spaces, the first two issues are really hard to fix in a way both black and pylint are happy.

Since both tensorflow_addon and tensorflow_tensorboard only uses black, I am inclined to drop pylint, and in favor of black only. Any concerns about that?

[gabrieldemarmiesse]

@yongtang In addons, we didn't drop code linting entirely, we replaced it by flake8 and we changed some default settings. We didn't have any issues yet with it, as we took it from the black documentation. See https://github.com/tensorflow/addons/blob/master/.flake8

[terrytangyuan]

Yes I agree. I've had good experience with the combination of black and flake8 but not with pylint. We can drop it.

[yongtang]

Thanks @gabrieldemarmiesse @terrytangyuan, I made some adjustment to pylint with the following overrides:

--indent-string='   ' (4 spaces to match black)
--max-line-length=88 (88 to match black)
--disable=bad-continuation (conflict with black)

That makes both black and pylint happy. Created a PR #860 with black. We can look into flake8 later.