Skip to content

refactor: prefer f-strings to other format/concatentation styles #8474

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

Merged
merged 1 commit into from
Aug 3, 2022
Merged

refactor: prefer f-strings to other format/concatentation styles #8474

merged 1 commit into from
Aug 3, 2022

Conversation

kkirsche
Copy link
Contributor

@kkirsche kkirsche commented Aug 2, 2022

Use f-strings for concatenating strings instead of '+' or .format

Python added f-strings in version 3.6, with PEP 498. F-strings are a flexible and powerful way to concatenate strings. They make the code shorter and more readable, since the code now looks more like the output. In addition, they also come with performance benefits as they reduce the number of times the string must be processed, resulting in faster string processing.

This aligns with the desire to ensure that scripts and tests should support currently supported versions of Python outlined here

Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you:

  1. Checked for %? Just to be sure
  2. Thought of any CI check to prevent this from happening again?

🙂

@kkirsche
Copy link
Contributor Author

kkirsche commented Aug 3, 2022

Have you:

  1. Checked for %? Just to be sure
  2. Thought of any CI check to prevent this from happening again?

🙂

  1. Yes, this included checks for % style formatting in addition to +=, +, .format, etc.
  2. This was something I was wondering. I detected this refactoring using the free version of Sourcery which does support open-source projects, but I'm not sure that the extra dependency, configuration, etc. is worth the overhead. This page provides a list of some of the automated refactorings it can do. This merge request shows what was automatically detected (across all python versions), which you'll notice recommends against some of what I expect were intentional code style decisions.

I've been toying around with breaking out this functionality and parts of flake8-pyi to be more generic so that I can more easily detect deprecated typing imports for target python version, f-strings if available, contextlib if available, etc. across open and closed source projects, but I'm concerned that it'll simply lead to more plugin fragmentation than a way to solve the core problem.

If you have a suggestion or approach you'd like to see, I'm happy to work on it

@srittau srittau merged commit bd7a02f into python:master Aug 3, 2022
@kkirsche kkirsche deleted the refactor/f-strings branch August 3, 2022 12:18
@AlexWaygood
Copy link
Member

2. Thought of any CI check to prevent this from happening again?

I don't think we really need a CI check for this. It's fine to do cleanups every now and then, but we don't have very many .py files in typeshed, and none of them are performance critical. Adding another check to our CI for this would hurt more than it would help, in my opinion (by slowing down our CI).

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.

4 participants