FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

MNT: Remove redundant f-strings by oscargus · Pull Request #22887 · matplotlib/matplotlib · GitHub

Repository navigation

MNT: Remove redundant f-strings - #22887

Closed
oscargus wants to merge 1 commit into
matplotlib:mainfrom
oscargus:redundantformatstrings
Closed

oscargus wants to merge 1 commit into
matplotlib:mainfrom
oscargus:redundantformatstrings

Conversation

Copy link
Copy Markdown
Member

PR Summary

No need for an f-string if no formatting is done.

As I stumbled upon a line that was mentioned in #21959 I also fixed that.

Closes #21968 (except that there is no test for it here).

PR Checklist

Tests and Styling

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (install flake8-docstrings and run flake8 --docstring-convention=all).

Documentation

  • New features are documented, with examples if plot related.
  • New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).

anntzer commented Apr 24, 2022

Copy link
Copy Markdown
Contributor

FWIW I prefer keeping the leading f's when some part of the implicitly concatenated string uses formatting (and will intentionally add them), to make the string prefixes aligned...

Copy link
Copy Markdown
Member

I have mixed feelings. On one hand I agree with @anntzer that keeping the prefixes lined up in helps keep the code readable. On the other hand I have had a linter catch "I meant to make this an f-string but forget the f" on a couple of occasions.

A suggestion that I think everyone will be against, we could use an explicit u on non-f-string strings in these stacks (it is legal https://docs.python.org/3.10/reference/lexical_analysis.html#string-and-bytes-literals) aligned and the linters happy.

anntzer commented Apr 25, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

I would rather fix the linters to allow (perhaps as an option) the "there's an f just to align with another f in the implicitly concatenated string". For example, pylint seems smart enough to not emit warnings in that case (while warning for unnecessary f's if there's no interpolation anywhere).

Also, note that what you mention seems to be the opposite case (catching a missing f, not an unneeded one).

Copy link
Copy Markdown
Member

I tend to agree with @anntzer.

This is a very general styling topic. Are we aware of any recommendations/best practice of other libraries?

jklymak commented May 16, 2022

Copy link
Copy Markdown
Member

I'll close, but feel free to reopen if we think this is really needed.

jklymak closed this May 16, 2022
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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.

5 participants


Back | FazBrowse Home | New Git URL