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

Add the ability to disable specific upgrades #63

Closed
vmarkovtsev opened this issue Oct 8, 2018 · 10 comments
Closed

Add the ability to disable specific upgrades #63

vmarkovtsev opened this issue Oct 8, 2018 · 10 comments
Labels

Comments

@vmarkovtsev
Copy link
Contributor

E.g. our style guide forces printf-like syntax (we switch to C-like languages often and cut the corners) and we have to revert the corresponding upgrades as in src-d/ml#322 (review)

So would be nice to enable/disable features besides the master switch by Python version.

@asottile
Copy link
Owner

asottile commented Oct 8, 2018

I'd suggest letting go of the past. printf-style formatting has been replaced (twice now! first with .format() and next with f-strings).

pyupgrade is an opinionated tool and will not be enabling opt-out of features except by language version.

@asottile
Copy link
Owner

asottile commented Oct 8, 2018

If you're not convinced, I'd suggest reading through https://pyformat.info -- and perhaps this list of oddities

@vmarkovtsev
Copy link
Contributor Author

@asottile No need to convince me since I mentioned the reason which has nothing to do with Python at all. Let me troll you some more: we also use double quotes hehe.

No worries. I will fork the project for our org and will maintain it under a different package name.
Thanks for pyupgrade btw! ❤️

@hugovk
Copy link
Contributor

hugovk commented Oct 8, 2018

That was my PR, and the .format() change is the most common criticism in pyupgrade PRs to other projects too.

One is that .format() is slower than % formatting (eg. 367 nsec v. 243 nsec), even if that probably is premature optimisation. At least I find .format() clearer, and clearer code is usually better than ever so slightly faster code.

Another is that % formatting isn't deprecated so there's no point changing it, or that there's nothing wrong with % formatting so it's not worth the code churn.

Some just prefer the old style, especially for those switching between languages (like @vmarkovtsev).

@vmarkovtsev Please let me know when the fork is ready, it'd be useful for those cases when needing to revert and redo. Thanks!

@asottile
Copy link
Owner

asottile commented Oct 8, 2018

Without conversion there's no direct conversion to f-strings which are faster.

If you're worried about performance differences between % formatting and .format() you're using the wrong language, python is slow.

@asottile
Copy link
Owner

asottile commented Oct 8, 2018

you know, now that I see how simple the change would be to opt out of percent formatting, make a PR -- I'll accept it 😭

sorry about the runaround

@asottile
Copy link
Owner

asottile commented Oct 8, 2018

This is available in 1.8.0.

@vmarkovtsev
Copy link
Contributor Author

LOL

I knew your nerves would not bare this 😆 That's a pity - I have just passed through the CI checks and prepared the fork for pypi.

@nonchris
Copy link

Hi. It's been some time.
But I'd love to hop in and ask for the same disable feature for the "redundant open" correction.
Yeah, I know. It's redundant, we could let it go.
But as the second sentence of the zen of python says:

Explicit is better than implicit.
Just make it possible for us to state explicitly that this is just a read-operation. It makes the code better readable.

Thanks for considering my request!

@asottile
Copy link
Owner

no means no

Repository owner locked as spam and limited conversation to collaborators Oct 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants