zerasul/blask

Use Black to enforce code style

klashxx opened this issue · 11 comments

Check: https://github.com/psf/black

The following notable open-source projects trust Black with enforcing a consistent code style: pytest, tox, Pyramid, Django Channels, Hypothesis, attrs, SQLAlchemy, Poetry, PyPA applications (Warehouse, Pipenv, virtualenv), every Datadog Agent Integration.

I've been watching Black since few weeks ago and I'm curious about it. Go for it.

thanks for the help. Im installing black and using it with VSCode integrated formatting tool.

If no one else is working on this issue, can I give it a go? I see that right now pylint is being used for linting in the GitHub Actions, I guess that the idea is changing that from pylint to black?

only if you can make it works on a ps2...

No promises!

@zerasul before making the pull request, I have a couple of questions/caveats:

  • Do we want to use the defaults or do we want to change anything?
  • Reformatting all the codebase to comply with Black style is going to ruin the git blame output in GitHub. There is an open issue regarding that, to support --ignore-revs-file in GitHub so this massive reformatting doesn't clutter up blame output. But it's yet unanswered. Should we go on with the change in light of this?

@Elvirarp92:

  1. we can use defaults for this project
  2. In this case we didn't use git for the formating we can use this tool only for information or introduce it as a local development tool (integrating it in vscode for example). What do you think that can be the best choice?
AABur commented

I analysed the code with black --check -t py37 --diff --color

black will change:

  • quotes from ' to ".

  • `lengthen' some of the lines that are now wrapped

Black defaults to 88 characters per line, which happens to be 10% over 80.

Finally, 11 files would be reformatted, 5 files would be left unchanged.

You need to decide whether the git blame clutter on GitHub is that important. Do you often use the blame in the GitHub GUI? Furthermore, there is some hope that this issue will be resolved at some point.

AABur commented

Step 1. Run linter on current code

pipenv run linter

************* Module blask.blaskcli
blask/blaskcli.py:95:15: C0209: Formatting a regular string which could be a f-string (consider-using-f-string)
blask/blaskcli.py:139:19: C0209: Formatting a regular string which could be a f-string (consider-using-f-string)
************* Module blask.blaskapp
blask/blaskapp.py:50:0: C0303: Trailing whitespace (trailing-whitespace)
************* Module blask.blogrenderer
blask/blogrenderer.py:74:12: W0707: Consider explicitly re-raising using the 'from' keyword (raise-missing-from)
blask/blogrenderer.py:84:12: W0707: Consider explicitly re-raising using the 'from' keyword (raise-missing-from)
blask/blogrenderer.py:129:27: W0108: Lambda may not be necessary (unnecessary-lambda)
blask/blogrenderer.py:178:29: W0108: Lambda may not be necessary (unnecessary-lambda)
blask/blogrenderer.py:226:0: R0902: Too many instance attributes (9/7) (too-many-instance-attributes)
************* Module blask.blasksettings
blask/blasksettings.py:60:12: C0206: Consider iterating with .items() (consider-using-dict-items)
blask/blasksettings.py:68:8: C0206: Consider iterating with .items() (consider-using-dict-items)
blask/blasksettings.py:78:23: C0209: Formatting a regular string which could be a f-string (consider-using-f-string)

-----------------------------------
Your code has been rated at 9.63/10

Step 2. Reformating with black

black -t py37 -v .
                                                                                                                        
.pytest_cache/CACHEDIR.TAG ignored: matches the .gitignore file content
.pytest_cache/README.md ignored: matches the .gitignore file content
.pytest_cache/.gitignore ignored: matches the .gitignore file content
.pytest_cache/v ignored: matches the .gitignore file content
.coverage ignored: matches the .gitignore file content
coverage.xml ignored: matches the .gitignore file content
.git ignored: matches the --exclude regular expression
.vscode/settings.json ignored: matches the .gitignore file content
blask/__init__.py wasn't modified on disk since last run.
blask/errors.py wasn't modified on disk since last run.
tests/__init__.py wasn't modified on disk since last run.
reformatted settings.py
reformatted main.py
setup.py already well formatted, good job.
reformatted tests/settings.py
reformatted tests/blaskcli_test.py
reformatted tests/testsettings.py
reformatted blask/blasksettings.py
reformatted tests/settings_test.py
reformatted tests/main_test.py
reformatted tests/blogrender_test.py
reformatted blask/blaskcli.py
reformatted blask/blaskapp.py
reformatted blask/blogrenderer.py
All done! ✨ 🍰 ✨
12 files reformatted, 4 files left unchanged.

Step 3. Run linter on reformated code

pipenv run linter                                                                                                                           

************* Module blask.blaskcli
blask/blaskcli.py:95:15: C0209: Formatting a regular string which could be a f-string (consider-using-f-string)
blask/blaskcli.py:146:19: C0209: Formatting a regular string which could be a f-string (consider-using-f-string)
************* Module blask.blogrenderer
blask/blogrenderer.py:76:12: W0707: Consider explicitly re-raising using the 'from' keyword (raise-missing-from)
blask/blogrenderer.py:86:12: W0707: Consider explicitly re-raising using the 'from' keyword (raise-missing-from)
blask/blogrenderer.py:130:27: W0108: Lambda may not be necessary (unnecessary-lambda)
blask/blogrenderer.py:178:29: W0108: Lambda may not be necessary (unnecessary-lambda)
blask/blogrenderer.py:226:0: R0902: Too many instance attributes (9/7) (too-many-instance-attributes)
************* Module blask.blasksettings
blask/blasksettings.py:61:12: C0206: Consider iterating with .items() (consider-using-dict-items)
blask/blasksettings.py:69:8: C0206: Consider iterating with .items() (consider-using-dict-items)
blask/blasksettings.py:79:23: C0209: Formatting a regular string which could be a f-string (consider-using-f-string)

------------------------------------------------------------------
Your code has been rated at 9.67/10 (previous run: 9.63/10, +0.03)

Step 3. Chow changes

git diff --stat                                                                                                                                  

 blask/blaskapp.py        | 42 ++++++++++++++++++++++--------------------
 blask/blaskcli.py        | 33 ++++++++++++++++++++-------------
 blask/blasksettings.py   |  7 ++++---
 blask/blogrenderer.py    | 38 +++++++++++++++++++-------------------
 main.py                  |  9 ++++-----
 settings.py              |  2 +-
 tests/blaskcli_test.py   | 16 ++++++++--------
 tests/blogrender_test.py |  3 +--
 tests/main_test.py       |  2 +-
 tests/settings.py        | 13 +++++--------
 tests/settings_test.py   | 27 +++++++++++++--------------
 tests/testsettings.py    |  8 ++++----
 12 files changed, 102 insertions(+), 98 deletions(-)
AABur commented

@zerasul, what is your decision? I can handle this issue.

Working in #237 i will review this PR and merge into develop