Use black for zopefoundation repositories?

[icemac]

The Plone developers are planning to use https://pypi.org/project/black/ for upcoming Plone versions.

I see the following Pros:

• end of style discussions in PRs
• consistent style over all packages

and these Cons:

• I do not like some of the style decisions of black
• It would produce large diffs for the first time it is used.

We would have to decide about the few options black provides:

• line length: default of 88 or keep 80 as currently used
• --skip-string-normalization to keep the diffs smaller
• or going with the defaults which would allow not to write a separate black config per repos

It would require the following work in each repository:

• Maybe configure black resp. configure Flake8 to go well with the decided black configuration
• Run black on the code.
• Add black to the checks run via tox and in TravisCI in addition to Flake8 to keep the code black.

Any opinions for this proposal?

[dataflake]

What problem are we trying to solve with yet more bureaucracy? I'm horrified by some of the defaults, and if you don't use defaults you have to suddenly go through and create configurations on all repositories? A strong -1 from me.

[jaraco]

What problem are we trying to solve?

Minimizing bikeshedding by enforcing styles mechanically in a way that's objectively popular (even if horrifyingly objectionable). It takes the subjectivity out of these inherently subjective concerns.

[dataflake]

I disagree. The subjectivity is just being moved from the project's own flake8 rule decisions to an outside entity. It feels like it's just easier to tell people who don't like the rules "hey, don't complain to us, we didn't make these rules, bitch at the black project".

Secondly, I don't know how it is on the Plone side, but here we do not have an issue with any noticeable disagreement to the current setup. This isn't a problem here so it doesn't need solving.

[dataflake]

P.S.: Just to clarify, I am speaking only for the zopefoundation repository side. If the Plone repository maintainers think this is a good idea for them, more power to them.

[jaraco]

From my perspective, it's best for a project to choose one of two philosophies: either allow all contributors to contribute in their preferred style or have a system that mechanically applies the style. Every compromise in between leads to subjective and manually-intensive application of whatever style is desired. Having spent a substantial portion of the past two years being forced by tools like flake8 to "try again", I'm utterly sick of that and am adopting black so I don't have to do that work any more.

[tseaver]

I'm -1e6 on the idea, myself. I've just lived through this for $DAY_JOB, and can state with perfect certainty that black does not fix the "try again" problem. Its choices are arbitrarily stupid, in plenty of cases, and you still have to remember to run it.

