-
-
Notifications
You must be signed in to change notification settings - Fork 32.2k
gh-132661: Disallow Template
/str
concatenation after PEP 750 spec update
#135996
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
base: main
Are you sure you want to change the base?
Conversation
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.
Thank you!
@@ -1902,6 +1907,7 @@ _PyPegen_concatenate_strings(Parser *p, asdl_expr_seq *strings, | |||
} | |||
} | |||
|
|||
|
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.
unrelated change :)
Py_TYPE(right)->tp_name); | ||
return NULL; | ||
} | ||
PyErr_Format(PyExc_TypeError, |
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.
You can use %T
to format object's type.
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.
The diff here is just a dedent of existing code; I was hesitant to change anything else. But happy to update it to use %T
if we'd prefer.
combined = t1 + ", world" | ||
self.assertTStringEqual(combined, ("Hello, world",), ()) | ||
self.assertEqual(fstring(combined), "Hello, world") | ||
with self.assertRaises(TypeError): |
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.
Will it make sense to check error message here?
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.
Good catch, will update.
Following the steering council decision and corresponding update to PEP750, we are removing support for both implicit and explicit
Template
/str
concatenation.@lysnikolaou I took a first pass at this, but it's still just a draft. I deleted the obvious code and updated our tests. But there's still more to clean up, both in
action_helpers.c
/_PyPegen_concatenate_strings
(I just raised a syntax error if templates/non-templates were mixed, rather than cleaning up code that now supports more than it needs to), and probably in_ast_unparse.py
amongst others. Happy to do this with your input, or happy to hand it off to you!