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 for formatting strings #743

Merged
merged 32 commits into from
Sep 27, 2021
Merged

Conversation

umarcor
Copy link
Member

@umarcor umarcor commented Sep 22, 2021

Currently, we are using a deprecated syntax ("some text %s" % variable) for formatting strings. CI linting is complaining about it.

There are three alternatives:

  • "some text %s".format(variable)"
  • f"some text {variable}"
  • "some text" + str(variable)

It seems that the official syntax going forward is the second one only (f-strings). I tried using the first one, but linters would still complain. With regard to the last one, that is not desirable I think, however, in some cases I found it necessary because I couldn't make it work using f-strings.

In this PR, most of the sources in the codebase are modified for using f-strings instead of "some text %s" % variable or "some text %s".format(variable)". The remaining sources belong to vunit/sim_if. I "fixed" those already, but I cannot test all of them. Hence, I decided to include the ghdl.py only, and handle those in an upcoming PR.

I followed the following guidelines:

  • Replace %s with {VARIABLE!s}.
  • Replace %r with {VARIABLE!r}.
  • Replace %i with {VARIABLE:d}.
  • Replace %.1f with {VARIABLE:.1f}.
  • Replace "%s\n" % " ".join(cmd) with " ".join(cmd) + "\n", and "%s\n" % ("=" * (max(max_len + 25, 0))) with ("=" * (max(max_len + 25, 0))) + "\n".

@eine eine requested a review from LarsAsplund September 22, 2021 13:07
@Paebbels
Copy link

What linter are you using?

@umarcor
Copy link
Member Author

umarcor commented Sep 22, 2021

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