black run over mock files

[cjw296]

Putting this up to see what people think. @mariocj89: you too!

I like it overall, the only rough edge is that we'd need to find a way to tell coverage to ignore all

def foo(..):
    pass

methods on the backport. @nedbat - can you think of any coverage config that would help with that?

If it's not possible, I could just exclude those from the diff.

I did try just sorting out the mishmash of different whitespace between methods, classes, etc by hand but that ends up being pretty arduous.

[tirkarthi]

Sorry, I am -1 on this. Formatting changes make git blame less useful though there are mechanisms to skip the black formatted commit. I would recommend having this approved by other core developers in mailing list or discourse since the general notion towards these in the past is not to do this since it introduces code churn with little benefit.

[cjw296]

Interestingly, GitHub's blame makes it relatively easy to "drill back", and there's not many actual code lines changed by this. I wish there was a way to just have black sort out the whitespacing issues, which are the ones I'd really like to fix.

[tirkarthi]

Git also has the option to store commits to always skip during blame : https://git-scm.com/docs/git-blame#Documentation/git-blame.txt---ignore-revs-fileltfilegt . It also will be a thing for future maintainers where git blame points to this commit initially for pretty much all the lines. I understand the need for consistency but there are also many parts where things are not formatted with pep8 that receive occasional patches that are rejected with similar reason. I would defer to the other devs on core team for this.

[cjw296]

"git blame points to this commit initially for pretty much all the lines" - hmm, not sure you've seen the diff... there's actually surprisingly few non-whitespace lines that are affected by this process.

Maybe not surprising, this codebase has been PEP8 since it was created, so there's not that much for black to correct ;-)

[tirkarthi]

I looked into diff stats it changes single quotes to double quotes, formats the signature of methods, changes formatting of multiline boolean statements, etc. For mock.py it shows 665 additions and 457 deletions. There are also tests which makes the diff to 2634 additions and 2200 deletions as I can see in GitHub. Empty lines also serve as some distraction. There were changes made to source tree like converting tabs to spaces, test refactors, etc. under various timelines. But I find this code churn to be less useful.

[cjw296]

Right, but most of those are whitespace.
I also discovered that we can tell black to leave the quotes alone too, which I'd be fine with (it's one of two parts of black I really dislike!)

[cjw296]

Okay, he's another take, this time without the string changes.

[ambv]

I'm rejecting this for now as it's too early to introduce Black to core Python code:

• the tool hasn't reached a stable version yet;
• auto-formatting a bunch of files without creating a way to keep the autoformatted code consistent in the future will cause the formatting to deviate.

Proper introduction of auto-formatting to core workflow will require a PEP.

[cjw296]

Yeah, it's just a shame as the formatting, particularly whitespace, is so egregiously bad in mock.py.