-
-
Notifications
You must be signed in to change notification settings - Fork 31k
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
Multiline expression brackets with format specifiers don't work in f-strings #110259
Comments
Thanks for the report! I have opened #110271 to fix it |
…specs Signed-off-by: Pablo Galindo <[email protected]>
…specs Signed-off-by: Pablo Galindo <[email protected]>
I'd be careful on that last one. If >>> x=datetime.datetime.now()
>>> f"""__{
... x:d
... }__"""
'__d\n__' |
In case that's not clear: format specs for >>> f"""__{
... x:%Y
... %m
... %d
... }__"""
'__2023\n10\n03\n__' |
Hummmm, then what you are saying is that we should not include the newline in the single quoted case but we should do it in the triple quoted case? |
Ah, indeed, this is the case because the triple quote case works like that in 3.11:
|
I don't understand why this is the case. Shouldn't these both lead to the same AST? Multi-line f-string: f"""__{
x:%Y
%m
%d
}__""" Single-quoted f-string: f"__{
x:%Y
%m
%d
}__" |
I don't think it's necessarily an issue. I was just pointing out that the error shown in the original report f"""__{
x:d
}__""" (a ValueError from the bad format spec "d\n" for int) is different from the error in f"__{
x:d
}__" (a SyntaxError). In any event, the format spec should be allowed to have newlines, including trailing newlines. |
@ericvsmith the triple quoted issue is indeed different from the single quoted case. But, is that a bug? f"""{
5:.2f
}""" doesn't work on 3.11 and below as well, but I feel like it should. |
@tusharsadhwani Maybe I could be talked into changing it, but the change would have to be in This change cannot be in the parser, because whether or not whitespace in the format spec is an error or is acceptable is up to the type being formatted. As things stand now, |
Agreed. The change for single quotes should still go through IMO, because the AST should be identical for the formatted brackets even if the fstring gets split on multiple lines. For triple quotes the ASTs must be different because of backwards compatibility reasons. |
There is still ambiguity on how to treat the single quote case. The most natural way for the tokenizer is to stop the format string on the first
but this is not:
The other option is allowing multi-line format specs but that may be a bit more annoying to implement. @ericvsmith @lysnikolaou what do you think? |
PR #110271 implements single-line format specifiers for single quoted f-strings, but we can change it if we think multi-line specs should be allowed. |
It seems like an arbitrary limitation on code formatters: splitting any f-string on the expression brackets will be valid, unless it's the specific case of a single quoted fstring with a format specifier. |
Not really, you can still be able to split the f-string in the brackets, but not the format specifier. This is fine because if you split the format specifier you are changing the f-string anyway. To clarify, you can change:
to
if you want, but you cannot split the |
Makes sense, the linked PR seems good to me |
So basically, in other words, if single quoted f-strings format specifiers cannot spawn multiple lines themselves. |
@pablogsal So I think you're saying that for single quoted f-strings, there can't be a newline in the format spec. Although it looks like that's a change, I think it's okay. ETA: Our messages crossed. I think we're in agreement. |
I think that's okay as well. Tokenizing multi-line format specifiers in single-quoted f-strings would be a bit cumbersome, so let's go with that. |
Sorry, I am also on a plane, so the timing of my messages may be a bit inconsistent :P |
Update: i tried to check how annoying would be to support the other option (support multi-line format specifiers) and turns out that is quite easy to support thanks to how we have structured this (yay!). So only if we want we can support that. It would look like this, for clarification:
which prints:
Sorry for the back and forth @lysnikolaou @ericvsmith. I think we are free to choose whatever we prefer. |
Notice that this has an important difference that I think it makes is less appealing: extra trailing newlines are included in the format specifier, which I think it will make formatters unhappy (@tusharsadhwani, please confirm) |
Can confirm, second option will restrict code formatting, pretty much making fstrings with format specs unformattable. But, fstrings don't get formatted by black right now as-is and there's no confirmation on my end that that's desired. |
…110271) Signed-off-by: Pablo Galindo <[email protected]>
…specs (pythonGH-110271) (cherry picked from commit cc389ef) Co-authored-by: Pablo Galindo Salgado <[email protected]> Signed-off-by: Pablo Galindo <[email protected]>
@tusharsadhwani black can't reformat the format specs without breaking datetime.datetime (for example). @pablogsal I don't think it matters all that much. I can't imagine that there are many single quote multi-line format specs, although they no doubt exist. |
Ok, I ended implementing the single-line restruction for format specs in single-quoted f-strings. Thanks everyone for your help, you rock 🚀 |
… specs (GH-110271) (#110396) gh-110259: Fix f-strings with multiline expressions and format specs (GH-110271) (cherry picked from commit cc389ef) Signed-off-by: Pablo Galindo <[email protected]> Co-authored-by: Pablo Galindo Salgado <[email protected]>
@ericvsmith wasn't single quote multiline format spec invalid syntax until now? |
@tusharsadhwani : I thought somewhere on this thread someone said it worked in 3.11, but I guess I misread that. |
## Summary Reported at python/cpython#110259 ## Test Plan Add test cases for the fix and update the snapshots
## Summary Reported at python/cpython#110259 ## Test Plan Add test cases for the fix and update the snapshots
…specs (python#110271) Signed-off-by: Pablo Galindo <[email protected]>
Bug report
Bug description:
In accordance with PEP 701, the following code works:
But the following fails:
This gives:
Is this intended behaviour? This is not clarified in the PEP.
Similarly,
Gives:
CPython versions tested on:
3.12
Operating systems tested on:
macOS
Linked PRs
The text was updated successfully, but these errors were encountered: