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 pyupgrade pre-commit hook #939

Closed
wants to merge 2 commits into from

Conversation

yagebu
Copy link
Contributor

@yagebu yagebu commented Sep 18, 2021

This hook automatically updates Python files with modern syntax, e.g., f-strings and sets.

@dairiki
Copy link
Contributor

dairiki commented Sep 18, 2021

As you might expect, I'm not a huge fan of this. It touches a lot of code for arguable benefit.

Mostly, though, I would really like to see a working release make it out the door before we make too many more changes.

@yagebu
Copy link
Contributor Author

yagebu commented Sep 18, 2021

Mostly, though, I would really like to see a working release make it out the door before we make too many more changes.

I already released 3.2.2 with the most important bugfixes

rev: v2.25.0
hooks:
- id: pyupgrade
args: ["--py36-plus"]
Copy link
Contributor

@dairiki dairiki Sep 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be --py37-plus (though, in practice, that doesn't seem to change anything) since we no longer support python 3.6.

@@ -87,7 +87,7 @@ def resolve_url_path(self, url_path):
return None

def __repr__(self):
return "<%s %r>" % (
return "<{} {!r}>".format(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a number of these cases where %-formatted strings are converted to .format() formatted strings rather than f-strings.

The pyupgrade README does say:

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 sufficiently complicated (as this can decrease readability).

That said, a lot of these cases seem like they are short enough that they would, in fact, be cleaner as f-strings. Should we manually convert them?

@yagebu
Copy link
Contributor Author

yagebu commented Apr 15, 2023

I'm closing this one. I still think it makes sense to have these lint rules but I think it makes more sense to use the newer, faster (not that pyupgrade wasn't fast enough already) and more powerful ruff.

@yagebu yagebu closed this Apr 15, 2023
@yagebu yagebu deleted the pre-commit-pyupgrade branch April 15, 2023 11:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants