➜
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.
Created on 2017-04-01 04:37 by selik, last changed 2022-04-11 14:58 by admin. 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) *  |
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) * |
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) * |
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) * |
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 |
| 2022-04-11 14:58:44 | admin | set | github: 74143 |
| 2017-04-08 21:43:04 | Mariatta | set | status: open -> closed resolution: fixed stage: patch review -> resolved |
| 2017-04-08 17:49:14 | Jim Fasarakis-Hilliard | set | nosy:
+ Jim Fasarakis-Hilliard messages: + msg291339 |
| 2017-04-02 06:02:33 | benjamin.peterson | set | messages: + msg291025 |
| 2017-04-01 16:30:17 | selik | set | messages: + msg290998 |
| 2017-04-01 15:56:28 | rhettinger | set | messages: + msg290993 |
| 2017-04-01 05:56:24 | serhiy.storchaka | set | nosy:
+ benjamin.peterson |
| 2017-04-01 05:20:21 | Mariatta | set | stage: patch review versions: + Python 3.7 |
| 2017-04-01 05:19:07 | Mariatta | set | messages: + msg290962 |
| 2017-04-01 05:11:32 | selik | set | messages: + msg290960 |
| 2017-04-01 05:04:09 | python-dev | set | pull_requests: + pull_request1121 |
| 2017-04-01 04:43:25 | selik | set | messages: + msg290957 |
| 2017-04-01 04:42:20 | rhettinger | set | nosy:
+ rhettinger, Mariatta messages: + msg290956 |
| 2017-04-01 04:37:19 | selik | create | |