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

Fix flake8 warnings about whitespace before/after f-string backets #1511

Closed
wants to merge 1 commit into from

Conversation

pawamoy
Copy link
Contributor

@pawamoy pawamoy commented Feb 21, 2025

No description provided.

@pawamoy pawamoy mentioned this pull request Feb 21, 2025
4 tasks
@waylan
Copy link
Member

waylan commented Feb 21, 2025

Hmm, this was an intentional style choice. I find it easier to read with the whitespace. I'm assuming this is a new addition to flake8. Can you point me to the warning and/or the documentation? We may want to disable this new rule and/or enforce the opposite behavior.

@pawamoy
Copy link
Contributor Author

pawamoy commented Feb 21, 2025

They show up as E201 and E202:

% uvx tox -e flake8
flake8: install_deps> python -I -m pip install flake8
flake8: commands[0]> flake8 /media/data/dev/markdown/markdown /media/data/dev/markdown/tests
/media/data/dev/markdown/markdown/extensions/abbr.py:124:39: E201 whitespace after '{'
/media/data/dev/markdown/markdown/extensions/abbr.py:124:85: E202 whitespace before '}'
/media/data/dev/markdown/markdown/postprocessors.py:89:34: E201 whitespace after '{'
/media/data/dev/markdown/markdown/postprocessors.py:89:58: E202 whitespace before '}'
/media/data/dev/markdown/markdown/postprocessors.py:97:40: E201 whitespace after '{'
/media/data/dev/markdown/markdown/postprocessors.py:97:57: E202 whitespace before '}'
/media/data/dev/markdown/markdown/postprocessors.py:97:65: E201 whitespace after '{'
/media/data/dev/markdown/markdown/postprocessors.py:97:82: E202 whitespace before '}'
flake8: exit 1 (0.50 seconds) /media/data/dev/markdown> flake8 /media/data/dev/markdown/markdown /media/data/dev/markdown/tests pid=326045
  flake8: FAIL code 1 (2.80=setup[2.30]+cmd[0.50] seconds)
  evaluation failed :( (2.86 seconds)

They come from pycodestyle. Strangely, only ( and ) are mentioned in the docs, not { and }: https://pycodestyle.pycqa.org/en/latest/intro.html#error-codes. Can't find anything relevant in their changelog.

Happy to revert and ignore these rules instead 🙂

@pawamoy
Copy link
Contributor Author

pawamoy commented Feb 21, 2025

pycodestyle's docs are not up to date... Latest version is 2.12.1, changelog only shows 2.11.1: https://pycodestyle.pycqa.org/en/latest/developer.html#id2. This was reported here: PyCQA/pycodestyle#1271.

@pawamoy
Copy link
Contributor Author

pawamoy commented Feb 21, 2025

OK so the errors don't show up in CI because it uses Python 3.11, and locally I used 3.13. They only show up on 3.12+ apparently.

@waylan
Copy link
Member

waylan commented Feb 25, 2025

Looks like this is the relevant changelog entry:

https://github.com/PyCQA/pycodestyle/blob/b852994a89639577d86070f513a9c186a74479b6/CHANGES.txt#L4-L10

That points to PyCQA/pycodestyle#1252 which in turn points to PyCQA/flake8#1949. However, I don't see how those directly connect to this. Unless the issue addressed there was causing f-strings to not be checked previously. Perhaps this change in behavior is an unexpected side affect.

In any event, the relevant error codes (E201 and E202) are just the basic codes for any Python code in brackets or braces. If we turn those off, that turns them off for all Python code. I only want the check disabled for the delimitators of f-string replacement fields (but not the contents of the replacement field or anywhere else).

I think I'm going to sit on this for now and see how things play out.

@pawamoy
Copy link
Contributor Author

pawamoy commented Feb 26, 2025

Fine by me 😊 Just note that contributors running flake8 on 3.12+ will get the warnings and might try fixing them again, so maybe you could try to enforce the Python version used for this tox env, if that's possible 🙂

@pawamoy pawamoy closed this Feb 26, 2025
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