MAINT: Use black

[larsoner]

black seems to be where the community is going -- NumPy is planning on using it I think (?), and from a quick look it looks like pandas and scikit-learn already do or are headed in that direction. I'm sure @agramfort has opinions/experience to share here :)

It's possible to make a single big commit that makes all the changes, then add this to a git blame ignore file, which removes the potential blame pollution which (to me) was a big barrier to adoption.

So overall +1 for moving to black from me using whatever pre-commit hooks or whatever pandas and scikit-learn have done (despite me actually not particularly liking how black code ends up looking sometimes!).

I can volunteer to help people rebase or merge PRs that are broken by the changes if they need help.

cc @cbrnr who mentioned wanting longer than 79 char line length, this would be one way to do it.

EDIT:

• https://github.blog/changelog/2022-03-24-ignore-commits-in-the-blame-view-beta/
• https://github.com/scikit-learn/scikit-learn/blob/main/.git-blame-ignore-revs

[welcome]

Hello! 👋 Thanks for opening your first issue here! ❤️ We will try to get back to you soon. 🚴🏽‍♂️

[cbrnr]

Thanks for opening your first issue here @larsoner 😆!

I'm reluctant to go all Black. Of course this alleviates us from all style-related issues, but on the other hand we sometimes want to use custom formatting to improve readability. This is just not possible with Black.

My vote is to just allow a line length of 92 characters for new code, so no changes are necessary for existing code. Of course, people are then free to rewrap a whole file as part of some other change.

[agramfort]

you're so European minded Clemens :) I would say it's either 0 or 1. Either we go with Black and follow what sklearn / numpy do or we don't change. let's not be exceptions here.

…

Message ID: ***@***.***>

[cbrnr]

you're so European minded Clemens :)

I hope this is a compliment 😄?

If you really want all or nothing, I'm going to vote against Black.

Can we maybe still do a quick poll just to see what others think?

1. 😄 Leave everything as is (79 chars hard limit with # noqa as a last resort)
2. 🎉 Use Black
3. 🚀 Just increase our maximum line length to 92 characters

[larsoner]

+0 on 1
+1 on 2
-1 on 3

[agramfort]

+0 on 1 +1 on 2 -1 on 3 Message ID: ***@***.***>

…

[mmagnuski]

I'm not familiar with Black but I have a vague recollection that I didn't like some of the formatting rules it applies. What would be the benefit of switching to Black (apart from the fact that it seems to be a trend now)?
Currently I'm +1 on staying how it is.

[cbrnr]

@mmagnuski the benefit is that Black auto-formats everything. The issue is that Black auto-formats everything. So there is no in between if for some reason (readability) you would like to have some custom formatting.

[agramfort]

standards are always a middle ground and a compromise. If you deviate from the standard then it's useless and you are back to exceptions and it can do more harm than good. Accept the standard as it is or don't consider it.

…

Message ID: ***@***.***>

[cbrnr]

Black is not a standard, it's an opinionated formatter. Line lengths can be set even in Black (https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html#line-length), so in my opinion we could set only a new line length (to 92 or 88). I'm happy with our style, it's just the line lengths that bother me.

[mmagnuski]

@cbrnr @agramfort
I'm also happy with our style (much more than what Black does). And I'm happy with current line limit - @cbrnr when is it an issue?

[cbrnr]

when is it an issue?

Everytime. It's just not enough characters, I almost always have to break it up into multiple lines.

Most recent example: https://github.com/mne-tools/mne-python/pull/10638/files

With 92 characters, the line could have stayed one line:

ch_data = many_chunk[:, ch_offsets[ci]:ch_offsets[ci + 1]].copy()

Now it's this:

ch_data = many_chunk[:,
                     ch_offsets[ci]:ch_offsets[ci + 1]].copy()

[mmagnuski]

I'm neutral on changing the line limit - I rarely encounter such edge cases. And I'd be fine with:

ch_data = (
    many_chunk[:, ch_offsets[ci]:ch_offsets[ci + 1]].copy())

in this particular case. :)
But I understand it can be a bit frustrating at times, when you are forced to perform line-length exercises.

[sappelhoff]

I am in favor of black. I have adopted it for most of my projects and the few times that I have run into issues are heavily outweighed by the consistency, readability, and "ease of reading across projects" that black provides me with.

The issues that i have run into are furthermore typically minor: For example "I want this list to be in one line!" and then black puts it into multiple lines. I can really live with that.

I also use black together with isort to properly sort imports and with flake8 + flake8-docstrings to enforce a reasonable docstring formatting (numpy format).