Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MAINT: Black, ruff, pre-commit #688

Closed
wants to merge 3 commits into from
Closed

Conversation

larsoner
Copy link
Member

Closes #686

  • Format with black
  • Add reformatting commit to ignore-revs
  • Change from flake8 to ruff
  • Add to CIs
  • Add to pre-commit config

Should be merged rather than squash-and-merged so that the black reformatting commit is ignored properly.

@larsoner larsoner force-pushed the sty branch 2 times, most recently from 0aca705 to 333d921 Compare December 13, 2022 18:16
@larsoner
Copy link
Member Author

I'm going to assume this is some weird GitHub problem and try again tomorrow...

@larsoner
Copy link
Member Author

Ahh, GH is degraded at the moment:

Investigating - We are investigating reports of degraded performance for Git Operations and Pull Requests.
Dec 13, 2022 - 17:39 UTC

@@ -452,7 +452,7 @@
```
"""

min_break_duration: float = 15.
min_break_duration: float = 15.0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hah, nice!!!

ArbitraryContrast
]
] = []
contrasts: Iterable[Union[Tuple[str, str], ArbitraryContrast]] = []
Copy link
Member

@hoechenberger hoechenberger Dec 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So difficult to read. Time to drop support for Python <3.10 so we can use X | Y instead of Union[X, Y] 😎

No but really, here we'd really gain a usability boost from a switch... maybe next spring or so??

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My vote is to go with whenever MNE-Python switches to 3.10 as the min based on EOL

@larsoner
Copy link
Member Author

I temporarily enabled merge commits for the repo so we can merge this one rather than rebase+merge or squash+merge. I also removed the make flake alias. Okay to "merge commit" when green?

@drammock
Copy link
Member

why not split out the black commit as a separate PR? and/or rebase to squash all the other commits in this PR into one, and then rebase-merge?

@hoechenberger
Copy link
Member

I temporarily enabled merge commits for the repo so we can merge this one rather than rebase+merge or squash+merge. I also removed the make flake alias. Okay to "merge commit" when green?

Okay with me

@hoechenberger
Copy link
Member

why not split out the black commit as a separate PR? and/or rebase to squash all the other commits in this PR into one, and then rebase-merge?

This might actually be the better idea 🤓

@larsoner
Copy link
Member Author

why not split out the black commit as a separate PR? and/or rebase to squash all the other commits in this PR into one, and then rebase-merge?

It's certainly another way to do it, but more work at this point. But since @hoechenberger is also for it, I'll do it that way

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Try out ruff?
3 participants