MRG: Use ruff-format instead of Black by hoechenberger · Pull Request #12261 · mne-tools/mne-python

hoechenberger · 2023-12-04T13:15:14Z

I also removed numerous unnecessary # noqa: D102's from __init__() methods. (Unnecessary there if the class itself has a docstring)

cbrnr · 2023-12-04T13:36:30Z

Style checks are still duplicated with "Tests / Style" (GHA) and "pre-commit.ci" checks. I think we can remove the "pre-commit.ci" check, since we disabled autofix. @larsoner?

mne/viz/_3d.py

mscheltienne · 2023-12-04T14:45:52Z

What is the benefit of ruff-format compared to black? Especially as black is available in many IDEs and ruff is not?

cbrnr · 2023-12-04T14:46:55Z

@mscheltienne

mscheltienne · 2023-12-04T14:50:36Z

OK found the linked issue, speed sounds good, although I would also be inclined to give them some time to refine this new functionality.
@cbrnr thanks,I knew about that one and have it setup, not sure if it's integrated in pycharm, and definitely not in spyder.

EDIT: OK so just Spyder lagging behind, not a big issue.

hoechenberger · 2023-12-04T14:51:07Z

What is the benefit of ruff-format compared to black? Especially as black is available in many IDEs and ruff is not?

It's faster and one tool less to run via pre-commit hooks, making for a smoother dev experience if these hooks are installed

hoechenberger · 2023-12-04T15:36:27Z

EDIT: OK so just Spyder lagging behind, not a big issue.

Spyder can use it via python-lsp-ruff in https://github.com/python-lsp/python-lsp-server

larsoner · 2023-12-04T16:33:25Z

Style checks are still duplicated with "Tests / Style" (GHA) and "pre-commit.ci" checks. I think we can remove the "pre-commit.ci" check, since we disabled autofix. @larsoner?

I'd rather move more stuff to pre-commit.ci when possible actually. It tends to be faster than GHA and distributes load better to have it there. I think it makes sense (and we have been moving toward) to have pre-commit + pre-commit.ci take care of as much as possible, and leave the checks that are too slow for pre-commit (like twine check) to the remaining GHA style check

larsoner

+1 for merge from me, with or without looking into the " + ".*" stuff. Overall I think this improves things and we can manually fix the " + ".*" when we come across them (or wait for the tool to get better and fix these for us automatically!).

larsoner · 2023-12-04T16:41:42Z

mne/beamformer/tests/test_rap_music.py

-        1
-        / (sigma * np.sqrt(2 * np.pi))
-        * np.exp(-((times - mu) ** 2) / (2 * sigma**2))
+        1 / (sigma * np.sqrt(2 * np.pi)) * np.exp(-((times - mu) ** 2) / (2 * sigma**2))


I was wondering why we had a ton of lines removed... I'm guessing it's mostly from this. I consider this a nice readability win

hoechenberger · 2023-12-04T17:21:32Z

@larsoner Currently, some GHA jobs like the one that runs pytest depend on the "style" job, which runs the pre-commit hooks. This is to avoid running pytest as long as there are still code style issues.

Can we make our GHA runs conditional on pre-commit.ci return codes?

larsoner · 2023-12-04T17:32:10Z

Can we make our GHA runs conditional on pre-commit.ci return codes?

Possibly but someone would have to look into it. FYI Azure also has a style run that other runs depend on IIRC

hoechenberger · 2023-12-04T17:39:35Z

But that would mean we run the pre-commit hooks twice (pre-commit.ci and GHA)? Unless we remove the job from GHA, businesses then the others cannot be (easily) conditional

What do you suggest we do?

larsoner · 2023-12-04T17:48:01Z

I think the way things work right now are okay. There's always room for improvement but I think we have more important stuff to work on (diminishing returns tweaking CIs further)

mne/channels/channels.py

mne/viz/topomap.py

mne/viz/utils.py

tutorials/epochs/20_visualize_epochs.py

tutorials/inverse/20_dipole_fit.py

tutorials/raw/10_raw_overview.py

mne/forward/tests/test_field_interpolation.py

hoechenberger · 2023-12-05T08:53:14Z

tutorials/stats-source-space/60_cluster_rmANOVA_spatiotemporal.py

    (clusters[cluster_ind]) for ii, cluster_ind in enumerate(good_cluster_inds)
-][
-    0
-]  # first cluster


@drammock here's an example where things are reversed and are looking better with ruff

Co-authored-by: Daniel McCloy <dan@mccloy.info>

…uff-format

hoechenberger · 2023-12-05T09:16:49Z

I think this is good to go.

hoechenberger · 2023-12-05T10:30:22Z

@mscheltienne Okay with you if we move forward here?

mscheltienne · 2023-12-05T10:32:34Z

Sure!

larsoner · 2023-12-05T16:47:39Z

Okay let's give it a shot!

I'll add e7dd158 to .git-blame-revs-ignore in some follow-up PR

