-
-
Notifications
You must be signed in to change notification settings - Fork 8.6k
py: Add PEP 750 template strings support #17557
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: master
Are you sure you want to change the base?
py: Add PEP 750 template strings support #17557
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #17557 +/- ##
==========================================
+ Coverage 98.38% 98.43% +0.05%
==========================================
Files 171 174 +3
Lines 22300 23038 +738
==========================================
+ Hits 21939 22677 +738
Misses 361 361 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Code size report: |
dpgeorge
left a comment
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.
Thanks for the contribution! This will be nice to have for the webassembly port.
I didn't do a review yet, but there will need to be tests to get full coverage of the new code.
|
@dpgeorge Thank you for the feedback and for taking time to look at this! I'm glad this will be useful for the webassembly port. I'll add more tests to ensure full coverage when I find time. |
|
@koxudaxi slightly off topic - but I notice you'll be at EuroPython in Prague, as will I. We should look out for each other and have a coffee or lunch together! 🇪🇺 🐍 |
|
@ntoll Sounds good! See you in Prague. ☕ |
7a1dc11 to
50dd4d1
Compare
I tried really hard to make 100% coverage but some code is never called and I cannot cover it. Do you know how to fix this? |
|
@dpgeorge |
| return MP_OBJ_FROM_PTR(result); | ||
| } | ||
|
|
||
| case MP_BINARY_OP_REVERSE_ADD: { |
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 just remove this case, it's not needed.
a567601 to
a5ced29
Compare
| # Whitespace in expression (MicroPython preserves, CPython strips trailing) | ||
| t_ws = t"{ 42 }" | ||
| print(f"Whitespace: {str(t_ws)}") |
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.
This behaves differently from CPython, but keeping it unchanged for now since CPython may have a bug.
Currently checking with Core developers to confirm if it's a bug.
1782b36 to
4037aad
Compare
|
@AJMansfield |
29e8816 to
bdf9e9e
Compare
bdf9e9e to
e66ee9a
Compare
Signed-off-by: Koudai Aono <[email protected]>
Signed-off-by: Koudai Aono <[email protected]>
Signed-off-by: Koudai Aono <[email protected]>
Signed-off-by: Koudai Aono <[email protected]>
Signed-off-by: Koudai Aono <[email protected]>
e66ee9a to
7292a59
Compare
Signed-off-by: Koudai Aono <[email protected]>
7292a59 to
d0b2c33
Compare
|
After taking another look at this PR for the first time in a while, I noticed that coverage was insufficient due to some dead code that had been left behind, so I removed it. However, there now appear to be errors in the Windows environment and the esp32 port that seem unrelated to this change. |
|
@koxudaxi I had a good look at this PR and how it parses t-strings. From what I understand, it creates a new nested parser when it encounters a t-string, in order to parse the expression parts of the t-string. That seems rather complicated and probably unnecessary, at least for MicroPython's implementation of t-strings. As far as I understand, t-strings are syntatically equivalent to f-strings (except that the f prefix is a t). So I'm wondering if it would be possible to reuse the f-string parser that we already have? The way f-strings work at the moment is that the lexer dynamically transforms the incoming f-string bytes into a new set of bytes for a corresponding str-format expression, like this: And then the lexer itself tokenizes the transformed bytes. So the parser only ever sees the RHS of the above, the parser never sees any f-strings. Could we make it so t-strings worked the same way? Something like: That could reuse most of the code for f-string parsing, and make the implementation minimal and quite efficient. That doesn't support nested t-strings, but I don't think we need that. We don't support nested f-strings. |
... ouch ... for the most common/popular use case, which is X/HTML representation, does that mean that this code would not work neither, because you cannot have an div = html(t"<div>{f"Hello {user}!"}</div>")This might be a blocker in terms of ergonomics / capabilities, but maybe it won't be the end of the world (as in: one simply factors out via lambdas all partial |
|
@dpgeorge
Even if we ignore (2) entirely, I don’t think a simple lexer-rewrite approach that only scans for delimiters (similar to the current f-string handling) is robust enough for (1), because inner |
Actually, MicroPython does already support this form of nesting in f-strings! We just had a test added for that in #18495. So it's not out of the question that f-strings can support nested f-strings using the current lexer-rewrite approach. And by extension, nested t-strings could also work with that approach. Eg the lexer would have both an
IMO the |
|
For the record, the f-string feature currently costs about 470 bytes (on ARM Cortex-M), and this t-string implementation costs about 7400 bytes (on ARM Cortex-M, including the frozen code for |
|
In #18588 I modified the existing f-string parser to support nested f-strings, with nesting to arbitrary depth. I think the same algorithm could now be used for a t-string parser. |
Implements PEP 750 template strings for MicroPython.
Started in discussion #17497. Template strings return Template objects instead of strings, allowing access to literal parts and expressions separately.
Changes:
Usage:
Implementation:
Testing:
Tested on unix (coverage, standard, minimal), windows, webassembly.
New tests in tests/basics/:
Feature detection in tests/feature_check/tstring.py - automatic skip when MICROPY_PY_TSTRINGS=0.
All existing tests pass.
Trade-offs:
Code size: ~10 KB when enabled, zero when disabled.
String module: minimal builtin with only templatelib blocks micropython-lib's full string module. Future: coordinate with micropython-lib.
Config: enabled when MICROPY_CONFIG_ROM_LEVEL >= EXTRA_FEATURES, requires MICROPY_PY_FSTRINGS=1.
To disable:
make CFLAGS_EXTRA=-DMICROPY_PY_TSTRINGS=0