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

Q000 error on f-string for CPython 3.12 #117

Closed
skirpichev opened this issue Sep 7, 2023 · 11 comments
Closed

Q000 error on f-string for CPython 3.12 #117

skirpichev opened this issue Sep 7, 2023 · 11 comments

Comments

@skirpichev
Copy link
Contributor

An example:

$ cat a.py
a = 'f'
b = [2, 3]
c = f'{a}({", ".join(map(str, b))})'
print(c)

No errors from the plugin on the CPython 3.11:

$ python -V
Python 3.11.3+
$ pip install -U flake518 flake8-quotes
[...]
$ flake518 a.py
$

While on the CPython 3.12 I got this error: a.py:3:12: Q000 Double quotes found but single quotes preferred.

skirpichev added a commit to skirpichev/diofant that referenced this issue Sep 7, 2023
skirpichev added a commit to skirpichev/diofant that referenced this issue Sep 8, 2023
@mscuthbert
Copy link

This has to do with the change in f-string behavior (adoption of a formal grammar) in Python 3.12 -- now each { ... } expression creates its own little scope, so within that scope single quotes are allowed again.

There are two issues that arise from this:

  1. The vast majority of us still maintain libraries that are compatible with 3.11 and earlier. Fixing the problem breaks the code on older versions of Python
  2. Even when Python 3.12 is the minimum version I support, I'm not sure I will want to switch to using single quotes within f-strings that are themselves opened and closed with single quotes -- I think it shows the hierarchy that we are within an f-string.

So an option to disable checking inside f-strings would be most helpful for either or both of these reasons. Thank you!

inducer added a commit to inducer/pyopencl that referenced this issue Oct 6, 2023
In a way, this addresses
zheller/flake8-quotes#117, since it forces the
Python that runs flake8 onto 'oldest supported', currently 3.8.
inducer added a commit to inducer/pyopencl that referenced this issue Oct 6, 2023
In a way, this addresses
zheller/flake8-quotes#117, since it forces the
Python that runs flake8 onto 'oldest supported', currently 3.8.
szbernat added a commit to deepqmc/deepqmc that referenced this issue Oct 30, 2023
These double quotes raise the Q000 error with python 3.12 and higher.
However, using single quotes here would break support for python 3.11
and lower. See: zheller/flake8-quotes#117
@ThiefMaster
Copy link

Please make this configurable, e.g. by using a separate error code for quotes inside fstrings.

Because even on a 3.12-only codebase one may prefer to NOT use the same type of quotes simple for the sake of readability and being nice towards editors whose syntax highlighting may not properly handle reusing the same quotes.

@ThiefMaster
Copy link

@JoeHitchen or @zheller (since you are the maintainers who can actually make a new release besides @twolfson who stepped down according to another issue): Any chance of this being fixed? Or at least a bugfix release being done if someone sends a PR to fix it?

@JoeHitchen
Copy link
Collaborator

Hi - Yes, if there is a pull request that fixes the problem and passes the tests then I can cast an eye over it and make a release.

@arnimarj
Copy link
Contributor

Hi - Yes, if there is a pull request that fixes the problem and passes the tests then I can cast an eye over it and make a release.

I quickly hacked together a POC at #118, the state-machine tracking the nesting depth of f-strings is simple enough, I'll see how far I can take it this weekend

@arnimarj
Copy link
Contributor

@JoeHitchen I have a preliminary version which passes (when run manually on my local machine) for Python 3.11 and 3.12. The fix is not backwards compatible though, there are positives in 3.12, which weren't there in 3.11, and guidance on that point would be appreciated.

@JoeHitchen
Copy link
Collaborator

We wouldn't want to break backwards compatibility, but is there a way to inspect the python version before deciding to apply the new rule or not?
e.g. https://docs.python.org/3/library/sys.html#sys.version_info

@arnimarj
Copy link
Contributor

@JoeHitchen I've built a simple test harness using tox to run the test-suite on the various Python versions. I'm wondering if you'd consider a PR to automate these using Github Actions?

(BTW: I've not managed to test this successfully on Python 3.4/3.5, perhaps its time to drop support for those, 3.5 was EOLed in 2020)

@ThiefMaster
Copy link

ThiefMaster commented Nov 12, 2023

IMHO anything that's EOL, ie <=3.7 should be removed.

Someone using such an obsolete Python version can just use an old version of this package...

+1 on GH actions. Might also want to switch to pytest at some point since the current test runner is awful (in fact when I started working on this same thing yesterday before I saw you already being on it, it was quite easy to run even the existing tests using pytest)

@JoeHitchen
Copy link
Collaborator

I'd be happy to accept a PR to master which works for >=3.6 but drops 3.4 & 3.5. A release which formally drops 3.4 & 3.5 make take a bit longer as we probably need to tidy up some other bits (e.g. documentation) to reflect that change, but it will be installable from the repo for those that want it sooner.

Also agree that Github Actions would help test compatibility with different versions python versions.

@JoeHitchen
Copy link
Collaborator

Should be resolved following the merge of #118

Nudge nudge #109 if anyone on this thread is interested

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

No branches or pull requests

5 participants