* upstream/main: MRG: Use ruff-format instead of Black (mne-tools#12261) MAINT: Make selenium optional and use on CircleCI (mne-tools#12263) MAINT: Post-release deprecations (mne-tools#12265) MAINT: Replace `Path.parent.parent` with `Path.parents[N]` in tests (mne-tools#12257)

Co-authored-by: Eric Larson <larson.eric.d@gmail.com> Co-authored-by: Daniel McCloy <dan@mccloy.info>

Use ruff-format instead of Black

a06d8c1

hoechenberger mentioned this pull request Dec 4, 2023

We may be able to ditch Black in favor of just Ruff #12255

Closed

Remove unncesessary #noqa: D102 from __init__() methods

07e932e

hoechenberger marked this pull request as ready for review December 4, 2023 13:28

hoechenberger requested review from adam2392, agramfort, alexrockhill, britta-wstnr, cbrnr, dengemann, drammock, jasmainak, larsoner and mscheltienne as code owners December 4, 2023 13:28

hoechenberger added 2 commits December 4, 2023 14:32

Update GH Action config

682bd43

Better naming

26ac122

cbrnr reviewed Dec 4, 2023

View reviewed changes

mne/viz/_3d.py Outdated Show resolved Hide resolved

larsoner reviewed Dec 4, 2023

View reviewed changes

drammock reviewed Dec 4, 2023

View reviewed changes

Merge branch 'main' into ruff-format

2dbd0f8

hoechenberger commented Dec 5, 2023

View reviewed changes

mne/forward/tests/test_field_interpolation.py Outdated Show resolved Hide resolved

hoechenberger commented Dec 5, 2023

View reviewed changes

mne/forward/tests/test_field_interpolation.py Outdated Show resolved Hide resolved

hoechenberger commented Dec 5, 2023

View reviewed changes

hoechenberger and others added 11 commits December 5, 2023 09:54

Apply suggestions from code review

3027dc3

Co-authored-by: Daniel McCloy <dan@mccloy.info>

Apply suggestions from code review

f647a0e

Co-authored-by: Daniel McCloy <dan@mccloy.info>

Apply suggestions from code review

ab07566

Co-authored-by: Daniel McCloy <dan@mccloy.info>

Apply suggestions from code review

8164953

Co-authored-by: Daniel McCloy <dan@mccloy.info>

Apply suggestions from code review

834ec27

Co-authored-by: Daniel McCloy <dan@mccloy.info>

Apply suggestions from code review

785d2a2

Co-authored-by: Daniel McCloy <dan@mccloy.info>

Apply suggestions from code review

7c46976

Co-authored-by: Daniel McCloy <dan@mccloy.info>

Run ruff format again

1574b26

Fix dipole tutorial

0930e45

Merge branch 'main' of https://github.com/mne-tools/mne-python into r…

3bdecee

…uff-format

Better docstring as suggested by drammock

9ec2180

hoechenberger changed the title ~~Demonstration: Use ruff-format instead of Black~~ MRG: Use ruff-format instead of Black Dec 5, 2023

Fix

41efdc8

hoechenberger added 3 commits December 5, 2023 11:38

Better names for pre-commit hooks

d7999bc

cleanup

964395b

Fix test

8a9956f

larsoner merged commit e7dd158 into mne-tools:main Dec 5, 2023

hoechenberger deleted the ruff-format branch December 5, 2023 17:30

mscheltienne mentioned this pull request Dec 20, 2023

Drop black in favor of ruff and generate stubs to expand docstrings in Pylance and IDEs vferat/pycrostates#137

Merged

snwnde pushed a commit to snwnde/mne-python that referenced this pull request Mar 20, 2024

MRG: Use ruff-format instead of Black (mne-tools#12261)

7906f20

Co-authored-by: Eric Larson <larson.eric.d@gmail.com> Co-authored-by: Daniel McCloy <dan@mccloy.info>

bruAristimunha mentioned this pull request Apr 4, 2024

[MNT] Black codebase braindecode/braindecode#584

Merged

Uh oh!

Conversation

hoechenberger commented Dec 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cbrnr commented Dec 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

mscheltienne commented Dec 4, 2023

Uh oh!

cbrnr commented Dec 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mscheltienne commented Dec 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hoechenberger commented Dec 4, 2023

Uh oh!

hoechenberger commented Dec 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

larsoner commented Dec 4, 2023

Uh oh!

larsoner left a comment

Choose a reason for hiding this comment

Uh oh!

larsoner Dec 4, 2023

Choose a reason for hiding this comment

Uh oh!

hoechenberger Dec 5, 2023

Choose a reason for hiding this comment

Uh oh!

hoechenberger commented Dec 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

larsoner commented Dec 4, 2023

Uh oh!

hoechenberger commented Dec 4, 2023

Uh oh!

larsoner commented Dec 4, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hoechenberger Dec 5, 2023

Choose a reason for hiding this comment

Uh oh!

hoechenberger commented Dec 5, 2023

Uh oh!

hoechenberger commented Dec 5, 2023

Uh oh!

mscheltienne commented Dec 5, 2023

Uh oh!

larsoner commented Dec 5, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

hoechenberger commented Dec 4, 2023 •

edited

Loading

cbrnr commented Dec 4, 2023 •

edited

Loading

cbrnr commented Dec 4, 2023 •

edited

Loading

mscheltienne commented Dec 4, 2023 •

edited

Loading

hoechenberger commented Dec 4, 2023 •

edited

Loading

hoechenberger commented Dec 4, 2023 •

edited

Loading