Paint pyexplabsys black

[KennethNielsen]

This PR introduces another of my post-SurfCat tool chain must-haves: black.

black famously is "the uncomprising code formatter" or as their tag line goes "Any color you like, as long as it is black".

The idea with a code formatter, quite simply is, that a consistent coding style stream-lines the reading experience across the entire code base and therefore increases productivity (and even across projects, if they use the same formatter and at this point black has taken the Python word by storm). Using a formatter, makes sure that it is consistent, and even just sometimes makes it easier to make formatting changes that you know would be an advantage, but may be too lazy to do by hand. black integrates nicely into most editors and IDE's, so you can have you code automatically reformatted every time you save. As an extra advantage, you get all of this, without ever needing to discuss the formatting that black handles in a code review.

As an examples, consider a function definition. If the whole thing fits on one line, black will format as such:

def myfunc(a, b):

If there are so many arguments (or so long a function name), that it will not fit on one line, but splitting it up to 3 lines, with a line for the arguments on it own (whereby you buy the space of the function name) works, then it does that:

def my_function_with_a_very_long_name(
    foo, bar, baz, default1="Default argument 1", default2=(1, 2, 3, 4, 5), 
):

Finally, of course, if even the arguments on a line of its own is too long, it breaks it down to one line per argument:

def my_function_with_a_very_long_name(
    foo,
    bar,
    baz,
    default1="Default argument 1",
    default2=(1, 2, 3, 4, 5), 
):

thereby, providing the best compromise between lines used and readability of code and diffs.

black does something similar to the formatting of lists and dicts, it ensure spaces around operators and assigment = and so on and so on.

"So, how is it different from something like pylint?". Well, pylint for one, doesn't fix anything, it just highlights what it considers problems. But more to the point, black concerns itself only with the format, not the content, so black has no opinions on variable names or the number of methods in a class. It will re-format the code exactly with the exact same content as when you wrote it. In general it has a much smaller scope, but a really important part of the scope.

"Is it safe?": Yes, black forms the AST of the code before making changes, then reformat into a temp file and once again create the AST and only if they match, will it copy the temp file into place. By this mechanism the authors of black ensures that they will not break code by bugs in the formatter.

"What do you mean by uncompromising?". black is opinionated and has only a very few options, like line length, but other than that you get what they offer. This may sound restricted, but the huge advantage of that is that you will never end up in discussions with other developers about what the one true format is.

"What if I don't like that style?". Then black isn't for you. But what I can can say from my own experience is that, while I care about code formatting and style (as some of you know) I care much more about it being applied and consistent, that me getting to decide it. When I adopted black the first time, I thought exactly that I generally liked the idea, except for that one part of the style that I wished was different. And since I couldn't change it, I considered abandoning the idea. About 2 weeks passed and then I had adopted the new style as my own and I have never looked back since.

So what is in this PR?

1. I reformatted all the source code in PyExpLabSys that parses as Python 3, which is the reason for the 3 files being excluded in the configuration. Due to the security concern mentioned above, black will not reformat source code python (3) cannot parse.
2. I went back and switched the formatting off* around a few pieces of source code when e.g. extra spaces was added for readability. I should say, that it is possible that I missed some. I quickly glanced the massive diff and grepped in the original source code to try and find them, but it is possible.
3. I added invoke tasks for check that the code is black formatted and to actually black format it.

• Switching off the formatter around certain pieces of code is done by:

# fmt: off
...  # This code will not be formatted
# fmt: on

Besides from the integrations I already added, black also fits nicely into pre-commit checks and CI on Gitub Actions (to come if you like the idea of black).

Let me know what you think.

[KennethNielsen]

Following the discussion on Slack I would like to clarify a few things here (copy from Slack in Danish):

1. Det (måske) eneste den redigerer, er whitespace mellem braces og udenfor strenge, altså den slags whitespace som er semantisk ligegyldig

2. (Måske) stammer fra at der muligvis er en ændrig ift. nylinje i slutningen af kommentarer

3. Før og efter har identisk Abstract Syntax Tree og vil derfor evaluarere til præcis det samme

4. Er filens syntaks ugyldig, altså f.eks. hvis man kører det mens man redigerer, sker der intet, kva 3

