Skip to content

Conversation

@cbrnr
Copy link
Contributor

@cbrnr cbrnr commented Jan 13, 2024

This PR enables Ruff UP rules (pyupgrade). These rules check if newer language constructs can be used (it automatically uses the minimum required version from pyproject.toml, so 3.9 in this case). I've only fixed some of the errors, the remaining ones need a bit more manual work. Thanks @hoechenberger for pointing out this very nice tool!

Edit: tutorials and examples are done.

@hoechenberger
Copy link
Member

hoechenberger commented Jan 14, 2024

Very nice, I generally like the idea of making use of modern language features.

I only hope we don't accidentally introduce new bugs.....

@cbrnr
Copy link
Contributor Author

cbrnr commented Jan 14, 2024

I only hope we don't accidentally introduce new bugs.....

If this happens, our tests need to be udpated.

@cbrnr
Copy link
Contributor Author

cbrnr commented Jan 14, 2024

And at least one person needs to go over all changes I'm afraid.

@cbrnr
Copy link
Contributor Author

cbrnr commented Jan 14, 2024

Turns out this is a huge PITA. If anyone wants to take over the remaining ~350 conversions from % to f-string, please knock yourselves out. Otherwise, I'll do it sometime soonish, but probably not in the next days.

@hoechenberger
Copy link
Member

I don't think we "need" to do these ... I mean it's great you're donating so much of your time, but I'm not sure if those strings are worth the effort!

@drammock
Copy link
Member

Turns out this is a huge PITA. If anyone wants to take over the remaining ~350 conversions from % to f-string, please knock yourselves out. Otherwise, I'll do it sometime soonish, but probably not in the next days.

Thanks for getting the ball rolling; may I suggest disabling UP031 and merging this sooner, and opening an issue (maybe tagged good-first-issue?) to re-enable it and fix the %s later?

@cbrnr
Copy link
Contributor Author

cbrnr commented Jan 14, 2024

Yes, sure. Can we keep the changes I've already made?

@drammock
Copy link
Member

Yes, sure. Can we keep the changes I've already made?

of course!

@hoechenberger
Copy link
Member

Great idea, @drammock!

@cbrnr cbrnr marked this pull request as ready for review January 15, 2024 06:30
@cbrnr cbrnr requested a review from britta-wstnr as a code owner January 15, 2024 06:30
@cbrnr
Copy link
Contributor Author

cbrnr commented Jan 15, 2024

This should be good to go. I've created #12360 to follow up; we don't have a "good first issue" label, so I've used "EASY" for now (but feel free to change it).

@cbrnr
Copy link
Contributor Author

cbrnr commented Jan 15, 2024

We get some Numpy 2.0 related errors in Pyvista/Pyvistaqt. It seems like there's no issue in their repos yet, since they probably do not test against pre-release Numpy.

@drammock
Copy link
Member

drammock commented Jan 15, 2024

We get some Numpy 2.0 related errors in Pyvista/Pyvistaqt. It seems like there's no issue in their repos yet, since they probably do not test against pre-release Numpy.

I've just opened pyvista/pyvista#5472, and pyvista/pyvista#5473 to fix it.

@cbrnr
Copy link
Contributor Author

cbrnr commented Jan 15, 2024

Thanks! Let me know once the PR is merged, I can then re-run the pip-pre job.

@drammock drammock enabled auto-merge (squash) January 15, 2024 18:11
@drammock drammock merged commit 2040898 into mne-tools:main Jan 15, 2024
@cbrnr cbrnr deleted the add-pyupgrade branch January 16, 2024 06:36
snwnde pushed a commit to snwnde/mne-python that referenced this pull request Mar 20, 2024
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
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.

3 participants