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

.format() in (legacy) code could be converted to f-strings. #5474

Closed
mkcor opened this issue Jul 15, 2021 · 15 comments
Closed

.format() in (legacy) code could be converted to f-strings. #5474

mkcor opened this issue Jul 15, 2021 · 15 comments

Comments

@mkcor
Copy link
Member

mkcor commented Jul 15, 2021

Description

To address this issue, a new contributor could start with a recursive grep for \.format\( on a subpackage of interest to them (or on the entire package). Replacing instances of .format() with f-strings can be a good pretext to explore the codebase.

Reference: https://www.python.org/dev/peps/pep-0498/

@alex-jw-brooks
Copy link
Contributor

Hello! May I work on this issue?

@that1solodev
Copy link
Contributor

I'd like to work on this

@mkcor
Copy link
Member Author

mkcor commented Jul 15, 2021

For sure! @alex-jw-brooks could do it for the filters subpackage and @Xyno18 for the segmentation subpackage: How does that sound?

@alex-jw-brooks
Copy link
Contributor

That sounds great, thanks!

@that1solodev
Copy link
Contributor

Sounds great. I'm on it

@Larkinnjm1
Copy link
Contributor

Hey folks looking to contribute to a first issue on this repo can I look to work on this issue for other sub modules such as restoration and registration?

@mkcor
Copy link
Member Author

mkcor commented Aug 9, 2021

Hi @Larkinnjm1,

can I look to work on this issue for other sub modules such as restoration and registration?

Sure, please go for it!

@MariosAchilias
Copy link
Contributor

Hello, i'd also like to contribute, can I work on this for the color, exposure, transform and util modules?

@mkcor
Copy link
Member Author

mkcor commented Aug 9, 2021

Hi @MariosAchilias,

Hello, i'd also like to contribute, can I work on this for the color, exposure, transform and util modules?

Ok!

@Larkinnjm1
Copy link
Contributor

Hi @Larkinnjm1,

can I look to work on this issue for other sub modules such as restoration and registration?

Sure, please go for it!

Cool great on it!

@jstriebel
Copy link
Contributor

Just a recommendation: pyupgrade is a tool which can automatically apply new python-features on code, such as converting "".format() calls to f-strings:
https://github.com/asottile/pyupgrade#f-strings

It might also do too many other conversions you don't want in the code-base, not sure about that, just a suggestion :-)

Larkinnjm1 pushed a commit to Larkinnjm1/scikit-image that referenced this issue Aug 12, 2021
".format()" of strings objects modified to f-strings within registration and restoration packages of library per issue scikit-image#5474.
Committer: Niall Larkin <nialllarkin@nialls-mbp.home>
On branch registration_restoration_fstrings
Changes to be committed:
	modified:   skimage/registration/_masked_phase_cross_correlation.py
	modified:   skimage/registration/tests/test_masked_phase_cross_correlation.py
	modified:   skimage/restoration/_denoise.py
@mkcor
Copy link
Member Author

mkcor commented Aug 16, 2021

Hi @jstriebel,

It might also do too many other conversions you don't want in the code-base, not sure about that, just a suggestion :-)

Exactly! But, it might be worth considering each pyupgrade item, to decide whether to include it or not. Thanks for the suggestion!

hxri added a commit to hxri/scikit-image that referenced this issue Aug 25, 2021
This is part of the issue scikit-image#5474

The .format() has been replaced with f String wherever possible.
@ovynnej
Copy link
Contributor

ovynnej commented Oct 1, 2021

Hello @mkcor! I'm not super experienced, so I just wanted to check -- should this issue be closed? Running grep -r ".format(" . on the package returns only these instances, which are not relevant to the issue:

./_shared/utils.py: warnings.warn(self.warning_msg.format(
./_shared/utils.py: warnings.warn(warning_msg.format(func_name=func.name),
./_shared/utils.py: warnings.warn(self.warning_msg.format(

Thank you!

@grlee77
Copy link
Contributor

grlee77 commented Oct 1, 2021

Thanks for the reminder, @ovynnej. Indeed, we had several contributors in July/August and I think all of these have been addressed.

@grlee77 grlee77 closed this as completed Oct 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants