asottile / pyupgrade

A tool (and pre-commit hook) to automatically upgrade syntax for newer versions of the language.
MIT License
3.59k stars 183 forks source link

pyupgrade on f-strings with .format() results in invalid f-string #329

Closed chriselion closed 4 years ago

chriselion commented 4 years ago

I understand that this is an edge-case because the input was buggy, but it does convert running code into non-running code.

$ cat a.py
print(f"foo {0}".format("bar"))
$ python a.py
foo 0
$ pyupgrade a.py
Rewriting a.py
$ cat a.py
print(f"foo {}".format("bar"))
$ python a.py
  File "a.py", line 1
    print(f"foo {}".format("bar"))
         ^
SyntaxError: f-string: empty expression not allowed
$ pip freeze | grep pyupgrade
pyupgrade==2.7.0

I'm not sure what the right behavior is in this case; maybe just don't touch numbered args inside an f-string?

asottile commented 4 years ago

interesting! yeah the number-fixing code should leave fstrings alone (it was written before they existed!)

chriselion commented 4 years ago

Here's the actual code that broke from this:

https://github.com/Unity-Technologies/ml-agents/blob/4261b4bf15e9d394b5d2426bc238d92940ab7e7d/ml-agents-envs/mlagents_envs/environment.py#L359-L364

(now being rewritten to use only f-strings)

asottile commented 4 years ago

thanks for the report! this should be fixed in v2.7.1