5. Som en af den få ting kan man ændre accepteret linjelængde, hvilket selvfølgelig påviker hvor man steder man får flere linjer fordi argument/element-lister ikke kan være på en linje

Har man brug for ekstra whitespace, f.eks. for a aligne sådan nogen som dette:

# fmt: off
d["a"]       = 10
d["bla_bla"] = 20
d["c"]       = 30
# fmt: on

kan man gøre det med fmt: off og fmt: on men jeg tror jeg kan tælle på fingre hvor mange gange jeg har gjort det, så her er ikke tale om sådan en ting mere som kommer til at smudse ens kode til ligesom pylint: disable. EDIT: Jeg har lige tjekket, i den ene arbejdskodebase, som er 6000 linjer, er der 3 stk.

Hvad angår hvor meget længere den gør filerne ift. til de formatteringsændringer den laver, skal det nævnes at den totale diff i dette PR er +10,775 −8,658.

En af de få andre ting, end linjelængde, som man kan indstille, er hvorvidt den skal normalisere strenge (altså om der bruges " eller '). Slår man dette fra, er diffen 6245 insertions(+), 4314 deletions.

Ligsom på Slack vil jeg dele at glæden jeg har haft med black er todelt. Som maintainer af open source projecter og vores arbejdsprojekter. Det er steder hvor jeg går op i at der er en konsistent stil, men hvor jeg samtig ikke sønderligt bryder mig om at håndhæve den. PR'er hvor man er nødt til at håndhæve stil er hårde at lave, tager energi fra den egentlige funktionalitet og tager lysten fra forfatteren, hvis ikke lige han bliver betalt for det. Som programmør har det ændret den måde jeg arbejder på, hvor, selvom jeg går op i konsistent stil, jeg nu ganske enkelt ikke bruger nogen søndelig energi på det. Jeg er endda endt der hvor jeg eksplicit skriver kode i den formattering som er hurtigst/nemmest at skrive, men grim, og så lader jeg bare black tage sig af at fikse det for mig. Jeg værgede mig i starten ved at give slip på det niveau af kontrol, men da jeg så havde gjort det virkere det ikke længere så vigtigt og det frigav energi til indholdet. Af disse årsager er det bedste jeg kan foreslå, hvis i er interesserde, at i prøver det. Men giv det gerne 2 uger, ellers når man ikke over den del hvor det føles forkert.

VIGIGT: Slutteligt vil jeg sige at jeg blev klar over at jeg nok pushede black lige hårdt nok på Slack. Det er vigtigt at vi er på samme side omkring det her. Jeg er på nuværende tidspunkt kun perifært involveret i PyExpLabSys, uden udsigt til at det vil ændre sig. Jeg deler derfor ikke dette som et bud på hvordan jeg gerne vil arbejde med PELS (for det kommer jeg sikker ikke til), men blot som at jeg ganske enkelt vil dele noget som jeg har haft stor glæde af. For at sådan en ændring her giver mening, er det nødvendigt at de af jer der arbejder med det til daglig synes det er en god idé, ikke blot at I synes at det ikke er en forfærdelig ide.

Lad os se hvor det ender og så vil jeg udskyde det næste PR jeg arbejdede på omkring toolchain, idet der var mere kildekoderedigering, omend i den endnu lettere ende (pre-commit fixers).

☮️

[robertjensen]

I am quite willing to accept the concept of black, provided that it is used with the --skip-string-normalization parameter.
In that connection, we should settle on a line-length which is apparently defined in both https://github.com/CINF/PyExpLabSys/blob/master/.flake8 and https://github.com/CINF/PyExpLabSys/blob/master/bootstrap/.pylintrc
I would personally vote for 85, but I also have no problems if people have other preferences, as long as we stay well below 100.

Is it correctly understand, that this PR is essentially a re-commit of all files after being run through black?

Anyway, in order to proceed, we will need to re-run with --skip-string-normalization

[Ejler]

I finally ran through many of the comments. I am still very strongly opinionated about the single/double quotes issue. Apart from that, I believe Black will generally be a nice addition. So with the --skip-string-normalization parameter, I have no issues with adopting Black.

Regarding line length, I have no real preferences. <100 still allows for split-screen on most (decent) monitors.