Use Black

[jarrodmillman]

Description

Fixes #7195. I use black for almost every other project I work on and I have become used to its formatting and prefer to rely on it rather than spending my time thinking about formatting. The discussion on the developer's list seemed to be generally ok with using black with some minor concerns. I decided to just make the PR and see what happens.

In e7f6abc, I told black to avoid reformatting some np arrays. I can remove that or redo it to include more. I just didn't want to spend too much time on this PR until I got some feedback from others.

[jarrodmillman]

The CI failures are unrelated.

[stefanv]

I am in favor, given that we can always swap out for a different formatter in the future. The biggest benefit is that we can stop thinking about formatting.

/cc @scikit-image/core

[jni]

I'm just gonna stand aside on this one. 😂 But I will say I'd rather we used -l79...

[lagru]

Very much in favor and very curious to try it out in practice. @jarrodmillman thanks for the "do-ocratic" approach.

[lagru]

I assume that you plan to make a merge commit? Otherwise this hash will not be current once merged.

[jarrodmillman]

Yes, but I am also expecting to have to rewrite history a few times before we merge.

[lagru]

Did you comb through the entire diff or did you use an automated process to find these? I am happy to include these exceptions were it makes sense.

[jarrodmillman]

I just visually scanned the git diff and added it for a few examples that I thought looked better before. I am not sure if we want to do that more or less than I already did.

[mkcor]

@jni wrote:

I'm just gonna stand aside on this one. 😂 But I will say I'd rather we used -l79...

#5006 (comment) for the record 😅

@stefanv wrote:

I am in favor, given that we can always swap out for a different formatter in the future. The biggest benefit is that we can stop thinking about formatting.

Here's a shy 👍 from me. Basically I support this move (thanks, @jarrodmillman) as long as we can include exceptions such as #5006 (comment).

[jarrodmillman]

This tells black to leave quotes as is.

[jni]

Is everyone ok with 90c line lengths?

[soupault]

I personally do not mind. 90c has become quite common as the default setting across the popular text editors.

[stefanv]

I much prefer it.

[jarrodmillman]

I prefer having the 8 or 9 extra characters per line.

[stefanv]

I mainly wanted to highlight the colorful term: "80 chars line limit olympics" :)

[jni]

Look if you want to abandon the commitment to excellence and olympic glory and settle for the mediocrity of the "Black 88 char line regional athletics meet", be my guest. 😜

[stefanv]

It is well known that our eyes, by default, can only take in 50-75 characters without jumping. That is the next axis along which we need to train ourselves to improve. For that purpose, I suggest decreasing font size by 9.1%, while keeping any existing terminal layout.

[stefanv]

If we fail to retrain our biology, there is still hope: since Black is going to do Whatever It Wants with our code anyway, there is no reason not to use an automated tool to rewrap to 80 characters before editing! We call this Virtualized Wrapping. The robustness of such a roundtripping approach is left as an exercise for the developer.

[jni]

That's actually pretty genius @stefanv! I don't ever have to look at black formatting again if I set up my post-checkout hooks correctly! 😍

[jarrodmillman]

See astral-sh/ruff#7310 (comment)

[jarrodmillman]

Before merging this, I wanted to check on how many lines exceed 79 before and after applying this PR. This isn't meant to change anyone's mind, I was just curious. I wrote this little script (counter.py):

import os, shutil

directory = "/home/jarrod/src/scikit-image"
filelist = []

for root, dirs, files in os.walk(directory):
    for file in files:
        filelist.append(os.path.join(root, file))

count = 0
total = 0
for fname in filelist:
    if fname.endswith(".py"):
        with open(fname) as f:
            for line in f:
                total += 1
                if len(line) > 79:
                    count += 1

print(count)
print(total)
print(count/total)

Here is the output on main:

(skimage) [jarrod@x111 scikit-image (main)]$ python counter.py 
3077
189903
0.016203008904546005

Here is the output on this branch:

(skimage) [jarrod@x111 scikit-image (black)]$ python counter.py 
4412
197062
0.022388892835757273

So this PR increases the percentage of lines longer than 79 characters by about 0.6% from 1.6% to 2.2%.

[jarrodmillman]

I also decided to check the branch with the line length set to 79 instead of the default 88.

Here is the output:

(skimage) [jarrod@x111 scikit-image (black2)]$ python counter.py 
2820
202729
0.013910195383985517

So when we tell black to wrap at 79 characters, we still have 1.4% of the lines longer than 79 chacters. I figured this was a better comparison than to the main branch, since we currently aren't enforcing any line length limits.

[jarrodmillman]

I am going to rebase this, rerun black, and update the git-blame-ignore hash tomorrow morning. After that I plan to merge this. Please let me know if you want me to wait.

[lagru]

Thanks @jarrodmillman! I'm excited for git pull upstream main now. :D

[lagru]

I do see a few places which we missed and might want to clean up over time (e.g. _skeletonize.py#L219-L741) or using a simple script to scan for likely candidates. But feel okay with giving that a low priority. :)