Issue 31417: Use the enumerate function where appropriate (original) (raw)
Created on 2017-09-11 10:43 by Mark Byrne, last changed 2022-04-11 14:58 by admin. This issue is now closed.
Messages (8)
Author: Mark Byrne (Mark Byrne) *
Date: 2017-09-11 10:43
To make code explicit and more readable
Use the enumerate function to replace occurrences of the pattern:
for i in range(len(sources)): src = sources[i]
Author: R. David Murray (r.david.murray) * 
Date: 2017-09-11 15:29
Thanks for wanting to improve Python, but we don't usually accept refactoring requests like this. We "fix" such issues when the code is touched for other reasons. We'll see what other developers think, though.
Author: STINNER Victor (vstinner) *
Date: 2017-09-11 15:49
Thanks for wanting to improve Python, but we don't usually accept refactoring requests like this. We "fix" such issues when the code is touched for other reasons. We'll see what other developers think, though.
The standard library is used by some developers are the reference style for best practices. So it makes sense to "upgrade" to code to use enumarate(). I read the short change, enumerate() usage is appropriate.
Author: R. David Murray (r.david.murray) *
Date: 2017-09-11 17:55
Anyone who uses stdlib code as examples of best practice doesn't understand the history of stdlib code.
Generally, we consider the danger of introducing bugs to be more significant than the benefit of the "cleanup" changes. The fact that you are discussing further changes to the code in the PR make this more likely.
Author: Serhiy Storchaka (serhiy.storchaka) *
Date: 2017-09-11 21:30
I concur with David in general. And as you can see there are questions to the concrete patch.
Author: Mark Byrne (Mark Byrne) *
Date: 2017-09-11 22:37
Generally, we consider the danger of introducing bugs to be more significant than the benefit of the "cleanup" changes. The fact that you are discussing further changes to the code in the PR make this more likely.
I haven't updated the PR with further code changes as suggested in the PR comments because the comments don't strictly relate to the changes put forward in this issue, and I don't want to go beyond the scope of the issue - the requested changes could go in a separate PR as needed.
The motivation is to use enumerate where it is a natural fit. If you decide not to merge, then that is ok; let me know if I can help with anything.
Author: R. David Murray (r.david.murray) *
Date: 2017-09-12 01:28
Mark: Yeah, I think my comment was directed more to haypo than you :)
Author: Vedran Čačić (veky) *
Date: 2017-09-12 05:22
Sorry, this seems like a classical example of "When all you have is a hammer, everything looks like a nail. Then, when you learn of a screwdriver, you suddenly see half of these nails as screws, completely ignoring there are other tools besides those two."
In fact, enumerate is not the good choice in most of those cases, as the comments show. One should use zip, another should use ordinary iteration because the index isn't needed at all, and yet another should probably stay as it is, since that way it's more obvious that we're transforming each item in place. [When LHS is consts[i] and right is const, it's not easy to see they refer to the same place.]
Even the fourth I would argue is more readable in the current form, for a similar reason: when you base the decision on words[i], and the decision is to del words[i:], it's obvious you delete "it and everything after". Not so much in the "refactored" code.
Please don't do such things. One of Python's great strengths, especially in stdlib, is code stability. The only reason we're able to get anything done is that we have (or at least used to have) a great policy referred to in https://bugs.python.org/issue31417#msg301879, so we don't lose ourselves in enormous bikeshedding about how to best use the enormous expressivity of Python to write the code that works perfectly fine in some slightly different way.
History
Date
User
Action
Args
2022-04-11 14:58:52
admin
set
github: 75598
2017-09-12 17:27:53
serhiy.storchaka
set
status: open -> closed
resolution: not a bug
stage: patch review -> resolved
2017-09-12 15:19:29
rhettinger
set
assignee: rhettinger ->
2017-09-12 05:22:38
veky
set
nosy: + veky
messages: +
2017-09-12 01:28:02
r.david.murray
set
messages: +
2017-09-11 22:37:08
Mark Byrne
set
messages: +
2017-09-11 21:30:33
serhiy.storchaka
set
nosy: + serhiy.storchaka
messages: +
2017-09-11 17:55:56
r.david.murray
set
messages: +
2017-09-11 15:49:03
vstinner
set
nosy: + vstinner
messages: +
2017-09-11 15:29:36
r.david.murray
set
nosy: + r.david.murray
messages: +
2017-09-11 11:56:52
serhiy.storchaka
set
assignee: rhettinger
nosy: + rhettinger
2017-09-11 10:52:01
python-dev
set
keywords: + patch
stage: patch review
pull_requests: + <pull%5Frequest3490>
2017-09-11 10:43:46
Mark Byrne
create