Skip to content
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.

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

bpo-30152: Reduce the number of imports for argparse. #1269

Merged
Merged
40 changes: 26 additions & 14 deletions Lib/argparse.py
Expand Up @@ -84,15 +84,12 @@


import collections as _collections
import copy as _copy
import os as _os
import re as _re
import sys as _sys
import textwrap as _textwrap

from gettext import gettext as _, ngettext


SUPPRESS = '==SUPPRESS=='

OPTIONAL = '?'
Expand Down Expand Up @@ -137,10 +134,16 @@ def _get_args(self):
return []


def _ensure_value(namespace, name, value):
if getattr(namespace, name, None) is None:
setattr(namespace, name, value)
return getattr(namespace, name)
def _copy_items(items):
if items is None:
return []
# The copy module is used only in the 'append' and 'append_const'
# actions, and it is needed only when the default value isn't a list.
# Delay its import for speeding up the common case.
if type(items) is list:
Copy link
Member

Choose a reason for hiding this comment

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

Why not isinstance(items, list)?

Copy link
Member Author

Choose a reason for hiding this comment

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

list subclass can override __copy__ or __reduce__.

return items[:]
Copy link
Member

Choose a reason for hiding this comment

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

Why not items.copy()?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is shorter.

Copy link
Contributor

Choose a reason for hiding this comment

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

And it's faster.

import copy
items = copy.copy(items)
Copy link

Choose a reason for hiding this comment

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

shouldn't that be 'return copy.copy(items)' ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, right!



# ===============
Expand Down Expand Up @@ -617,12 +620,17 @@ def _iter_indented_subactions(self, action):

def _split_lines(self, text, width):
text = self._whitespace_matcher.sub(' ', text).strip()
return _textwrap.wrap(text, width)
# The textwrap module is used only for formatting help.
# Delay its import for speeding up the common usage of argparse.
import textwrap
return textwrap.wrap(text, width)

def _fill_text(self, text, width, indent):
text = self._whitespace_matcher.sub(' ', text).strip()
return _textwrap.fill(text, width, initial_indent=indent,
subsequent_indent=indent)
import textwrap
return textwrap.fill(text, width,
initial_indent=indent,
subsequent_indent=indent)

def _get_help_string(self, action):
return action.help
Expand Down Expand Up @@ -950,7 +958,8 @@ def __init__(self,
metavar=metavar)

def __call__(self, parser, namespace, values, option_string=None):
items = _copy.copy(_ensure_value(namespace, self.dest, []))
items = getattr(namespace, self.dest, None)
items = _copy_items(items)
items.append(values)
setattr(namespace, self.dest, items)

Expand All @@ -976,7 +985,8 @@ def __init__(self,
metavar=metavar)

def __call__(self, parser, namespace, values, option_string=None):
items = _copy.copy(_ensure_value(namespace, self.dest, []))
items = getattr(namespace, self.dest, None)
items = _copy_items(items)
items.append(self.const)
setattr(namespace, self.dest, items)

Expand All @@ -998,8 +1008,10 @@ def __init__(self,
help=help)

def __call__(self, parser, namespace, values, option_string=None):
new_count = _ensure_value(namespace, self.dest, 0) + 1
setattr(namespace, self.dest, new_count)
count = getattr(namespace, self.dest, None)
if count is None:
count = 0

Choose a reason for hiding this comment

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

Why not just use getattr(namespace, self.dest, 0) and not have the if test?

Copy link
Member

Choose a reason for hiding this comment

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

If think the current code is equivalent to getattr(namespace, self.dest) or 0, not getattr(namespace, self.dest, 0). The first one can never have None for count, the second does (if namespace has an attribute with the name in self.dest and the value None).

setattr(namespace, self.dest, count + 1)


class _HelpAction(Action):
Expand Down
11 changes: 4 additions & 7 deletions Lib/enum.py
@@ -1,7 +1,5 @@
import sys
from types import MappingProxyType, DynamicClassAttribute
from functools import reduce
from operator import or_ as _or_

# try _collections first to reduce startup cost
try:
Expand Down Expand Up @@ -744,11 +742,10 @@ def __xor__(self, other):

def __invert__(self):
members, uncovered = _decompose(self.__class__, self._value_)
inverted_members = [
m for m in self.__class__
if m not in members and not m._value_ & self._value_
]
inverted = reduce(_or_, inverted_members, self.__class__(0))
inverted = self.__class__(0)
for m in self.__class__:
if m not in members and not (m._value_ & self._value_):
inverted = inverted | m
Copy link
Member

Choose a reason for hiding this comment

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

I don't see how this change is related to removing imports.

Copy link
Member Author

Choose a reason for hiding this comment

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

