WIP: Use assign expr for match/group#8097
Conversation
Replace:
m = regex.match(text)
if m:
return m.group(1)
with:
if (m := regex.match(text)):
return m.group(1)
Notes:
* Similar code using "if not m: return" is unchanged
* When the condition was more complex than "if m", the code is left
unchanged
|
As my previous PR #8095, I didn't write this PR to merge it, but just to discuss when it's appropriate or not to use assingment expressions (PEP 572). That's why I wrote "WIP" in the title and added the DO-NOT-MERGE label. |
|
See also PR #8115 for the general case. |
JimJJewett
left a comment
JimJJewett
left a comment
There was a problem hiding this comment.
Again, I find these to be improvements. That said, I would find them even better in " as " form.
With the current form, it helps to read " := " as " being ". So instead of "x is now . If x ...", I now see "if x (being ) ..."
I also think some of them would be improved by remembering that match objects are always true, and dropping the "is not None" clauses. Tastes may differ on that.
I don't see any of the long indented if situations that I remember seeing during discussion -- including one supposedly from the stdlib.
| else: | ||
| path = ml.split()[-1] | ||
| run('delete', (path,)) | ||
|
|
There was a problem hiding this comment.
Note that the original author was already doing
if: then_stuff
else: else_stuff
to keep it from taking up too much white space. Your rewrite is actually a line longer, but only because you've gone back to the standard "suite indented on the next line" format. So that suggests this is relieving a real problem.
| if m: | ||
| vars[m.group(1)] = 0 | ||
| elif (m := undef_rx.match(line)): | ||
| vars[m.group(1)] = 0 |
There was a problem hiding this comment.
This one looks like an improvement to me, because otherwise I smell a bug ... what if it went down the else case, but then the 2nd if wasn't true? Is that case handled?
Now I still smell the potential bug, but with an elif, it is a lot easier to explain what I'm worried about, as a simple "What if neither pattern matches?"
|
The PEP 572 has been accepted. I only wrote these WIP PEPs to help me to discuss the PEP. I never intended to propose these changes to be merged. So I close this PR. |
4 participants
Replace:
with:
Notes:
unchanged