It will pretty much destroy using "git blame" to figure out why a change was made (the extra effort to remember how to get back beyond the "blackening day" commit means one doesn't try).

If you do go this route, certainly kill off the ridiculous 88 character line length and the string normalization (which doesn't even match the CLI's own usage!).

[dataflake]

I am sure there are plenty of tools to automatically apply e.g. the current flake8 config. That's not an argument for choosing yet another tool with yet other style rules, some of which are totally different from what we have been using for many many years.

Tres also identified the two items that I was horrified about, the line length and the string normalization.

[dataflake]

In the end (also referring to what I am currently enforcing for the whole developer team at my $DAY_JOB) what's most important is consistency and readability. These goals are served well by our existing tools and standards.

[mgedmin]

As someone who tries to review most PRs submitted to ZF's repos, I'd rather not have to review the huge initial diffs that Black produces. If y'all make the decision to use Black, can we agree to commit these directly to master? In theory it should be safe, yes? ;)

(As for my personal opinion, I'm -0.5 on the idea, but I won't stand in the way.)

[ale-rt]

I think adopting black (or whatever equivalent tool) has more pros that cons

I am just reporting my experience.

1. Running black on ~100k lines of Python code resulted in a green build (so it should be safe)
2. Working with "blacked" code is a better experience: you have a feeling that the things are more clean and you do not have to waste mental energies thinking about really silly things (which quotes should I use? single or double? Should I split this tuple on multiple lines? And other stuff like that).
3. Whenever you work on a project that still does not use black (or some auto formatter) you regret it because you will have the feeling that something useful is missing.

I also used to prefer some other tools rather than black see plone/Products.CMFPlone#2754 (comment), but the point is that any energy spent on this topic is energy drained out from something probably more important.

[dataflake]

There's no energy being spent here. What we have does the job well enough and I have not seen any purported discussions about code style that could be squashed by shifting blame to a different tool.

[jugmac00]

A couple of annotations:

you have to remember to run black

No, if you configure pre-commit, which "all the cool kids do nowaydays" (I have not tried it yet).
https://pre-commit.com/

you destroy git blame

Well, like 99% of the time when I do git blame see something like "This is the real code", "Get the code" or any other 10+ year old commit message, at least at that areas I worked on.

Also, at least PyCharm (and maybe other IDEs, too) supports going back more than one commit to view the history of a line.

we have flake8

As PyCharm has some sensible checks, I never used flake8 before.

I just installed it and ran it on Products.MailHost, which I plan to update today.

And all I got was this:
"pyflakes" failed during execution due to "'NoneType' object has no attribute 'parent'"

So. Either "flake8" was not run at this project or my flake8 installations (VirtualEnv with py2.7 / py3.6 and via pipx) are all broken. Or flake8 is buggy.

So, if we agree upon flake8 is enough, it should be documented at least somewhere.

At PyConWeb in Munich this weekend there were a couple of interesting talks, amongst some on black ( https://gist.github.com/zupo/eb6dbfb98bc7548adb8d9208a8e3f57a ) and one on how a guy created a new web framework.

The interesting part was that he explained how he tried to encourage contribution and the creation of a community, and one point was to create a good "CONTRIBUTING" document, which we currently lack.

The slides:
https://github.com/florimondmanca/talks/blob/master/2019_05_25-bocadillo_pyconweb2019.pdf

The article:
https://blog.florimond.dev/how-i-built-a-web-framework-and-became-an-open-source-maintainer

His CONTRIBUTING document:
https://github.com/bocadilloproject/bocadillo/blob/master/CONTRIBUTING.md

That all said.... I am a lot less emotionally touched by this decision.

If we go for it, very fine, if we do not go for it (yet), well, that's ok, too.

I think in 5 years nobody will talk about it, as everybody will use it.

[dataflake]

Jürgen you don't install anything to run the flake8 checks. You run the buildout and then bin/tox. It's been like that literally for years now.

[jugmac00]

Jürgen you don't install anything to run the flake8 checks. You run the buildout and then bin/tox. It's been like that literally for years now.

Thanks! bin/tox -e=flake8 actually works!

[jugmac00]

So, I created a pull request for Products.MailHost, which failed at first because of flake8 issues, as Products.MailHost enforces single quotes - which e.g. Zope and other projects do not.
https://travis-ci.org/zopefoundation/Products.MailHost/jobs/538223097

Something like this would not have happened if we went for black with defaults.

The one key difference between flake8 and black is:

• flake8 tells you what to do
• black does it for you

[dataflake]

@jugmac00 the single quotes are enforced for consistency. That whole codebase uses them. black tells you what to do as well, and with even stranger defaults. I don't see the difference.

Even if you're rightfully arguing that we have diverging flake8 configurations across zopefoundation repositories, black is not a magic panacea for that, sorry.

To avoid Travis CI runs going red you run bin/tox like everyone else and fix the issues. This is a normal workflow.

Again, I believe black is answering a question that has not been asked on our zopefoundation side.

[jugmac00]

black tells you what to do as well

Black does not tell me what to do, it does it for me.

When I run black (or the pre-commit hook does it for me), I do not get an output like line 200: you have to use double instead of single quotes, I get a modified file which I just need to commit.

But as I said, I do not strongly lean towards one or the other side. I am fine with flake8, too - as long as we try to use it consistently over the zopefoundation repos.

[dataflake]

Most of the actively developed packages have a flake8 linting configuration. They differ in details, as you noticed yourself.

You'll probably never have a situation where you reach complete consistency across all repositories, unless someone spends a lot of time setting it up/comparing it for all repositories. IMHO that's a non-goal, anyway. No one should spend time updating dead packages, of which there are a lot.

At that point I'd be back at my question: Where is the actual problem we are trying to solve that is not solved by what we have right now?

[strichter]

We have evaluated black for Shoobx code and I have veto'ed its use. Since it indiscriminately formats everything, a lot of my purposefully grouped formatting would be lost. Also, I do not buy the premise of minimizing merge diffs since it increases vertical space a lot. And I read code much more than merge diffs.

On the other hand, given the chatter at the Language Summit a few weeks back, it is pretty much a lost battle at this point.

[icemac]

As black now lives in the Python GitHub organisation it seems to be more adapted by the core developers. Even some big projects are already using it.

For now I am going to close this issue as there is no consensus about using black. You guys seem to share most of the disadvantages I see.

Maybe we should try it in a small project before thinking about using it on a larger code base.
Let's see what the next years have to offer.