classification
Title: unnecessary LBYL for key contained in defaultdict, lib2to3/btm_matcher
Type: Stage: resolved
Components: 2to3 (2.x to 3.x conversion tool) Versions: Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Jim Fasarakis-Hilliard, Mariatta, benjamin.peterson, rhettinger, selik
Priority: normal Keywords:

Created on 2017-04-01 04:37 by selik, last changed 2017-04-08 21:43 by Mariatta. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 938 merged python-dev, 2017-04-01 05:04
Messages (9)
msg290955 - (view) Author: Michael Selik (selik) * Date: 2017-04-01 04:37
Minor, but it looks like someone decided to use a defaultdict but forgot to remove the checks for whether a key exists.

Creating a defaultdict(list):
https://github.com/python/cpython/blob/6f0eb93183519024cb360162bdd81b9faec97ba6/Lib/lib2to3/btm_matcher.py#L100

Checking for the key, then initializing an empty list:
https://github.com/python/cpython/blob/6f0eb93183519024cb360162bdd81b9faec97ba6/Lib/lib2to3/btm_matcher.py#L120

Again:
https://github.com/python/cpython/blob/6f0eb93183519024cb360162bdd81b9faec97ba6/Lib/lib2to3/btm_matcher.py#L137

Because the ``results`` is getting returned, perhaps it'd be better to use a regular dict and dict.setdefault instead of a defaultdict.
msg290956 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2017-04-01 04:42
Nice catch.  You've got sharp eyes.

Do you want to submit a pull request or have another dev work on it?
msg290957 - (view) Author: Michael Selik (selik) * Date: 2017-04-01 04:43
I'll submit a pull request momentarily.
msg290960 - (view) Author: Michael Selik (selik) * Date: 2017-04-01 05:11
PR submitted. I also signed the contributor agreement, but the bot doesn't seem to have noticed.
msg290962 - (view) Author: Mariatta (Mariatta) * (Python committer) Date: 2017-04-01 05:19
Thanks for the PR Michael :)

It may take one US business day for the CLA to be received.
Then a core dev has to manually update the label in GitHub.
msg290993 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2017-04-01 15:56
Rather than removing the defaultdict, I think a cleaner fix is to just remove the unnecessary LBYL.   That leaves the code a little more compact, faster, and nice looking.
msg290998 - (view) Author: Michael Selik (selik) * Date: 2017-04-01 16:30
Ok, I'll change the PR.
msg291025 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2017-04-02 06:02
New changeset 11fa3c7cd1b151e302d4eee0369cafbaf151c8fb by Benjamin Peterson (Michael Selik) in branch 'master':
bpo-29957: change LBYL key lookup to dict.setdefault (#938)
https://github.com/python/cpython/commit/11fa3c7cd1b151e302d4eee0369cafbaf151c8fb
msg291339 - (view) Author: Jim Fasarakis-Hilliard (Jim Fasarakis-Hilliard) * Date: 2017-04-08 17:49
bump to close issue now that PR was merged
History
Date User Action Args
2017-04-08 21:43:04Mariattasetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2017-04-08 17:49:14Jim Fasarakis-Hilliardsetnosy: + Jim Fasarakis-Hilliard
messages: + msg291339
2017-04-02 06:02:33benjamin.petersonsetmessages: + msg291025
2017-04-01 16:30:17seliksetmessages: + msg290998
2017-04-01 15:56:28rhettingersetmessages: + msg290993
2017-04-01 05:56:24serhiy.storchakasetnosy: + benjamin.peterson
2017-04-01 05:20:21Mariattasetstage: patch review
versions: + Python 3.7
2017-04-01 05:19:07Mariattasetmessages: + msg290962
2017-04-01 05:11:32seliksetmessages: + msg290960
2017-04-01 05:04:09python-devsetpull_requests: + pull_request1121
2017-04-01 04:43:25seliksetmessages: + msg290957
2017-04-01 04:42:20rhettingersetnosy: + rhettinger, Mariatta
messages: + msg290956
2017-04-01 04:37:19selikcreate