Core: Use yapf to auto-format the code

[balloob]

yapf is a formatter that will take care of formatting our Python code. It means that we can disable Hound any style related linter rule and just add 1: check if the author ran yapf. Formatting will never be an issue again.

What do people think about this?

CC @home-assistant/developers

[bachya]

I like it; however, I would caution that in the past, my use of yapf has resulted in changes that I’ve been asked to revert. The most obvious example is putting newlines between schema config keys and values.

[emlove]

I'm open in concept. I'd certainly prefer the bickering to be over knobs in the yapf config file, that would then be uniformly applied throughout the codebase.

Maybe I'm overestimating the effort, but I'd personally prefer an incremental roll out. It would be nice to have someone at least skim all the newly changed files. Z-Wave Discovery Schema comes to mind, and I'm sure we have other places where an override is still probably best. We'd probably want plenty of time to evaluate the initial impact, since changing knobs later will continue to noisy up our blames.

[andrey-git]

Hound has the advantage that it is really fast.
If yapf check runs on travis it will be much longer till the PR author is notified of the needed change.

When we started to use something similar for Java - it was painful at the start and formatting oddities jumped out. But overall the experience is good.

[MartinHjelmare]

If it can handle all the style particulars we use today, I'm very positive.

[MartinHjelmare]

I think we should also include isort, by the same reasoning.

[balloob]

@bachya those style approaches that you were asked to revert should then become accepted. yapf = the boss.

@armills it's possible to have yapf skip blocks with certain comments.

[fabaff]

I'm all for it. Sure, we need to tweak it at a couple of places but with yapf: enable/disable the handling will be similar to pylint.

[Danielhiversen]

I just tried yapf on our code: https://github.com/home-assistant/home-assistant/compare/yapf?expand=1
(This is with default parameters)

I think the code gets more readable (in most cases) and it will be easier to maintain a correct style.

I do not think this is an improvement: https://github.com/home-assistant/home-assistant/compare/yapf?expand=1#diff-c4190fb3f1d722a891951f11ce263541R36

[lwis]

I'm down for a uniform format, but I'm not a fan of things like this. So if we're going to use it, we need to make sure it's tuned with sane defaults.

[balloob]

Yeah I wonder why it prefers to put expressions for keys and values on their own line. Although I am not a fan, I wouldn't consider it a deal breaker if it solves everything else.

[Danielhiversen]

ALLOW_SPLIT_BEFORE_DICT_VALUE: False seems to fix that

Now I used:
yapf homeassistant -i --recursive --style='{based_on_style: pep8, ALLOW_SPLIT_BEFORE_DICT_VALUE: False}'

[bachya]

Playing around with ways that we can make it easier for contributors to do this (via "please follow this" documentation) – perhaps a pre-commit Git hook?

#!/bin/sh

# Redirect output to stderr:
exec 1>&2

# Grab all changed Python files:
changed_files=$(git diff --staged --name-only | grep ".py")

for file in ${changed_files}
do
  pydocstyle "$file"
  [[ "$?" -gt "0" ]] && exit 1
done

for file in ${changed_files}
do
  echo "YAPF-ing ${file}"
  yapf "$file" -i --style='{based_on_style: pep8, ALLOW_SPLIT_BEFORE_DICT_VALUE: False}'
  git add "$file"
done

Essentially, when a contributor attempts to commit any changed Python files:

1. pydocstyle gets run; any violations get printed and the commit fails.
2. yapf gets run.
3. The final files are added and committed.

[sdague]

It looks like it's being really odd about type annotations in param lists.

We experimented with yapf a couple of years ago in OpenStack and didn't decide to do it because the stability of the format wasn't clear, and it was likely to lead to bulk format change patches on new versions. Also, it ends up being critical that all users run exactly the same version locally, which if people are rebasing and playing catch up ends up getting in folks way.

Is making people get through hound really being problematic?

[balloobbot]

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates.

Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment :+1:

[balloobbot]

This issue will be auto-closed because there hasn't been any activity for a few months. Feel free to open a new one if you still experience this problem 👍