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

double-string-fixer will introduce syntax errors for nested f-strings for Python pre-3.12 versions when pre-commit is run by Python 3.12 #971

Closed
XuehaiPan opened this issue Oct 5, 2023 · 3 comments · Fixed by #973

Comments

@XuehaiPan
Copy link

Thanks for the great tool! It is really convenient to maintain a consistent code style among contributors. I use the double-string-fixer to keep Python strings in a single-quoted format cooperating with black, which prefers double-quoted strings.

Python 3.12 was just released several days ago. It allows users to write arbitrary nested f-strings without changing the quote styles (PEP 701 – Syntactic formalization of f-strings).

# Python 3.11
f'-DPYTHON_INCLUDE_DIR={sysconfig.get_path("platinclude")}'

# Python 3.12
f'-DPYTHON_INCLUDE_DIR={sysconfig.get_path('platinclude')}'  # syntax error in Python 3.11

The pre-commit-hooks are invoked by the Python interpreter which runs pre-commit. When the pre-commit command is installed with Python 3.12, it enables PEP 701 and will introduce syntax errors for Python pre-3.12 versions.

- f'-DPYTHON_INCLUDE_DIR={sysconfig.get_path("platinclude")}'
+ f'-DPYTHON_INCLUDE_DIR={sysconfig.get_path('platinclude')}'

This maybe a problem for a project that supports multiple Python versions.

@asottile
Copy link
Member

asottile commented Oct 5, 2023

yeah for now I would recommend running it in the oldest python version you support

@XuehaiPan
Copy link
Author

Thanks for the advice. I have pinned the Python version in our CI.

I'm also looking forward to the double-string-fixer can work as expected for a better local development experience. Maybe we can add an extra argument, e.g., --target-version=py311.

@asottile
Copy link
Member

asottile commented Oct 5, 2023

personally I think the nested fstrings stuff is bad so I'll be making it just ignore the stuff inside there (like it always did)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants