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

Use f-strings where possible #12337

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

AA-Turner
Copy link
Member

No description provided.

@chrisjsewell
Copy link
Member

Sorry but I am very -1 against this,
I don't think you should be enforcing ad-hoc formatting rules, that touch nearly every file of the project, whilst blocking an actual formatter (#12335 (comment))

I assume you used some form of automated formatter to actually create this PR? why not add it to the formatters?

@AA-Turner
Copy link
Member Author

AA-Turner commented Apr 29, 2024

I assume you used some form of automated formatter to actually create this PR? why not add it to the formatters?

We already enable the UP031 and UP032 codes -- this work was me going through manually for the cases that those rules don't capture, for whatever reason.

The idea behind this was the (small) performance improvements that f-strings yield over percent-formatting, rather than style, but nevertheless I'm happy to close and revisit this later if you'd prefer.

A

@chrisjsewell
Copy link
Member

but nevertheless I'm happy to close and revisit this later if you'd prefer.

thanks for explaining;
I'm mean, the irony is that I am definitely in favor of this kind of change,
I just feel if we are anyway making such a "pervasive" change to the code-base, let's add the formatter.

But look, if you want to merge, I won't block, because life's too short for disagreements over these more trivial things 😄

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.

None yet

2 participants