Skip to content

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

Merged
rhettinger merged 1 commit into
python:masterfrom
rhettinger:improve_argparse
Nov 22, 2019
Merged

Defer import of shutil which is only needed for help and usage#17334
rhettinger merged 1 commit into
python:masterfrom
rhettinger:improve_argparse

Conversation

@rhettinger

@rhettinger rhettinger commented Nov 22, 2019

Copy link
Copy Markdown
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
Comment thread Lib/argparse.py

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

@corona10 corona10 Nov 22, 2019

Copy link
Copy Markdown
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?

@rhettinger rhettinger Nov 22, 2019

Copy link
Copy Markdown
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.

@corona10 corona10 Nov 22, 2019

Copy link
Copy Markdown
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

blueyed commented Jul 6, 2020

Copy link
Copy Markdown
Contributor

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