Skip to content

MNT: Remove Python 2 leftovers#1116

Merged
larsoner merged 2 commits intosphinx-gallery:masterfrom
StefRe:mnt/python2
Mar 30, 2023
Merged

MNT: Remove Python 2 leftovers#1116
larsoner merged 2 commits intosphinx-gallery:masterfrom
StefRe:mnt/python2

Conversation

@StefRe
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
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
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor

Thanks @StefRe !

@StefRe StefRe deleted the mnt/python2 branch March 30, 2023 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants