Skip to content

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

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 26 additions & 14 deletions Lib/argparse.py
Original file line number Diff line number Diff line change
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
return copy.copy(items)


# ===============
Expand Down Expand Up @@ -619,12 +622,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 @@ -952,7 +960,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 @@ -978,7 +987,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 @@ -1000,8 +1010,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
Original file line number Diff line number Diff line change
@@ -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
14 changes: 9 additions & 5 deletions Lib/gettext.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,10 @@
# find this format documented anywhere.


import copy
import locale
import os
import re
import struct
import sys
from errno import ENOENT


__all__ = ['NullTranslations', 'GNUTranslations', 'Catalog',
Expand Down Expand Up @@ -342,7 +339,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 @@ -520,7 +519,9 @@ def translation(domain, localedir=None, languages=None,
if not mofiles:
if fallback:
return NullTranslations()
raise OSError(ENOENT, 'No translation file found for domain', domain)
from errno import ENOENT
raise FileNotFoundError(ENOENT,
'No translation file found for domain', domain)
# Avoid opening, reading, and parsing the .mo file after it's been done
# once.
result = None
Expand All @@ -533,6 +534,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
Original file line number Diff line number Diff line change
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
21 changes: 9 additions & 12 deletions Lib/os.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@

#'
import abc
import sys, errno
import sys
import stat as st

_names = sys.builtin_module_names
Expand Down Expand Up @@ -590,12 +590,10 @@ def _execvpe(file, args, env=None):
argrest = (args,)
env = environ

head, tail = path.split(file)
if head:
if path.dirname(file):
exec_func(file, *argrest)
return
last_exc = saved_exc = None
saved_tb = None
saved_exc = None
path_list = get_exec_path(env)
if name != 'nt':
file = fsencode(file)
Expand All @@ -604,16 +602,15 @@ def _execvpe(file, args, env=None):
fullname = path.join(dir, file)
try:
exec_func(fullname, *argrest)
except (FileNotFoundError, NotADirectoryError) as e:
last_exc = e
except OSError as e:
last_exc = e
tb = sys.exc_info()[2]
if (e.errno != errno.ENOENT and e.errno != errno.ENOTDIR
and saved_exc is None):
if saved_exc is None:
saved_exc = e
saved_tb = tb
if saved_exc:
raise saved_exc.with_traceback(saved_tb)
raise last_exc.with_traceback(tb)
if saved_exc is not None:
raise saved_exc
raise last_exc


def get_exec_path(env=None):
Expand Down
2 changes: 1 addition & 1 deletion Lib/pathlib.py
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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