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

pyupgrade is aggressively converting everything to f-strings #526

Closed
Dreamsorcerer opened this issue Sep 7, 2021 · 6 comments
Closed

pyupgrade is aggressively converting everything to f-strings #526

Dreamsorcerer opened this issue Sep 7, 2021 · 6 comments

Comments

@Dreamsorcerer
Copy link

We've just got a pre-commit auto-update, which has bumped pyupgrade to the latest release.

It seems that pyupgrade is now aggressively reformatting anything it can to an f-string, which is not what the documentation says it should do.

note: pyupgrade is intentionally timid and will not create an f-string if it would make the expression longer or if the substitution parameters are anything but simple names or dotted names (as this can decrease readability).

As one example, where it's pushed practically an entire line of code into a string:
https://github.com/aio-libs/aiohttp/pull/5985/files#diff-8d8821e084bea8fa7388131b9aa226100488108387b49090d7982e0fb96fa6ffR77

I'd also appreciate on option to disable f-string conversion entirely, as it's not a universally accepted best-practice (for example, it's disallowed in wemake-style-guide).

@asottile
Copy link
Owner

asottile commented Sep 7, 2021

@asottile asottile closed this as completed Sep 7, 2021
@Dreamsorcerer
Copy link
Author

Firstly, let me repeat the documentation:

note: pyupgrade is intentionally timid and will not create an f-string if it would make the expression longer or if the substitution parameters are anything but simple names or dotted names (as this can decrease readability).

Secondly, the option is declaring that we only need to support python 3.6+, not that we specifically want to use f-strings everywhere.

I realise that all your projects are very opinionated and you probably won't add an option to disable the feature, but I do expect you to honour the documentation which says that it avoids trying to decrease readability, which it is clearly not doing.

@Dreamsorcerer
Copy link
Author

So, unless fm_size(sum(j) if j else 0) looks like a simple name or dotted name to you, something is clearly wrong.

@asottile
Copy link
Owner

asottile commented Sep 7, 2021

ah right, things changed in #298 -- I've clarified in 6b2d0d8

@Dreamsorcerer
Copy link
Author

OK, I think that example and a couple of others are too complex for my liking.

Again, it would be nice to have an option to opt-out of f-strings, without having to opt out of all other (non-controversial) 3.6+ features.

@asottile
Copy link
Owner

asottile commented Sep 7, 2021

options in pyupgrade are only for breakages

you're also free to assign local variables to simplify your expressions (though I'd argue the .format(...) code was the same complexity and probably should have been refactored anyway)

Repository owner locked as resolved and limited conversation to collaborators Sep 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants