-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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 parse test case on Python 3.12 #15577
Conversation
This fixes the test case testFStringWithFormatSpecifierExpression on Python 3.12. Strip redundant empty string literal from AST generated from f-string. It's not generated on earlier Python versions and it seems fine to just drop it.
elif len(strs_to_join.items) > 1: | ||
last = strs_to_join.items[-1] | ||
if isinstance(last, StrExpr) and last.value == "": | ||
# 3.12 can add an empty literal at the end. Delete it for consistency |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting, why is that needed in CPython 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting, why is that needed in CPython 🤔
It caused a crash over at flake8-bugbear too: PyCQA/flake8-bugbear#393
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this has caused issues in two projects, should we report upstream?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be worth it, yeah
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
Looks like this might be changed in CPython, so maybe we should just wait for the next 3.12 beta rather than merging this: |
We can wait until the next beta, unless perhaps we want to enable 3.12 in CI before the next beta is available. I'll keep this open for now. |
For now could we maybe just skip the test? I can't remember the pytest equivalent off the top of my head, but for unittest I'd just decorate the test with: @skipIf(
sys.version_info[:4] == (3, 12, 0, "beta") and sys.version_info[4] < 4,
"Early 3.12 betas had a bug"
) Though mypy's test suite is very data-driven I guess, so maybe it's more difficult to be that granular for a single test case |
This is still failing for me on Python 3.12.0 beta 4. I'm leaning towards merging this. We can always revert this afterwards if it's no longer needed. |
Makes sense! |
This fixes the test case testFStringWithFormatSpecifierExpression on Python 3.12.
Strip redundant empty string literal from AST generated from f-string. It's not generated on earlier Python versions and it seems fine to just drop it.