Skip to content

gh-96859: [argparse] Avoid copying lists on every append and extend action #124745

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

gh-96859: [argparse] Avoid copying lists on every append and extend action #124745

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
25 changes: 19 additions & 6 deletions Lib/argparse.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ def _get_args(self):
def _copy_items(items):
if items is None:
return []
# The copy module is used only in the 'append' and 'append_const'
# The copy module is used only in the 'append', 'append_const', and 'extend'
# 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:
Expand Down Expand Up @@ -974,7 +974,13 @@ def __init__(self,
deprecated=deprecated)


class _AppendAction(Action):
class _CloningAction(Action):

def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)


class _AppendAction(_CloningAction):

def __init__(self,
option_strings,
Expand Down Expand Up @@ -1009,12 +1015,11 @@ def __init__(self,

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


class _AppendConstAction(Action):
class _AppendConstAction(_CloningAction):

def __init__(self,
option_strings,
Expand All @@ -1038,7 +1043,6 @@ def __init__(self,

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

Expand Down Expand Up @@ -1230,7 +1234,6 @@ def __call__(self, parser, namespace, values, option_string=None):
class _ExtendAction(_AppendAction):
def __call__(self, parser, namespace, values, option_string=None):
items = getattr(namespace, self.dest, None)
items = _copy_items(items)
items.extend(values)
setattr(namespace, self.dest, items)

Expand Down Expand Up @@ -1941,6 +1944,7 @@ def _parse_known_args(self, arg_strings, namespace):
# converts arg strings to the appropriate and then takes the action
seen_actions = set()
seen_non_default_actions = set()
cloned = set()
warned = set()

def take_action(action, argument_strings, option_string=None):
Expand All @@ -1960,6 +1964,15 @@ def take_action(action, argument_strings, option_string=None):
# take the action if we didn't receive a SUPPRESS value
# (e.g. from a default)
if argument_values is not SUPPRESS:
# copy the destination attribute in case the action modifies it
# in-place.
if isinstance(action, _CloningAction) and action not in cloned:
# only copy on the first action call.
cloned.add(action)

items = getattr(namespace, action.dest, None)
setattr(namespace, action.dest, _copy_items(items))

action(self, namespace, argument_values, option_string)

# function to convert arg_strings into an optional action
Expand Down
Loading