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.

Sign up for GitHub

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

Jump to bottom

MNT: Remove Python 2 leftovers #1116

Merged
merged 2 commits into from
Mar 30, 2023
Merged

MNT: Remove Python 2 leftovers #1116

merged 2 commits into from
Mar 30, 2023

Conversation

StefRe
Copy link
Contributor

@StefRe StefRe commented Mar 28, 2023

to improve readability and unify coding style (older code used %, newer contributions use format etc). Modifications were done using pyupgrade.

Files in tinybuild\examples\future were deliberately left unchanged.

I know it's a lot of changed files but I think it greatly improves readability and the Python docs also recommend the use of format over %.

@larsoner
Copy link
Contributor

and the Python docs also recommend the use of format over %.

I actually prefer f-strings nowadays. Is it simple to change to that?

@StefRe
Copy link
Contributor Author

StefRe commented Mar 28, 2023

Is it simple to change to that?

I think so, using the --py36-plus option (which will also change some other things but these changes can be reverted if they occur).

See also the note in the linked docs which sounds good:

pyupgrade is intentionally timid and will not create an f-string if it would make the expression longer or if the substitution parameters are sufficiently complicated (as this can decrease readability).

I'm going to do the f-strings in a separate commit so you can have a look.

@StefRe StefRe force-pushed the mnt/python2 branch 3 times, most recently from 1b4dba3 to 7874ace Compare March 28, 2023 14:39
@StefRe StefRe marked this pull request as ready for review March 28, 2023 14:55
StefRe added 2 commits March 30, 2023 16:36
@StefRe
MNT: Remove python 2 leftovers
74433b1 
to improve readability and unify coding style (older code used %, newer
contributions used format etc). Modifications were done using pyupgrade.

Files in tinybuild\examples\future were deliberately left unchanged.
@StefRe
MNT: Use f-strings for string formatting
e178de6
@StefRe
Copy link
Contributor Author

StefRe commented Mar 30, 2023

After rebasing I get two failing tests that pass locally so I assume it's not a real test failure. I can't "Rerun failed jobs" in https://dev.azure.com/sphinx-gallery/sphinx-gallery/_build/results?buildId=1246&view=results due to TF400813: The user '7dbe823d-48de-6791-9031-8f872678a096' is not authorized to access this resource. What should I do i such a case? Force push a dummy commit to retrigger the pipeline or is there something more intelligent?

@larsoner
Copy link
Contributor

I usually git commit --allow-empty -m 'TST: Ping' or something, yeah. (Arguably slightly cleaner than a force push.) But I do have permissions as an admin so I'll restart for you now :)

@larsoner larsoner merged commit 795e11b into sphinx-gallery:master Mar 30, 2023
@larsoner
Copy link
Contributor

Thanks @StefRe !

@StefRe StefRe deleted the mnt/python2 branch March 30, 2023 18:04
@StefRe StefRe mentioned this pull request Mar 31, 2023
Merged
@StefRe StefRe mentioned this pull request Apr 24, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
maintenance
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@StefRe @larsoner