functools.reduce no longer imported.

return self.__class__(inverted)


Expand Down
9 changes: 6 additions & 3 deletions Lib/gettext.py
Expand Up @@ -46,11 +46,9 @@
# find this format documented anywhere.


import copy
import locale
import os
import re
import struct
import sys
from errno import ENOENT
Copy link
Member

Choose a reason for hiding this comment

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

Would it be worth it to defer errno import as well? It's only used in one case, to raise an error when something goes wrong.

Copy link
Member Author

Choose a reason for hiding this comment

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

It is imported by os (where it is only used in one case too).


Expand Down Expand Up @@ -337,7 +335,9 @@ def _get_versions(self, version):

def _parse(self, fp):
"""Override this method to support alternative .mo formats."""
unpack = struct.unpack
# Delay struct import for speeding up gettext import when .mo files
# are not used.
from struct import unpack
filename = getattr(fp, 'name', '')
# Parse the .mo file header, which consists of 5 little endian 32
# bit words.
Expand Down Expand Up @@ -528,6 +528,9 @@ def translation(domain, localedir=None, languages=None,
# Copy the translation object to allow setting fallbacks and
# output charset. All other instance data is shared with the
# cached object.
# Delay copy import for speeding up gettext import when .mo files
# are not used.
import copy
t = copy.copy(t)
if codeset:
t.set_output_charset(codeset)
Expand Down
6 changes: 3 additions & 3 deletions Lib/locale.py
Expand Up @@ -14,10 +14,9 @@
import encodings
import encodings.aliases
import re
import collections.abc
import _collections_abc
from builtins import str as _builtin_str
import functools
import warnings

# Try importing the _locale module.
#
Expand Down Expand Up @@ -215,7 +214,7 @@ def format_string(f, val, grouping=False, monetary=False):
percents = list(_percent_re.finditer(f))
new_f = _percent_re.sub('%s', f)

if isinstance(val, collections.abc.Mapping):
if isinstance(val, _collections_abc.Mapping):
new_val = []
for perc in percents:
if perc.group()[-1]=='%':
Expand Down Expand Up @@ -244,6 +243,7 @@ def format_string(f, val, grouping=False, monetary=False):

def format(percent, value, grouping=False, monetary=False, *additional):
"""Deprecated, use format_string instead."""
import warnings
warnings.warn(
"This method will be removed in a future version of Python. "
"Use 'locale.format_string()' instead.",
Expand Down
2 changes: 1 addition & 1 deletion Lib/pathlib.py
Expand Up @@ -6,7 +6,7 @@
import posixpath
import re
import sys
from collections.abc import Sequence
from _collections_abc import Sequence
from errno import EINVAL, ENOENT, ENOTDIR
from operator import attrgetter
from stat import S_ISDIR, S_ISLNK, S_ISREG, S_ISSOCK, S_ISBLK, S_ISCHR, S_ISFIFO
Expand Down
8 changes: 4 additions & 4 deletions Lib/types.py
Expand Up @@ -172,9 +172,6 @@ def deleter(self, fdel):
return result


import functools as _functools
import collections.abc as _collections_abc

class _GeneratorWrapper:
# TODO: Implement this in C.
def __init__(self, gen):
Expand Down Expand Up @@ -247,7 +244,10 @@ def coroutine(func):
# return generator-like objects (for instance generators
# compiled with Cython).

@_functools.wraps(func)
# Delay functools and _collections_abc import for speeding up types import.
import functools
import _collections_abc
@functools.wraps(func)
def wrapped(*args, **kwargs):
coro = func(*args, **kwargs)
if (coro.__class__ is CoroutineType or
Expand Down
6 changes: 3 additions & 3 deletions Lib/weakref.py
Expand Up @@ -21,7 +21,7 @@

from _weakrefset import WeakSet, _IterationGuard

import collections.abc # Import after _weakref to avoid circular import.
import _collections_abc # Import after _weakref to avoid circular import.
import sys
import itertools

Expand Down Expand Up @@ -87,7 +87,7 @@ def __ne__(self, other):
__hash__ = ref.__hash__


class WeakValueDictionary(collections.abc.MutableMapping):
class WeakValueDictionary(_collections_abc.MutableMapping):
"""Mapping class that references values weakly.

Entries in the dictionary will be discarded when no strong
Expand Down Expand Up @@ -340,7 +340,7 @@ def __init__(self, ob, callback, key):
super().__init__(ob, callback)


class WeakKeyDictionary(collections.abc.MutableMapping):
class WeakKeyDictionary(_collections_abc.MutableMapping):
""" Mapping class that references keys weakly.

Entries in the dictionary will be discarded when there is no
Expand Down