Skip to content

Defer import of shutil which is only needed for help and usage #17334

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Nov 22, 2019
Merged

Defer import of shutil which is only needed for help and usage #17334

merged 1 commit into from
Nov 22, 2019

Conversation

rhettinger
Copy link
Contributor

Minor improvement that saves some downstream imports as well:

   shutil <- {os sys stat fnmatch collections errno}
   fnmatch <- {os posixpath re functools} 
   posixpath <- {os sys stat genericpatch}

@rhettinger rhettinger added type-feature A feature request or enhancement skip news labels Nov 22, 2019
corona10
corona10 approved these changes Nov 22, 2019
View reviewed changes
Lib/argparse.py
@@ -264,7 +264,7 @@ def add_argument(self, action):
invocations.append(get_invocation(subaction))

# update the maximum item length
invocation_length = max([len(s) for s in invocations])
invocation_length = max(map(len, invocations))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rhettinger Looks awesome, is there any performance difference?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was either this or a switch to a generation expression to save memory. Depending on the number of invocations, the map version has fewer global lookups and fewer turns around the ceval loop. The win is likely very minor though. Mostly, it reads a little better than before.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation :)

@rhettinger rhettinger merged commit b4e5eea into python:master Nov 22, 2019
@rhettinger rhettinger deleted the improve_argparse branch November 22, 2019 06:51
jacobneiltaylor pushed a commit to jacobneiltaylor/cpython that referenced this pull request Dec 5, 2019
@rhettinger
Defer import of shutil which only needed for help and usage (pythonGH…
b7ac390 
…-17334)
shihai1991 pushed a commit to shihai1991/cpython that referenced this pull request Jan 31, 2020
@rhettinger @shihai1991
Defer import of shutil which only needed for help and usage (pythonGH…
a0b6ede 
…-17334)
@blueyed
Copy link
Contributor

blueyed commented Jul 6, 2020

Note that it is still used via add_argument already (https://bugs.python.org/issue9348#msg373111).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@corona10 corona10 corona10 approved these changes

Assignees
No one assigned
Labels
skip issue skip news type-feature A feature request or enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@rhettinger @blueyed @corona10 @the-knights-who-say-ni @bedevere-bot