Use black to format code

[JarnoRFB]

Closes #576

[coveralls]

Coverage remained the same at 85.306% when pulling 94567df on JarnoRFB:feature/black into 2b1e756 on IDSIA:master.

[JarnoRFB]

Python 3.5 seems to be a culprit again, as it cannot parse trailing commas after **kwargs, which are currently inserted by black even though they should not be. I therefore formatted the affected files and turned the formatting off afterwards (only three files where affected). I would leave it this way until the bug is fixed in black (or I find out what I am doing wrong).

[gabrieldemarmiesse]

What if you put python3.5 here? Would that fix the issues?

[JarnoRFB]

Just fixed the issue by using the black from the master branch in the pre-commit hook. language_version: here just refers to the python version of the virtual env in which the precommit hook is run. The python version that black supports is configured in pyproject.toml

[boeddeker]

Small question: Is there a reason not to use --skip-string-normalization for black?
It feels that many developers use ' instead of " what black proposes and in practice this doesn't matter and is only a burden for contributors.

For example in sacred are more lines that contain ' than ":

$ grep -r '"' * | wc -l
1653
$ grep -r "'" * | wc -l
3090

[gabrieldemarmiesse]

@boeddeker I agree with that. Our goal when using black was mainly to avoid pushing commits to fix pep8. If we use single quotes or double quotes doesn't matter for pep8 and doesn't change the structure of the code. Also, the diff of the current PR will be much smaller if we do that.

[JarnoRFB]

@boeddeker I would say the whole purpose of black is to have consistent formatting for the whole python community. I also used to use single quotes and at the beginning the felt very strange to now have double quotes all over my code. However, I quickly got used to it and I believe that consistency in the community is more valuable than personal preference.
From the black readme

If you are adopting Black in a large project with pre-existing string conventions (like the popular "single quotes for data, double quotes for human-readable strings"), you can pass --skip-string-normalization on the command line. This is meant as an adoption helper, avoid using this for new projects.

As we do not have any of those conventions, I see no benefit in sticking with the current quoting.

If you still like to write your code using single quotes you can do so and the precommit hook will take care of formatting your code.

[gabrieldemarmiesse]

I actually don't have any strong feeling from either sides after thinking about it. As long as I don't have to push commits for pep8, I'll be happy.

[JarnoRFB]

So from my side this would be ready now.

[gabrieldemarmiesse]

we should really drop appveyor, it takes forever to run.

[gabrieldemarmiesse]

should we add black here? when does it get downloaded?

[JarnoRFB]

I first had it there, but then the py35 build fails, because it installs the dev-requirements but black needs py36 to run.

[JarnoRFB]

so black will get downloaded when you commit the first time after running pre-commit install. It is installed in its own virtual environment that does not contain anything else.

[gabrieldemarmiesse]

I see, that makes sense. I'll merge it then.

[boeddeker]

@JarnoRFB Thanks for the explanation.
There is at least one reason to use the --skip-string-normalization:
Many people tend to use ' and it is uncommon to install a pre commit hook.
So new people that want to contribute to sacred have the burden to follow the exact black style.