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. #911

Merged
merged 5 commits into from
Sep 30, 2020
Merged

Use f-strings where possible. #911

merged 5 commits into from
Sep 30, 2020

Conversation

JevinJ
Copy link
Member

@JevinJ JevinJ commented Sep 30, 2020

Overview

With #905, f-strings can be used.

Converts .format and % syntax to f-strings where possible, including docs.

Some of the conversion was automated so have some caution reviewing.

@codecov
Copy link

codecov bot commented Sep 30, 2020

Codecov Report

Merging #911 into master will increase coverage by 0.00%.
The diff coverage is 63.77%.

@@           Coverage Diff           @@
##           master     #911   +/-   ##
=======================================
  Coverage   86.88%   86.89%           
=======================================
  Files          36       36           
  Lines        8985     8984    -1     
=======================================
  Hits         7807     7807           
+ Misses       1178     1177    -1     

Copy link
Member

@banesullivan banesullivan left a comment

Choose a reason for hiding this comment

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

LGTM - I didn't look over every change but I assume the tests should catch any issues arising from this.

Thanks for doing this, @JevinJ!

Copy link
Member

@akaszynski akaszynski left a comment

Choose a reason for hiding this comment

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

Agree with the change. We had a mix of old (%) and newer format methods and it's a good refactor to just use f-strings now that they're supported.

@akaszynski
Copy link
Member

I can merge this since it looks like the Python3.5 checks won't be reported since we're not building those. Not sure why it's still requiring those...

Let me know when you're good for me to merge this.

@banesullivan
Copy link
Member

I just turned off the 3.5 required checks in the branch protection rules

@akaszynski
Copy link
Member

I just turned off the 3.5 required checks in the branch protection rules

Thanks, totally forgot about that. I'll do the same for pyvistaqt if you've not done so.

@JevinJ
Copy link
Member Author

JevinJ commented Sep 30, 2020

@akaszynski Good to go.

@akaszynski akaszynski merged commit c7d7b58 into master Sep 30, 2020
@JevinJ JevinJ deleted the refactor/f-strings branch October 2, 2020 09:52
@akaszynski akaszynski mentioned this pull request Nov 8, 2020
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

3 participants