msg409101 - (view) |
Author: Gregory P. Smith (gregory.p.smith) * |
Date: 2021-12-23 19:58 |
Now that we have a shiny new parser, can we finally get rid of this language wart:
assert thing, description # works as intended
assert (thing, description) # always True as non-empty tuples are Truthy
This most often happens when extending thing or description beyond a single line on assert statements as () are the natural way to do that and as it is with assert being a statement, knowing specifically where to place the ()s to not fall into the pit of snakes of unintentionally nerfing your assertion to be an always true tuple is hard for human authors.
This would obsolete the pylint error about tuple assertion and enable more natural assert use.
py.test framework users would presumably rejoice as well.
This parsing change would need a PEP. I fail to see any obvious downsides though.
|
msg409102 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2021-12-23 20:15 |
It does not need any change in parser, it can be done in the code generator which currently explicitly warns about such ambiguity.
Although it needs changes in formal grammar which will be more complex.
But what is a benefit? What is an advantage of writing
assert (thing, description)
instead of
assert thing, description
?
|
msg409103 - (view) |
Author: Pablo Galindo Salgado (pablogsal) * |
Date: 2021-12-23 20:18 |
I can try to prototype something in the parser to see how it looks and work on the pep if everything looks ok.
Parentheses are a bit tricky in general as backtracking ends causing all sorts of tricky situations with custom syntax errors as the parser needs to distinguish between function calls, tuple construction and grouping so that's the only dangerous situation I can think of
|
msg409105 - (view) |
Author: Gregory P. Smith (gregory.p.smith) * |
Date: 2021-12-23 20:35 |
It's not about an advantage, it's about removing the problem of what edit to make when working on
assert thing_that_has_a_meaningful_name.methods_have_good_names(value_from_somewhere) == other_thing_that_is_meaningful, "Description of what the issue is if this fails for humans, as if the names weren't enough"
and making that fit within whatever line length limit your codebase has.
put () in the wrong place and it triggers the long standing Python wart or parsing as a tuple.
rather than warn about the syntax wart, we should just do the thing code authors want in the first place.
|
msg409115 - (view) |
Author: Raymond Hettinger (rhettinger) * |
Date: 2021-12-24 00:17 |
> can we finally get rid of this language wart
Yes, please. This is a pretty bad pitfall.
I've seen this happen to people who've been conditioned by other languages to think of assert() as a macro or function:
assert(sometest, somemessage)
|
msg409117 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2021-12-24 01:01 |
We managed to do this for 'with' so it should be possible here too, I'd think. The "committing" token would be the newline following the close parenthesis.
Serhiy: the benefit is when you want to split it across two lines, e.g.
assert (a_very_long_condition(),
"A Very Long Message")
I know you can do this using backslash, e..
assert a_very_long_condition(), \
"A Very Long Message"
but there's a strong cultural rejection of backslash for line splitting (it's in PEP 8 and engrained in many people's brains) and it's just so natural to use parentheses -- they work for 'import', 'with', function calls, 'if', etc.
|
msg409122 - (view) |
Author: Pablo Galindo Salgado (pablogsal) * |
Date: 2021-12-24 05:09 |
> We managed to do this for 'with' so it should be possible here too, I'd think. The "committing" token would be the newline following the close parenthesis.
I am not so sure is that inmediate. Changing the assert statement from:
'assert' a=expression b=[',' z=expression { z }]
to
| 'assert' '(' a=expression b=[',' z=expression { z }] ')'
| 'assert' a=expression b=[',' z=expression { z }]
will render this invalid:
assert (a, b) <= c, "something"
The reason is that it will parse the (a, b) as the assert statement eagerly and then it will fail to parse the rest.
|
msg409123 - (view) |
Author: Pablo Galindo Salgado (pablogsal) * |
Date: 2021-12-24 05:14 |
We could do this, but feels a bit hacky:
| 'assert' '(' a=expression b=[',' z=expression { z }] ')' &(NEWLINE | ';')
| 'assert' a=expression b=[',' z=expression { z }]
|
msg409124 - (view) |
Author: Pablo Galindo Salgado (pablogsal) * |
Date: 2021-12-24 05:30 |
Another possibility is actually handled this in the compiler:
if we see an assert with a tuple of two elements, we can assume is basically in the form that we want and proceed as if is in the form
assert A, B
This also feels a bit hacky because the AST is somehow wrong as the assert node is already prepared to differentiate these two cases.
|
msg409126 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2021-12-24 06:04 |
I like the lookahead. We could also make the comma and the message mandatory when inside parentheses:
| 'assert' '(' a=expression ',' b=expression [','] ')' &(NEWLINE | ';')
| 'assert' a=expression b=[',' z=expression { z }]
(And probably add an "invalid" rule to cover tuples with 0, 1, 3 or more elements.)
|
msg409132 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2021-12-24 08:44 |
For very long expression or very long message you can add parentheses around the expression or message. Black will format it as:
assert (
very very long
expression
), (
"very very long "
"message"
)
With the proposed feature it will be:
assert (
very very long
expression,
"very very long "
"message",
)
It saves one line, but the border between an expression and a message is blur. Since they are separated by a comma at the end of line and all lines have the same indentation it looks less readable to me.
Note also that Black adds a comma after message.
|
msg409147 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2021-12-24 15:57 |
You don’t have to use the new feature. But people expect it to work.
|
msg410178 - (view) |
Author: Pablo Galindo Salgado (pablogsal) * |
Date: 2022-01-10 00:22 |
I created a PEP to formally propose the change:
https://www.python.org/dev/peps/pep-0679/
|
|
Date |
User |
Action |
Args |
2022-04-11 14:59:53 | admin | set | github: 90325 |
2022-01-10 00:22:08 | pablogsal | set | messages:
+ msg410178 |
2021-12-24 15:57:30 | gvanrossum | set | messages:
+ msg409147 |
2021-12-24 08:44:22 | serhiy.storchaka | set | messages:
+ msg409132 |
2021-12-24 06:30:21 | pablogsal | set | keywords:
+ patch stage: patch review pull_requests:
+ pull_request28465 |
2021-12-24 06:04:48 | gvanrossum | set | messages:
+ msg409126 |
2021-12-24 05:30:57 | pablogsal | set | messages:
+ msg409124 |
2021-12-24 05:14:07 | pablogsal | set | messages:
+ msg409123 |
2021-12-24 05:09:07 | pablogsal | set | messages:
+ msg409122 |
2021-12-24 01:01:21 | gvanrossum | set | messages:
+ msg409117 |
2021-12-24 00:17:10 | rhettinger | set | nosy:
+ rhettinger messages:
+ msg409115
|
2021-12-23 23:11:48 | xtreak | set | nosy:
+ gvanrossum, serhiy.storchaka
|
2021-12-23 20:35:19 | gregory.p.smith | set | messages:
+ msg409105 |
2021-12-23 20:18:06 | pablogsal | set | nosy:
- gvanrossum, serhiy.storchaka messages:
+ msg409103
|
2021-12-23 20:15:15 | serhiy.storchaka | set | nosy:
+ serhiy.storchaka, gvanrossum messages:
+ msg409102
|
2021-12-23 20:01:30 | slebedev | set | nosy:
+ slebedev
|
2021-12-23 19:58:53 | gregory.p.smith | create | |