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

Should Black change .format() and % into f-strings in 3.6+ mode? #363

Closed
tweakimp opened this issue Jun 18, 2018 · 12 comments
Closed

Should Black change .format() and % into f-strings in 3.6+ mode? #363

tweakimp opened this issue Jun 18, 2018 · 12 comments
Labels
T: style What do we want Blackened code to look like? T: user support OP looking for assistance or answers to a question

Comments

@tweakimp
Copy link

For Python3.6+ formatting, I would really like to see black convert all formatted strings to f-strings if possible.
F-strings are shorter, easier to read and, in my opinion, more pythonic.

# Originals
"filename" + date + ".pdf" 
"filename%s.pdf" % date
"filename{0}.pdf".format(date)

# Suggested formatted output
f"filename{date}.pdf"  
@anthrotype
Copy link

Check out https://github.com/asottile/pyupgrade

@ambv ambv added the T: style What do we want Blackened code to look like? label Jun 19, 2018
@ambv
Copy link
Collaborator

ambv commented Jun 19, 2018

This is a nice idea but I feel it's out of scope for Black. Black is not a linter, it's a formatter. It starts and ends with the same AST. I'll think about it, in the mean time Anthony's pyupgrade seems like a good tool that feels this need.

@ambv ambv changed the title [Feature request] Format to f-strings Should Black change .format() and % into f-strings in 3.6+ mode? Jun 19, 2018
@Mariatta
Copy link
Contributor

I'm all for more f-strings, but I also think that it is out of the scope for black to do the conversion.

@ambv ambv added the T: user support OP looking for assistance or answers to a question label Jun 20, 2018
@yutkin
Copy link

yutkin commented Jun 21, 2018

Such converting can be harmful. For example, this code works well:

class Base:
	formatted_str = "Hello! I'm {name}"
	def say_hello(self):
		return self.formatted_str.format(name=self.first_name)

class SubClass1(Base):
	first_name = 'SubClass1'

class SubClass2(Base):
	first_name = 'SubClass2'

subclass1 = SubClass1()
subclass2 = SubClass2()

print(subclass1.say_hello())
print(subclass2.say_hello())

But it's very difficult to replace .format in a base class.

@JelleZijlstra
Copy link
Collaborator

Black would (if it ever did this) only do it for calls to <string literal>.format(), so I don't think your example would be affected.

@GuillemGSubies
Copy link

I was going to open an issue for this, but I saw that it already existed. In my opinion f-strings are also about format. A f-string is much easier to read and understand that the old strings so it would be awesome if Black could make this changes automatically.

@thomasf
Copy link

thomasf commented May 12, 2019

Please don't make black too smart by auto upgrading code. I'm just looking through issues to get an idea of where black is heading and plan to start using black on projects when it has a stable release. Suggestions like this makes me feel a bit uneasy. I believe that it's good if all AST modifying features were turned off by default and enabled via 'pyproject.toml` or not added at all.

@chris-rands
Copy link

While I agree that black is probably not the right tool for conversion to f-strings, I notice that black does appear to (virtually?) only use f-strings in this repo (although I can't see any relevant pre-commit stuff enforcing this). Does black only use f-strings for it's source code as an internal policy? If yes, does it achieve this via pyupgrade or other tools like flynt?

@zsol
Copy link
Collaborator

zsol commented Jan 13, 2020

Does black only use f-strings for it's source code as an internal policy? If yes, does it achieve this via pyupgrade or other tools like flynt?

Via code review :)

@tweakimp
Copy link
Author

I think we can see this as rejected for understandable reasons.

@ericvsmith
Copy link

I know this issue is closed, but I'll comment on it in case someone ever finds it while doing research.

First off, I agree this is out of scope for black, not that my opinion matters. And as @JelleZijlstra rightly points out, you could only do this for .format() calls on string literals.

Second, converting "".format() calls to f-strings is very, very difficult for a number of reasons. I'll just touch on one of them: parameters appearing multiple times and functions with side effects.

For example, consider: "{0} {0}".format(func()). The naive translation is f"{func()} {func()}". But if func() has side effects, these two do not do the same thing. You could add temporary variables, etc., but at this point it's hardly an improvement.

Okay, I'll touch on another problem: subscripting is treated differently:

>>> d = {'a': 0, 1:2, 8:3}
>>> a=1
>>> '{d[a]}'.format(d=d)
'0'
>>> f'{d[a]}'
'2'
>>> '{d[08]}'.format(d=d)
'3'
>>> f'{d[08]}'
  File "<fstring>", line 1
    (d[08])
        ^
SyntaxError: invalid token

There are other nuances to the subscript problem as well, such as whether a subscript "looks like" a number, and just what "looks like" means.

@pradyunsg
Copy link
Contributor

@ambv You might wanna remove this from the corresponding project board. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T: style What do we want Blackened code to look like? T: user support OP looking for assistance or answers to a question
Projects
None yet
Development

No branches or pull requests