This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Simplify some while-loops with walrus operator
Type: Stage: patch review
Components: Library (Lib) Versions: Python 3.11
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: AlexWaygood, iritkatriel, miss-islington, nickdrozd, rhettinger, terry.reedy, zach.ware
Priority: normal Keywords: patch

Created on 2021-12-03 23:15 by nickdrozd, last changed 2022-04-11 14:59 by admin.

Pull Requests
URL Status Linked Edit
PR 29347 open nickdrozd, 2021-12-03 23:15
PR 31083 merged nickdrozd, 2022-02-02 17:24
PR 31091 merged miss-islington, 2022-02-03 01:59
PR 31092 merged terry.reedy, 2022-02-03 02:30
PR 31107 merged terry.reedy, 2022-02-03 19:03
PR 31109 merged miss-islington, 2022-02-03 19:44
PR 31110 merged miss-islington, 2022-02-03 19:44
Messages (13)
msg407615 - (view) Author: Nick Drozd (nickdrozd) * Date: 2021-12-03 23:15
The following pattern occurs a few times in the codebase:

  while 1:
      data = conn.recv(blocksize)
      if not data:
          break
      ...

There is an unbounded while loop in which some kind of input is read. If the input is there, it is processed and the loop is continued; otherwise the loop is broken.

This can be expressed more cleanly with the walrus operator:

  while data := conn.recv(blocksize):
     ...

Some of this code goes back to the days of Python 1. I assume that the original authors would have used the walrus idiom if it had been available, which it wasn't. But now it is.

Rewriting the instances of this pattern shaves off 148 lines from the codebase. Removing the manual break statements makes the logic more straightforward.

This should not change the behavior of anything. I'm assuming that this code is all under test and that any deviations from the existing behavior will be caught. Anything that isn't tested shouldn't be changed (and should be tested).
msg407619 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2021-12-03 23:46
As a general rule, refactorings like this tend to be rejected as the risk of inadvertently adding a new bug outweighs the benefit of subjectively cleaner code ("if it ain't broke, don't fix it!").

Is there any measurable performance benefit from this? Reducing several hundred thousand lines of code by 148 is not a compelling benefit :)
msg407629 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2021-12-04 02:49
The general rule as stated by Zachary is correct; however, I'm in mildly favor of this PR because these are the use cases that the walrus operator was specifically designed for.  That said, it would be nice to verify that timings don't get worse and to carefully review each edit to make sure it doesn't introduce a bug.
msg407649 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2021-12-04 09:53
As I mentioned in the PR, merging this can make future backports to 3.6 and 3.7 more complicated.
msg407675 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2021-12-04 15:58
> As I mentioned in the PR, merging this can make future
> backports to 3.6 and 3.7 more complicated.

That's correct, but it is also true that we do very few of those and the likelihood of a conflict with one of these edits is low.
msg407684 - (view) Author: Nick Drozd (nickdrozd) * Date: 2021-12-04 20:22
> Is there any measurable performance benefit from this?

I wouldn't expect any performance changes either way. If it worked out to be slower, that would an unpleasant surprise and a good reason to reject this change. If it worked out to be faster, well, that would be great!

> Reducing several hundred thousand lines of code by 148 is not a compelling benefit :)

But it's not just any old 148 lines. By my count it includes the removal of 47 `break` statements. For a change of this nature there's certainly a chance of introducing errors. On the other hand, every one of those `break` statements is a site of manual loop-handling logic, and those already present some risk of introducing errors.
msg411839 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2022-01-27 04:49
To me, the five idlelib changes make the code easier to understand.  Nick, please either move them into a separate PR or allow me to do so, and subject to manual testing, I will merge and backport.  (I requested this on the PR by maybe you missed it.)

