Code style enforcement

[janosh]

Closes #208.

Feel free to make edits, suggest changes (e.g. do you prefer different config options? note that black enforces double quotes and this is non-configurable) or close this PR outright as it wasn't that much work. Just thought I'd help get the ball rolling on #208.

I only ran autoformatting on a fairly random subselection of files. If you think this could eventually be merged, I'm happy to add all remaining unformatted files.

Tests passing locally:

============ 65 passed, 1 skipped, 774 warnings in 714.06s (0:11:54) ============

[ardunn]

Hey @janosh thanks for another PR!

I am actually working on some fairly significant changes to automatminer which I will merge in the coming days so I will close this PR (since there would be lots of merge conflicts to resolve) and do the formatting after.

What would be helpful is sharing the config files/extra commands you used to run black+flake8 (if they are different from the snippet in #208 ). I haven't used a code formatter before so that would help out immensely once I do the formatting

[janosh]

I am actually working on some fairly significant changes to automatminer which I will merge in the coming days...

Cool, looking forward to that! Feel free to ping me afterwards about the config files or about recreating this PR.

[ardunn]

hey @janosh, I pushed most of the changes I was talking about, so I am all ears for enforcing code formatting!

[janosh]

@ardunn Very cool! Thanks for the new release.

I pulled all your changes, resolved conflicts and pushed a new batch of example files. Let me know what you think about the style. Maybe @utf also wants to chime in. Happy to change the config.

Also, which editor are you guys using? It's a good idea to enable format on save to continuously ensure style conformity. Otherwise you're likely to be hit with a plethora of style violations blocking you once you're ready to commit your changes.

I'm working with VS Code where I added the following settings to auto-format files on save.

"editor.formatOnSave": true,
"python.formatting.provider": "black",
"python.linting.flake8Enabled": true,
"python.linting.pylintEnabled": false,
"python.sortImports.args": [
  "--settings-path=${workspaceFolder}/setup.cfg"
],
"editor.codeActionsOnSave": {
  "source.organizeImports": true
},

Other editors probably have similar settings...

[ardunn]

@janosh looks great! I'll take a detailed look at it in the coming days

Most of our group uses PyCharm. I mainly have just used the default code formatting so I'm not sure how to change it to use black/flake8, but I'm sure there is a way.

One thing I noticed on a quick skim is that black sets the character length at 88 while PEP8 is 79? If possible I'd like to keep the character length at 79. It doesn't seem like the files in this PR have line lengths of 88 but maybe worth configuring anyway. This blog post https://ljvmiranda921.github.io/notebook/2018/06/21/precommits-using-black-and-flake8/ suggested using a .toml file to set the line length

[janosh]

@ardunn Funny that you mention that post by Lj Miranda. I remember reading it a few months ago.

Regarding the line length, it's currently configured in setup.cfg as 100. I changed it to 79.

[flake8]
max-line-length = 100
exclude = .git
# E731: do not assign a lambda expression, use a def
# W503: line break before binary operator
ignore = E731, W503

# Make isort play nicely with black's import formatting.
# https://github.com/microsoft/vscode-python/issues/5840
[isort]
multi_line_output = 3
include_trailing_comma = True

I'd advocate for keeping setup.cfg in favor of pyproject.toml since the former is cross-tool, has been there for years and hence is tested and stable.

Also, I added instructions on setting up the pre-commit hook before submitting a PR to CONTRIBUTING.md.

[janosh]

Fyi, black explicitly advises against a max line length of <= 80.

[ardunn]

thanks @janosh !

[janosh]

@ardunn You welcome. Out of curiosity, I just pulled all your latest changes and let my editor auto-format all .py files in this project. Ended up with over 50 changed files, all of them a few lines shorter than they are atm. Interested in a PR for that?

[ardunn]

Sure sounds good by me! Are you just calling the black command with no args? Btw I set up a CI to check for code flake8 compliance, if CI fails it’s likely because of that (you just need to make sure flake8 doesn’t have any errors)

…

On Tue, Oct 22, 2019 at 5:30 AM Janosh Riebesell ***@***.***> wrote: @ardunn <https://github.com/ardunn> You welcome. Out of curiosity, I just pulled all your latest changes and let my editor auto-format all .py files in this project. Ended up with over 50 changed files, all of them a few lines shorter than they are atm. Interested in a PR for that? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#245?email_source=notifications&email_token=AEYDHS2RYEDXJLP7TOMYHXLQP3W5DA5CNFSM4I74JLR2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEB5RU2A#issuecomment-544938600>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEYDHS3EWZCZ5QKWGNQHTX3QP3W5DANCNFSM4I74JLRQ> .

[janosh]

Are you just calling the black command with no args?

Yep, black with no args along with isort for sorting import statements.

you just need to make sure flake8 doesn’t have any
errors

Will do.