opensistemas-hub / osbrain

osBrain - A general-purpose multi-agent system module written in Python
https://osbrain.readthedocs.io/en/stable/
Apache License 2.0
175 stars 43 forks source link

Black #343

Closed Peque closed 4 years ago

Peque commented 4 years ago

Using Black formatting now that we are integrating the tool in other internal projects.

@ocaballeror We all knew this day would come. :joy:

Closes https://github.com/opensistemas-hub/osbrain/issues/277.

codecov[bot] commented 4 years ago

Codecov Report

Merging #343 into master will decrease coverage by 0.02%. The diff coverage is 99.53%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #343      +/-   ##
==========================================
- Coverage   99.27%   99.24%   -0.03%     
==========================================
  Files          26       26              
  Lines        3573     3573              
  Branches      259      259              
==========================================
- Hits         3547     3546       -1     
  Misses         14       14              
- Partials       12       13       +1
Impacted Files Coverage Δ
osbrain/logging.py 100% <ø> (ø) :arrow_up:
osbrain/tests/test_bugs.py 90.9% <0%> (-9.1%) :arrow_down:
osbrain/nameserver.py 99.21% <100%> (ø) :arrow_up:
osbrain/tests/test_agent_serialization.py 100% <100%> (ø) :arrow_up:
osbrain/tests/test_agent_transport.py 97.59% <100%> (ø) :arrow_up:
osbrain/tests/test_timer.py 99.02% <100%> (ø) :arrow_up:
osbrain/tests/test_agent_async_requests.py 100% <100%> (ø) :arrow_up:
osbrain/tests/test_agent.py 100% <100%> (ø) :arrow_up:
osbrain/tests/test_agent_sync_publications.py 100% <100%> (ø) :arrow_up:
osbrain/tests/common.py 100% <100%> (ø) :arrow_up:
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d0b241d...39e656f. Read the comment docs.

ocaballeror commented 4 years ago

Yay! I love black (for the most part), I've been using it in all my projects for a few months now.

However, I'm really not a fan of the string normalization feature. It doesn't really enforce the PEP8 standard, and it forces you to use double quotes everywhere, which generates a ton of changes everywhere. Also, they look uglier, at leas to me.

I'd say we add the skip-string-normalization option to pyproject.toml

Peque commented 4 years ago

@ocaballeror I prefer single quotes too. Wanted to be more impartial and not make that decision based on my own preferences. You may have given me an excuse to leave the quotes as they were before... :stuck_out_tongue_winking_eye:

I will probably revert that commit tomorrow (it is separated from the other Black-formatting changes).

ocaballeror commented 4 years ago

I know, black is meant to be used with all its defaults so that there is a consistent style across the board and you don't have to decide anything. 

However, since quotes are down to personal choice anyway, I'd rather keep OUR personal choice rather than Black's developers. 

Also, it's not like we're not enforcing a consistent quoting style in this project, we have a dedicated flake8 plugin just to ensure everything follows a standard. 

Sep 23, 2019, 23:44 by notifications@github.com:

@ocaballeror https://github.com/ocaballeror> I prefer single quotes too. Wanted to be more impartial and not make that decision based on my own preferences. You may have given me an excuse to leave the quotes as they were before... 😜

I will probably revert that commit tomorrow (it is separated from the other Black-formatting changes).

— You are receiving this because you were mentioned. Reply to this email directly, > view it on GitHub https://github.com/opensistemas-hub/osbrain/pull/343?email_source=notifications&email_token=AE4EDLWR3T6DGYH72QBWDH3QLE2CZA5CNFSM4IYWEYPKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7MLXHQ#issuecomment-534297502> , or > mute the thread https://github.com/notifications/unsubscribe-auth/AE4EDLS6CSM62CHF2FBZLDLQLE2CZANCNFSM4IYWEYPA> .

Peque commented 4 years ago

@ocaballeror Done.

Until there is an official recommendation (i.e.: PEP), we will stick to single-quote style. Not only to match our preferences, which is probably a weak argument, but to also minimize the number of LoC modified for the style change, since we were already being consistent in that respect.