If there are other plausible changes in idlelib that you would like considered, make a spinoff issue where we can discuss them first.
msg412410 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2022-02-03 01:59
New changeset 51a95be1d035a717ab29e98056b8831a98e61125 by Nick Drozd in branch 'main':
bpo-45975: Use walrus operator for some idlelib while loops (GH-31083)
https://github.com/python/cpython/commit/51a95be1d035a717ab29e98056b8831a98e61125
msg412411 - (view) Author: miss-islington (miss-islington) Date: 2022-02-03 02:28
New changeset 2ddc278875f789de622262ee8ff5a1c3788f031c by Miss Islington (bot) in branch '3.10':
bpo-45975: Use walrus operator for some idlelib while loops (GH-31083)
https://github.com/python/cpython/commit/2ddc278875f789de622262ee8ff5a1c3788f031c
msg412413 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2022-02-03 03:12
New changeset fafd2dadf63973a04f5693e5be19f3e7521c10d4 by Terry Jan Reedy in branch '3.9':
[3.9] bpo-45975: Use walrus operator for some idlelib while loops (GH-31083)
https://github.com/python/cpython/commit/fafd2dadf63973a04f5693e5be19f3e7521c10d4
msg412459 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2022-02-03 19:44
New changeset 916d0d822c79933f4c420f7a36f16f3eb788646b by Terry Jan Reedy in branch 'main':
bpo-45975: IDLE - Remove extraneous parens (GH-31107)
https://github.com/python/cpython/commit/916d0d822c79933f4c420f7a36f16f3eb788646b
msg412466 - (view) Author: miss-islington (miss-islington) Date: 2022-02-03 20:44
New changeset 63523e7b2a631f28134b25a8063d50e08c741db6 by Miss Islington (bot) in branch '3.10':
bpo-45975: IDLE - Remove extraneous parens (GH-31107)
https://github.com/python/cpython/commit/63523e7b2a631f28134b25a8063d50e08c741db6
msg412467 - (view) Author: miss-islington (miss-islington) Date: 2022-02-03 20:44
New changeset cf7cb1a2bf40516dc571d1d90c12b632dcd9b8c8 by Miss Islington (bot) in branch '3.9':
bpo-45975: IDLE - Remove extraneous parens (GH-31107)
https://github.com/python/cpython/commit/cf7cb1a2bf40516dc571d1d90c12b632dcd9b8c8
History
Date User Action Args
2022-04-11 14:59:53adminsetgithub: 90133
2022-02-03 20:44:27miss-islingtonsetmessages: + msg412467
2022-02-03 20:44:20miss-islingtonsetmessages: + msg412466
2022-02-03 19:44:59miss-islingtonsetpull_requests: + pull_request29293
2022-02-03 19:44:54miss-islingtonsetpull_requests: + pull_request29292
2022-02-03 19:44:39terry.reedysetmessages: + msg412459
2022-02-03 19:03:57terry.reedysetpull_requests: + pull_request29290
2022-02-03 03:12:47terry.reedysetmessages: + msg412413
2022-02-03 02:30:06terry.reedysetpull_requests: + pull_request29277
2022-02-03 02:28:56miss-islingtonsetmessages: + msg412411
2022-02-03 01:59:33miss-islingtonsetnosy: + miss-islington
pull_requests: + pull_request29276
2022-02-03 01:59:31terry.reedysetmessages: + msg412410
2022-02-02 17:24:57nickdrozdsetkeywords: + patch
stage: patch review
pull_requests: + pull_request29267
2022-01-27 04:49:19terry.reedysetnosy: + terry.reedy
messages: + msg411839
2021-12-04 20:27:35AlexWaygoodsetnosy: + AlexWaygood
2021-12-04 20:22:08nickdrozdsetmessages: + msg407684
2021-12-04 15:58:33rhettingersetmessages: + msg407675
2021-12-04 09:53:06iritkatrielsetnosy: + iritkatriel
messages: + msg407649
2021-12-04 02:49:48rhettingersetnosy: + rhettinger
messages: + msg407629
2021-12-03 23:46:36zach.waresetnosy: + zach.ware
messages: + msg407619
2021-12-03 23:15:50nickdrozdcreate