bpo-30166: Import command-line parsing modules only when needed.#1293
bpo-30166: Import command-line parsing modules only when needed.#1293serhiy-storchaka merged 4 commits intopython:masterfrom
Conversation
|
@serhiy-storchaka, thanks for your PR! By analyzing the history of the files in this pull request, we identified @kbkaiser, @terryjreedy and @tim-one to be potential reviewers. |
berkerpeksag
left a comment
There was a problem hiding this comment.
I'm not familiar with IDLE, but rest of the PR looks good to me. Just left a comment about code styling.
Thanks!
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| import argparse |
There was a problem hiding this comment.
Style nit: You may want to add a new line after import argparse like the other cases.
There was a problem hiding this comment.
This is a thing that compelled me to hesitate. Some usages of argparse have an empty line after imports, some doesn't have (and the majority doesn't have). I added a new line if the following code is large or separated on sections by empty lines, and didn't add it if the following code is short and dense, as in this case.
|
I left reviewing IDLE on @terryjreedy. |
terryjreedy
left a comment
There was a problem hiding this comment.
Moving imports not needed in the main body is consistent with what I did in Fall 2015. It is also how I introduced a bug. So I checked that os can be moved in profile.py and verified the changes in idlelib/pyshell.py. (They will be helpful if I ever move main() to a different module.) The one requested change is obviously not crucial.
| import warnings | ||
|
|
||
| from idlelib import testing # bool value | ||
| import idlelib |
There was a problem hiding this comment.
Move this also to main. 'idlelib' by itself is never used in main body of module.
|
|
||
|
|
||
| import sys | ||
| import os |
There was a problem hiding this comment.
def trace_dispatch (line 184 in 3.6.1) is preceded by an apparently out-of-date comment
# Heavily optimized dispatch routine for os.times() timer
That os.times is no longer used is why the os import can be moved.
self.timer is now time.process time
|
I have backported the idlelib.pyshell part to 3.6 |
|
@ibanezz1999 Please explain your comment. This was merged months ago and any additional changes will be a new patch. |
No description provided.