Skip to content

Commit

Permalink
Support list append from command line and config file (#138)
Browse files Browse the repository at this point in the history
Added append to lists support both from command line and config file #85.
  • Loading branch information
mauvilsa authored May 24, 2022
1 parent 8506d79 commit 09ed681
Show file tree
Hide file tree
Showing 10 changed files with 222 additions and 37 deletions.
2 changes: 1 addition & 1 deletion .bumpversion.cfg
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[bumpversion]
current_version = 4.7.3
current_version = 4.8.0.dev1
commit = True
tag = True
tag_name = v{new_version}
Expand Down
2 changes: 1 addition & 1 deletion .sonarcloud.properties
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
sonar.sources=jsonargparse
sonar.projectVersion=4.7.3
sonar.projectVersion=4.8.0.dev1
sonar.python.version=3.6, 3.7, 3.8, 3.9, 3.10
9 changes: 9 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,15 @@ The semantic versioning only considers the public API as described in
paths are considered internals and can change in any moment.


v4.8.0 (2022-05-??)
-------------------

Added
^^^^^
- Support append to lists both from command line and config file `#85
<https://github.com/omni-us/jsonargparse/issues/85>`__.


v4.7.3 (2022-05-10)
-------------------

Expand Down
47 changes: 47 additions & 0 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -858,6 +858,53 @@ Some notes about this support are:
not validated.


.. _list-append:

List append
-----------

As detailed before, arguments with ``List`` type are supported. By default when
specifying an argument value, the previous value is replaced, and this also
holds for lists. Thus, a parse such as ``parser.parse_args(['--list=[1]',
'--list=[2, 3]'])`` would result in a final value of ``[2, 3]``. However, in
some cases it might be desided to append to the list instead of replacing. This
can be achieved by adding ``+`` as suffix to the argument key, for example:

.. testsetup:: append

from jsonargparse import ArgumentParser
from typing import List
parser = ArgumentParser()

.. doctest:: append

>>> parser.add_argument('--list', type=List[int]) # doctest: +IGNORE_RESULT
>>> parser.parse_args(['--list=[1]', '--list+=[2, 3]'])
Namespace(list=[1, 2, 3])
>>> parser.parse_args(['--list=[4]', '--list+=5'])
Namespace(list=[4, 5])

Append is also supported in config files. For instance the following two config
files would first assign a list and then append to this list:

.. code-block:: yaml
# config1.yaml
list:
- 1
.. code-block:: yaml
# config2.yaml
list+:
- 2
- 3
Appending works for any type for the list elements. List elements with class
type is also supported and the short notation for ``init_args`` when used (see
:ref:`sub-classes`), gets applied to the last element of the list.


.. _restricted-numbers:

Restricted numbers
Expand Down
2 changes: 1 addition & 1 deletion jsonargparse/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,4 +61,4 @@
__all__ += deprecated.__all__


__version__ = '4.7.3'
__version__ = '4.8.0.dev1'
15 changes: 8 additions & 7 deletions jsonargparse/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
_ActionPrintConfig,
_ActionConfigLoad,
_ActionLink,
_ActionHelpClassPath,
_is_branch_key,
_find_action,
_find_action_and_subcommand,
Expand Down Expand Up @@ -99,12 +98,12 @@ def add_argument(self, *args, enable_path:bool=False, **kwargs):
self.add_dataclass_arguments(theclass, nested_key, **kwargs)
return _find_action(parser, nested_key)
if ActionTypeHint.is_supported_typehint(kwargs['type']):
if 'action' in kwargs:
raise ValueError('Type hint as type does not allow providing an action.')
if ActionTypeHint.is_subclass_typehint(kwargs['type']):
dest = re.sub('^--', '', args[0])
super().add_argument(f'--{dest}.help', action=_ActionHelpClassPath(baseclass=kwargs['type'])) # type: ignore
kwargs['action'] = ActionTypeHint(typehint=kwargs.pop('type'), enable_path=enable_path)
args = ActionTypeHint.prepare_add_argument(
args=args,
kwargs=kwargs,
enable_path=enable_path,
container=super(),
)
action = super().add_argument(*args, **kwargs)
if isinstance(action, ActionConfigFile) and getattr(self, '_print_config', None) is not None:
self.add_argument(self._print_config, action=_ActionPrintConfig) # type: ignore
Expand Down Expand Up @@ -298,6 +297,8 @@ def _parse_common(

elif defaults:
cfg = self.merge_config(cfg, self.get_defaults(skip_check=True))
with load_value_context(self.parser_mode):
ActionTypeHint.apply_appends(self, cfg)
with _lenient_check_context():
ActionTypeHint.add_sub_defaults(self, cfg)

Expand Down
32 changes: 19 additions & 13 deletions jsonargparse/signatures.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from functools import wraps
from typing import Any, Callable, List, Optional, Set, Tuple, Type, Union

from .actions import _ActionConfigLoad, _ActionHelpClassPath
from .actions import _ActionConfigLoad
from .namespace import Namespace
from .typehints import ActionTypeHint, ClassType, is_optional, LazyInitBaseClass
from .typing import is_final_class
Expand Down Expand Up @@ -340,6 +340,8 @@ def _add_signature_parameter(
kwargs['required'] = True
is_subclass_typehint = False
is_final_class_typehint = is_final_class(annotation)
dest = (nested_key+'.' if nested_key else '') + name
args = [dest if is_required and as_positional else '--'+dest]
if annotation in {str, int, float, bool} or \
_issubclass(annotation, (str, int, float)) or \
is_final_class_typehint or \
Expand All @@ -348,32 +350,36 @@ def _add_signature_parameter(
elif annotation != inspect_empty:
try:
is_subclass_typehint = ActionTypeHint.is_subclass_typehint(annotation)
enable_path = is_subclass_typehint and sub_configs
kwargs['action'] = ActionTypeHint(typehint=annotation, enable_path=enable_path)
kwargs['type'] = annotation
sub_add_kwargs = None
if is_subclass_typehint:
prefix = name + '.init_args.'
subclass_skip = {s[len(prefix):] for s in skip if s.startswith(prefix)}
sub_add_kwargs = {'fail_untyped': fail_untyped, 'skip': subclass_skip}
args = ActionTypeHint.prepare_add_argument(
args=args,
kwargs=kwargs,
enable_path=is_subclass_typehint and sub_configs,
container=group,
sub_add_kwargs=sub_add_kwargs,
)
except ValueError as ex:
self.logger.debug(skip_message+str(ex)) # type: ignore
if 'type' in kwargs or 'action' in kwargs:
dest = (nested_key+'.' if nested_key else '') + name
if dest in added_args:
self.logger.debug(skip_message+'Argument already added.') # type: ignore
else:
opt_str = dest if is_required and as_positional else '--'+dest
sub_add_kwargs = {
'fail_untyped': fail_untyped,
'sub_configs': sub_configs,
'instantiate': instantiate,
}
if is_subclass_typehint:
help_action = group.add_argument(f'--{dest}.help', action=_ActionHelpClassPath(baseclass=annotation))
prefix = name + '.init_args.'
subclass_skip = {s[len(prefix):] for s in skip if s.startswith(prefix)}
help_action.sub_add_kwargs = {'fail_untyped': fail_untyped, 'skip': subclass_skip}
elif is_final_class_typehint:
if is_final_class_typehint:
kwargs.update(sub_add_kwargs)
action = group.add_argument(opt_str, **kwargs)
action = group.add_argument(*args, **kwargs)
action.sub_add_kwargs = sub_add_kwargs
if is_subclass_typehint and len(subclass_skip) > 0:
action.sub_add_kwargs['skip'] = subclass_skip # type: ignore
action.sub_add_kwargs['skip'] = subclass_skip
added_args.append(dest)
elif is_required and fail_untyped:
raise ValueError(f'Required parameter without a type for {obj} parameter "{name}".')
Expand Down
102 changes: 89 additions & 13 deletions jsonargparse/typehints.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
"""Action to support type hints."""

import copy
import inspect
import os
import re
Expand Down Expand Up @@ -32,7 +31,7 @@
Union,
)

from .actions import _find_action, _find_parent_action, _is_action_value_list
from .actions import _ActionHelpClassPath, _find_action, _find_parent_action, _is_action_value_list
from .loaders_dumpers import get_loader_exceptions, load_value
from .namespace import is_empty_namespace, Namespace
from .typing import is_final_class, registered_types
Expand Down Expand Up @@ -131,6 +130,7 @@ def __init__(
if 'metavar' not in kwargs:
kwargs['metavar'] = typehint_metavar(self._typehint)
super().__init__(**kwargs)
self._supports_append = self.supports_append(self._typehint)
self.normalize_default()


Expand All @@ -144,6 +144,21 @@ def normalize_default(self):
self.default = get_import_path(default)


@staticmethod
def prepare_add_argument(args, kwargs, enable_path, container, sub_add_kwargs=None):
if 'action' in kwargs:
raise ValueError('Providing both type and action allowed.')
typehint = kwargs.pop('type')
if ActionTypeHint.supports_append(typehint):
args = tuple(list(args)+[args[0]+'+'])
if ActionTypeHint.is_subclass_typehint(typehint):
help_action = container.add_argument(args[0]+'.help', action=_ActionHelpClassPath(baseclass=typehint))
if sub_add_kwargs:
help_action.sub_add_kwargs = sub_add_kwargs
kwargs['action'] = ActionTypeHint(typehint=typehint, enable_path=enable_path)
return args


@staticmethod
def is_supported_typehint(typehint, full=False):
"""Whether the given type hint is supported."""
Expand Down Expand Up @@ -246,6 +261,28 @@ def add_sub_defaults(parser, cfg):
parser._apply_actions(cfg)


@staticmethod
def supports_append(action):
typehint = typehint_from_action(action)
typehint_origin = get_typehint_origin(typehint)
return typehint and (
typehint_origin in sequence_origin_types or
(
typehint_origin == Union and
any(get_typehint_origin(x) in sequence_origin_types for x in typehint.__args__)
)
)

@staticmethod
def apply_appends(parser, cfg):
for key in [k for k in cfg.keys() if k.endswith('+')]:
action = _find_action(parser, key[:-1])
if ActionTypeHint.supports_append(action):
val = action._check_type(cfg[key], append=True, cfg=cfg)
cfg[key[:-1]] = val
cfg.pop(key)


def serialize(self, value, dump_kwargs=None):
sub_add_kwargs = getattr(self, 'sub_add_kwargs', {})
with dump_kwargs_context(dump_kwargs):
Expand Down Expand Up @@ -280,20 +317,27 @@ def __call__(self, *args, **kwargs):
cfg_dest['class_path'] = default['class_path']
except (KeyError, TypeError):
pass
if 'class_path' not in cfg_dest:
if not(
('class_path' in cfg_dest and not isinstance(cfg_dest, list)) or
(self._supports_append and cfg_dest and isinstance(cfg_dest, list) and 'class_path' in cfg_dest[-1])
):
raise ParserError(f'Found {opt_str} but not yet known to which class_path this corresponds.')
if not sub_opt.startswith('init_args.'):
sub_opt = 'init_args.' + sub_opt
if len(sub_opt.split('.', 2)) == 3:
val = NestedArg(key=sub_opt[len('init_args.'):], val=val)
sub_opt = 'init_args'
cfg_dest[sub_opt] = val
if isinstance(cfg_dest, list):
cfg_dest[-1][sub_opt] = val
else:
cfg_dest[sub_opt] = val
val = cfg_dest
val = self._check_type(val, cfg=cfg)
append = opt_str == f'--{self.dest}+'
val = self._check_type(val, append=append, cfg=cfg)
args[1].update(val, self.dest)


def _check_type(self, value, cfg=None):
def _check_type(self, value, append=False, cfg=None):
islist = _is_action_value_list(self)
if not islist:
value = [value]
Expand All @@ -305,23 +349,32 @@ def _check_type(self, value, cfg=None):
except get_loader_exceptions():
config_path = None
path_meta = val.pop('__path__', None) if isinstance(val, dict) else None
sub_add_kwargs = getattr(self, 'sub_add_kwargs', {})
prev_val = cfg.get(self.dest) if cfg else None
kwargs = {
'sub_add_kwargs': getattr(self, 'sub_add_kwargs', {}),
'prev_val': cfg.get(self.dest) if cfg else None,
'append': append,
}
try:
with change_to_path_dir(config_path):
val = adapt_typehints(val, self._typehint, prev_val=prev_val, sub_add_kwargs=sub_add_kwargs)
val = adapt_typehints(val, self._typehint, **kwargs)
except ValueError as ex:
val_is_int_float_or_none = isinstance(val, (int, float)) or val is None
if lenient_check.get():
value[num] = orig_val if val_is_int_float_or_none else val
continue
if val_is_int_float_or_none and config_path is None:
val = adapt_typehints(orig_val, self._typehint, prev_val=prev_val, sub_add_kwargs=sub_add_kwargs)
val = adapt_typehints(orig_val, self._typehint, **kwargs)
else:
if self._enable_path and config_path is None and isinstance(orig_val, str):
msg = f'\n- Expected a config path but "{orig_val}" either not accessible or invalid.\n- '
raise type(ex)(msg+str(ex)) from ex
raise ex

if not append and self._supports_append:
prev_val = kwargs.get('prev_val')
if isinstance(prev_val, list) and not_append_diff(prev_val, val) and get_typehint_origin(self._typehint) == Union:
warnings.warn(f'Replacing list value "{prev_val}" with "{val}". To append to a list use "{self.dest}+".')

if path_meta is not None:
val['__path__'] = path_meta
if isinstance(val, (Namespace, dict)) and config_path is not None:
Expand Down Expand Up @@ -388,15 +441,16 @@ def completer(self, prefix, **kwargs):
return argcomplete_warn_redraw_prompt(prefix, msg)


def adapt_typehints(val, typehint, serialize=False, instantiate_classes=False, prev_val=None, sub_add_kwargs=None):
def adapt_typehints(val, typehint, serialize=False, instantiate_classes=False, prev_val=None, append=False, sub_add_kwargs=None):

adapt_kwargs = {
'serialize': serialize,
'instantiate_classes': instantiate_classes,
'prev_val': prev_val,
'append': append,
'sub_add_kwargs': sub_add_kwargs or {},
}
subtypehints = getattr(typehint, '__args__', None)
subtypehints = get_typehint_subtypes(typehint, append=append)
typehint_origin = get_typehint_origin(typehint) or typehint

# Any
Expand Down Expand Up @@ -490,6 +544,14 @@ def adapt_typehints(val, typehint, serialize=False, instantiate_classes=False, p

# List, Iterable or Sequence
elif typehint_origin in sequence_origin_types:
if append and prev_val is not None:
if not isinstance(prev_val, list):
try:
prev_val = [adapt_typehints(prev_val, subtypehints[0], **adapt_kwargs)]
except Exception:
pass
if isinstance(prev_val, list):
val = prev_val + (val if isinstance(val, list) else [val])
if not isinstance(val, list):
raise ValueError(f'Expected a List but got "{val}"')
if subtypehints is not None:
Expand All @@ -506,7 +568,7 @@ def adapt_typehints(val, typehint, serialize=False, instantiate_classes=False, p
val = {cast(k): v for k, v in val.items()}
for k, v in val.items():
if "linked_targets" in adapt_kwargs["sub_add_kwargs"]:
kwargs = copy.deepcopy(adapt_kwargs)
kwargs = deepcopy(adapt_kwargs)
sub_add_kwargs = kwargs["sub_add_kwargs"]
sub_add_kwargs["linked_targets"] = {t[len(k + "."):] for t in sub_add_kwargs["linked_targets"]
if t.startswith(k + ".")}
Expand Down Expand Up @@ -698,6 +760,20 @@ def adapt_class_type(val_class, init_args, serialize, instantiate_classes, sub_a
return init_args


def not_append_diff(val1, val2):
if isinstance(val1, list) and isinstance(val2, list):
val1 = [x.get('class_path') if is_class_object(x) else x for x in val1]
val2 = [x.get('class_path') if is_class_object(x) else x for x in val2]
return val1 != val2


def get_typehint_subtypes(typehint, append):
subtypes = getattr(typehint, '__args__', None)
if append and subtypes:
subtypes = sorted(subtypes, key=lambda x: get_typehint_origin(x) not in sequence_origin_types)
return subtypes


def get_typehint_origin(typehint):
if not hasattr(typehint, '__origin__') and get_import_path(typehint.__class__) == 'types.UnionType':
return Union
Expand Down
Loading

0 comments on commit 09ed681

Please sign in to comment.