-
Notifications
You must be signed in to change notification settings - Fork 90
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: Use Black for formatting #1144
Conversation
What's with the setuptools pinning..? |
I'm getting:
|
Our version naming / numbering doesn't conform to PEP-440, we should fix that instead of pinning setuptools to a version that accepts it :) |
Agreed -- I don't see how we are violating the rules though: https://peps.python.org/pep-0440/#developmental-releases |
the dashes maybe..? |
Maybe @larsoner can help switch versioning over to setuptools-scm |
they are inserted by the tool, our version seems fine: Line 3 in a19b20f
|
this package says our version is fine, too. |
6a4c01b
to
026cf7b
Compare
Okay I think I caught it. It was a silly mistake in parsing the version, see: Line 17 in a19b20f
so you were right that something was fishy with the dashes, thanks! |
Glad you figured it out! Still, maybe we should try switching to setuptools-scm like we did for MNE-Python and MNE-BIDS-Pipeline. Seems so much more convenient and can make the release process easier. |
I'll "rebase and merge" this one once tests pass :-) Don't squash and merge, please! |
@@ -0,0 +1 @@ | |||
58d9cf0b3a916af3e48fbb63b85b699c998c7f7a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI a rebase and merge will change the relevant commit number
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(at least it will assuming it actually does a rebase... if it's already up to date maybe it won't?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh damn, so I should create a merge commit? Thanks!
PR Description
fixes #1143
Let's see how this goes.
Merge checklist
Maintainer, please confirm the following before merging.
If applicable: