diff --git a/CHANGELOG.rst b/CHANGELOG.rst index be7f89a4..55adb917 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -31,6 +31,15 @@ Added - New ``docstrings`` and ``typeshed`` extras, which install individually the respective optional dependencies of the ``signatures`` extra (`#988 `__). +- Support for ``*args`` in signatures, added as a list argument named like the + parameter, or a positional with ``as_positional=True``. The AST resolver now + also follows a ``*args`` forwarded without ``**kwargs`` or to a callable that + has its own ``*args`` (`#986 + `__). +- New ``instantiate`` parameter of ``add_function_arguments`` and + ``add_method_arguments``, and links applied on instantiate can target the + parameters of these groups (`#986 + `__). Fixed ^^^^^ @@ -82,6 +91,17 @@ Fixed - ``--print_config`` failing when a required subcommand is not given, unlike other required arguments (`#984 `__). +- ``instantiate``, ``auto_cli`` and ``from_config`` failing when a signature + has positional-only parameters (`#986 + `__). +- AST resolver failing for a method whose ``self`` is positional-only (`#986 + `__). +- A typed positional with ``nargs="*"`` set in a config file being reset to + empty when no values for it are given in the command line (`#986 + `__). +- A positional-only parameter that has a default being required, even though + the call can omit it (`#986 + `__). Changed ^^^^^^^ @@ -141,6 +161,15 @@ Changed - A ``types.ModuleType`` value given as a module object is now normalized to its import path, instead of only being normalized when given as a default (`#983 `__). +- ``instantiate`` now replaces a group added by ``add_function_arguments`` or + ``add_method_arguments`` by a ``functools.partial`` with the arguments bound, + or by an ``operator.methodcaller`` for a method that is called with an + instance, instead of keeping the parsed namespace (`#986 + `__). +- ``add_subcommands`` now raises an error when the parser already has a + positional that accepts a variable number of values, since argparse parses + this combination incorrectly, e.g. taking the subcommand name as one of the + values (`#986 `__). Removed ^^^^^^^ @@ -154,6 +183,19 @@ Removed `__). +v4.53.0 (unreleased) +-------------------- + +Deprecated +^^^^^^^^^^ +- Groups added by ``add_function_arguments`` or ``add_method_arguments`` with a + ``nested_key`` are kept as parsed by ``instantiate``. From v5.0.0 they will be + replaced by a ``functools.partial`` with the arguments bound, or by an + ``operator.methodcaller`` for a method that is called with an instance. Warns + only with ``JSONARGPARSE_DEPRECATION_WARNINGS=all`` (`#985 + `__). + + v4.52.0 (2026-09-01) -------------------- diff --git a/DOCUMENTATION.rst b/DOCUMENTATION.rst index 4e78dd66..da13598d 100644 --- a/DOCUMENTATION.rst +++ b/DOCUMENTATION.rst @@ -1561,8 +1561,8 @@ and the method run, as follows: parser.add_method_arguments(MyClass, "mymethod", "myclass.method") cfg = parser.parse_args() - myclass = MyClass(**cfg.myclass.init.as_dict()) - myclass.mymethod(**cfg.myclass.method.as_dict()) + init = parser.instantiate(cfg) + init.myclass.method(init.myclass.init) The :meth:`add_class_arguments <.ArgumentParser.add_class_arguments>` call adds @@ -1574,11 +1574,13 @@ the :meth:`add_method_arguments <.ArgumentParser.add_method_arguments>` call adds ``myclass.method.bar`` as a required float and ``myclass.method.baz`` as an optional boolean with default false. -Several classes added with :meth:`add_class_arguments -<.ArgumentParser.add_class_arguments>` are instantiated at once with -:meth:`instantiate <.ArgumentParser.instantiate>`. In the example above, ``cfg = -parser.instantiate(cfg)`` makes ``cfg.myclass.init`` an instance of ``MyClass``, -built from the parsed arguments. +All the groups added by these methods are instantiated at once with +:meth:`instantiate <.ArgumentParser.instantiate>`. In the example above, +``init.myclass.init`` is an instance of ``MyClass`` built from the parsed +arguments, and ``init.myclass.method`` is an :func:`operator.methodcaller` with +the arguments of ``mymethod`` bound, which is called with an instance. A function, +static or class method group becomes a :func:`functools.partial` instead. Give +``instantiate=False`` to keep a group as parsed. All values can be given in a single config file (see :ref:`configuration-files`). For convenience, the values of each argument group @@ -1605,13 +1607,22 @@ A wide range of type hints is supported for signature parameters, see - ``fail_untyped`` decides which parameters without a type annotation raise an exception instead: the required ones with the default ``True``, all of them - with ``"all"``, and none with ``False``. Positional-only parameters are always - required. Use ``"all"`` only for code you own, since one untyped parameter of - a dependency would make its signature impossible to add. + with ``"all"``, and none with ``False``. Use ``"all"`` only for code you own, + since one untyped parameter of a dependency would make its signature + impossible to add. - Parameters whose name starts with ``_`` are considered internal and skipped, unless they are required. +- A ``*args`` is added as a list argument with its name, e.g. ``*files: str`` as + ``files`` of type ``list[str]``. With ``as_positional=True`` it is a + positional that takes zero or more values, which argparse is unable to combine + with subcommands, so ``as_positional=False`` is required then. When calling, + positional-only parameters, and when ``*args`` has values also the ones before + it, are given positionally. Binding such values after positionals given on + call, e.g. for a ``Callable`` that returns a class, requires Python 3.14 or + later. + - The ``skip`` parameter excludes arguments, e.g. ``parser.add_method_arguments(MyClass, 'mymethod', skip={'baz'})``. In a subclass spec, a skipped parameter can still be given in ``dict_kwargs``, see @@ -2017,6 +2028,11 @@ needed because the parser does not know which call will happen at runtime, and including them would make :meth:`instantiate <.ArgumentParser.instantiate>` fail with unexpected keyword arguments. +A ``*args`` forwarded to calls is replaced by what the calls take positionally, +including their own ``*args``, when all the calls agree on it. A ``*args`` that +the code uses itself, or whose use can't be resolved, is added as a list +argument, and one that is not used is left out. + .. note:: The resolvers log failures and unsupported cases. To see these logs, set the diff --git a/jsonargparse/_cli.py b/jsonargparse/_cli.py index cfd45f91..4c4c2214 100644 --- a/jsonargparse/_cli.py +++ b/jsonargparse/_cli.py @@ -6,6 +6,7 @@ from ._actions import ActionConfigFile, _ActionPrintConfig, remove_actions from ._core import ArgumentParser +from ._instantiation import bind_call, get_call_arguments from ._namespace import Namespace, dict_to_namespace from ._optionals import get_doc_short_description from ._signatures import FailUntyped @@ -90,7 +91,7 @@ def auto_cli( parser.set_defaults(set_defaults) cfg = parser.parse_args(args) init = parser.instantiate(cfg) - return _run_component(components, init) + return _run_component(components, init, parser) elif isinstance(components, list): components = {c.__name__: c for c in components} @@ -110,7 +111,9 @@ def auto_cli( else: break component = components_ns[subcommand] - return _run_component(component, init.get(subcommand)) + for name in subcommand.split("."): + parser = parser._subcommands_action._name_parser_map[name] # type: ignore[union-attr] + return _run_component(component, init.get(subcommand), parser) def auto_parser(*args, **kwargs) -> ArgumentParser: @@ -200,17 +203,23 @@ def _add_component_to_parser( return added_args -def _run_component(component, cfg): +def _get_call_values(cfg: Namespace) -> dict: + return dict(cfg.items(branches=True, nested=False)) + + +def _run_component(component, cfg, parser): cfg.pop("config", None) subcommand = cfg.pop("subcommand") if inspect.isclass(component) and subcommand: subcommand_cfg = cfg.pop(subcommand, {}) subcommand_cfg.pop("config", None) - component_obj = component(**cfg) + component_obj = bind_call(component, parser._call_layouts[None], _get_call_values(cfg))() if isinstance(getattr(component, subcommand), property): return getattr(component_obj, subcommand) component = getattr(component_obj, subcommand) + parser = parser._subcommands_action._name_parser_map[subcommand] cfg = subcommand_cfg + args, kwargs = get_call_arguments(parser._call_layouts[None], _get_call_values(cfg), component) if inspect.iscoroutinefunction(component): - return __import__("asyncio").run(component(**cfg)) - return component(**cfg) + return __import__("asyncio").run(component(*args, **kwargs)) + return component(*args, **kwargs) diff --git a/jsonargparse/_core.py b/jsonargparse/_core.py index 0e2204b7..5c938513 100644 --- a/jsonargparse/_core.py +++ b/jsonargparse/_core.py @@ -139,6 +139,7 @@ def __init__(self, *args, **kwargs) -> None: """Initializer for ActionsContainer instance.""" super().__init__(*args, **kwargs) self._accepted_kwargs = {} + self._call_layouts = {} self.register("type", None, identity) self.register("action", "parsers", ActionSubCommands) self.register("action", "config", ActionConfigFile) @@ -807,6 +808,13 @@ def add_subcommands(self, required: bool = True, dest: str = "subcommand", **kwa **kwargs: All options that `argparse.ArgumentParser.add_subparsers `_ accepts. """ + for action in self._actions: + if not action.option_strings and action.nargs in {"*", "+", argparse.REMAINDER}: + raise ValueError( + f"Positional '{action.dest}' accepts a variable number of values, which argparse is unable to " + "combine with subcommands. It must be an optional argument, in auto_cli by setting " + "as_positional=False." + ) if "description" not in kwargs: kwargs["description"] = "For more details of each subcommand, add it as an argument followed by --help." default_config_files = self.default_config_files diff --git a/jsonargparse/_from_config.py b/jsonargparse/_from_config.py index 632f95eb..8ff8e323 100644 --- a/jsonargparse/_from_config.py +++ b/jsonargparse/_from_config.py @@ -6,6 +6,7 @@ from ._common import parser_context from ._core import ArgumentParser +from ._instantiation import CallLayout, bind_call from ._loaders_dumpers import get_loader_exceptions, load_value from ._optionals import _get_config_read_mode from ._paths import path_dir_context @@ -57,12 +58,14 @@ def from_config(cls: type[T], config: str | PathLike | dict) -> T: Args: config: Path to a config file or a dict with config values. """ - kwargs, cls = _parse_class_kwargs_from_config(cls, config, **cls.__from_config_parser_kwargs__) # type: ignore[attr-defined] - return cls(**kwargs) + kwargs, cls, call_layout = _parse_class_kwargs_from_config(cls, config, **cls.__from_config_parser_kwargs__) # type: ignore[attr-defined] + return bind_call(cls, call_layout, kwargs)() -def _parse_class_kwargs_from_config(cls: type[T], config: str | PathLike | dict, **kwargs) -> tuple[dict, type[T]]: - """Parse the init kwargs for ``cls`` from a config file or dict.""" +def _parse_class_kwargs_from_config( + cls: type[T], config: str | PathLike | dict, **kwargs +) -> tuple[dict, type[T], CallLayout]: + """Parse the init kwargs for ``cls`` from a config file or dict, and how they are given in the call.""" parser = ArgumentParser(exit_on_error=False, **kwargs) cfg_path = None if not isinstance(config, dict): @@ -96,7 +99,7 @@ def _parse_class_kwargs_from_config(cls: type[T], config: str | PathLike | dict, clear_required(parser, required) with load_config_path_context(cfg_path), path_dir_context(cfg_path): cfg = parser.parse_object(config, defaults=False) - return parser.instantiate(cfg).as_dict(), cls + return parser.instantiate(cfg).as_dict(), cls, parser._call_layouts[None] def _override_init_defaults(cls: type[T], parser_kwargs: dict) -> None: @@ -107,7 +110,7 @@ def _override_init_defaults(cls: type[T], parser_kwargs: dict) -> None: if not (isinstance(config, (str, PathLike)) and Path(config).is_file()): return - defaults, cls = _parse_class_kwargs_from_config(cls, config, **parser_kwargs) + defaults, cls, _ = _parse_class_kwargs_from_config(cls, config, **parser_kwargs) _override_init_defaults_this_class(cls, defaults) _override_init_defaults_parent_classes(cls, defaults) diff --git a/jsonargparse/_instantiation.py b/jsonargparse/_instantiation.py index 5208a453..786a7c0a 100644 --- a/jsonargparse/_instantiation.py +++ b/jsonargparse/_instantiation.py @@ -1,11 +1,86 @@ +import dataclasses +import functools import inspect -from typing import Protocol +import sys +from collections.abc import Callable, Mapping +from functools import partial +from typing import Any, Protocol from ._common import ClassType, applied_instantiation_links, get_parsing_setting, is_subclass, parser_context from ._namespace import Namespace, get_value_and_parent, split_key __all__ = ["add_instantiator"] +kinds = inspect._ParameterKind + + +@dataclasses.dataclass(frozen=True) +class CallLayout: + """How the values of the parameters of a signature are given in a call.""" + + # parameters that can be given positionally, in order, i.e. positional-only ones and the ones + # before a *args + positional: tuple[str, ...] = () + # number of leading positional ones that are always given positionally, i.e. up to the last + # positional-only one. The rest are only given positionally when *args is not empty. + always_positional: int = 0 + var_positional: str | None = None + # defaults of the positional ones, used when one was not given, e.g. because it was skipped + defaults: Mapping[str, Any] = dataclasses.field(default_factory=dict) + # number of leading positionals that are not bound, but given when calling, e.g. by the + # caller of a callable that returns a class + open_positionals: int = 0 + + +def get_call_layout(params: list, open_positionals: int = 0) -> CallLayout: + """Gets the call layout for a list of resolved parameters.""" + var_idx = next((n for n, p in enumerate(params) if p.kind == kinds.VAR_POSITIONAL), None) + positional_only_idxs = [n for n, p in enumerate(params) if p.kind == kinds.POSITIONAL_ONLY] + end = var_idx if var_idx is not None else (positional_only_idxs[-1] + 1 if positional_only_idxs else 0) + positional = [p for p in params[:end] if p.kind in {kinds.POSITIONAL_ONLY, kinds.POSITIONAL_OR_KEYWORD}] + always = [n for n, p in enumerate(positional) if p.kind == kinds.POSITIONAL_ONLY] + return CallLayout( + positional=tuple(p.name for p in positional), + always_positional=always[-1] + 1 if always else 0, + var_positional=None if var_idx is None else params[var_idx].name, + defaults={p.name: p.default for p in positional if p.default is not inspect._empty}, + open_positionals=open_positionals, + ) + + +def get_call_arguments(layout: CallLayout, values: Mapping[str, Any], component: Any) -> tuple[list, dict]: + """Splits values into the positional and keyword arguments with which to call a component.""" + kwargs = dict(values) + var_positional = list(kwargs.pop(layout.var_positional, ())) if layout.var_positional else [] + num_positional = len(layout.positional) if var_positional else layout.always_positional + args = [] + for num, name in enumerate(layout.positional[:num_positional]): + if name in kwargs: + args.append(kwargs.pop(name)) + elif name in layout.defaults: + args.append(layout.defaults[name]) + else: + following = layout.var_positional if var_positional else layout.positional[num_positional - 1] + raise ValueError( + f'Calling {component} requires a value for parameter "{name}", since it precedes "{following}" ' + "which is given positionally." + ) + return args + var_positional, kwargs + + +def bind_call(func: Callable, layout: CallLayout, values: Mapping[str, Any], component: Any = None) -> Callable: + """Binds values to func according to a call layout, returning a callable that makes the call.""" + args, kwargs = get_call_arguments(layout, values, component or func) + if layout.open_positionals and args: + # the open positionals are given on call, before the bound ones + if sys.version_info < (3, 14): + raise ValueError( + f"Binding values to positionals of {component or func} that follow {layout.open_positionals} " + "positionals given on call is only supported in Python 3.14 or later." + ) + args = [functools.Placeholder] * layout.open_positionals + args + return partial(func, *args, **kwargs) + class InstantiatorCallable(Protocol): def __call__(self, class_type: type[ClassType], *args, **kwargs) -> ClassType: @@ -32,8 +107,8 @@ def instantiate( - **Class/subclass type arguments** (``add_argument`` with a class type or ``add_class_arguments``/``add_subclass_arguments``): An object with ``class_path`` and optionally ``init_args`` is replaced by an instance - of the referenced class, created by calling - ``class_type(**init_args)``. For the case of classes with disabled + of the referenced class, created by calling the class with the + ``init_args``. For the case of classes with disabled subclasses, the namespace can have directly the init args without the ``class_path`` + ``init_args`` wrapper. @@ -45,6 +120,12 @@ def instantiate( call arguments are provided yet — a :func:`functools.partial` bound to the given ``init_args``. + - **Function and method groups** (``add_function_arguments`` or + ``add_method_arguments`` with a ``nested_key``): Replaced by a + :func:`functools.partial` with the arguments bound, or by an + :func:`operator.methodcaller` for a method that is called with an + instance. + - **Instantiation order**: Components are processed in the order determined by argument links applied on instantiation. @@ -52,7 +133,7 @@ def instantiate( namespace: The configuration object to use. Must have been produced by one of the ``parse_*`` methods and not modified in a way that breaks the structure expected by the parser. - instantiate_groups: Whether class groups should be instantiated. + instantiate_groups: Whether class, function and method groups should be instantiated. Returns: A new configuration object where every registered signature diff --git a/jsonargparse/_parameter_resolvers.py b/jsonargparse/_parameter_resolvers.py index 25dfcf88..46a54151 100644 --- a/jsonargparse/_parameter_resolvers.py +++ b/jsonargparse/_parameter_resolvers.py @@ -459,16 +459,36 @@ def is_param_subclass_instance_default(param: ParamData) -> bool: ) -def split_args_and_kwargs(params: ParamList) -> tuple[ParamList, ParamList]: - args = [p for p in params if p.kind == kinds.POSITIONAL_ONLY] - kwargs = [p for p in params if p.kind in {kinds.KEYWORD_ONLY, kinds.POSITIONAL_OR_KEYWORD, kinds.VAR_KEYWORD}] +kwargs_kinds = {kinds.KEYWORD_ONLY, kinds.POSITIONAL_OR_KEYWORD, kinds.VAR_KEYWORD} + + +def split_args_and_kwargs(params: ParamList, forwards_kwargs: bool = True) -> tuple[ParamList, ParamList]: + """Splits the parameters of a target into the ones given through ``*args`` and through ``**kwargs``. + + The ones before a ``*args`` of the target go through ``*args``, since a non-empty ``*args`` + requires them to be given positionally. Without ``**kwargs`` forwarded, the parameters that + can be given positionally are only accepted positionally. + """ + args_idx = get_arg_kind_index(params, kinds.VAR_POSITIONAL) + args = [] + kwargs = [] + for num, param in enumerate(params): + if param.kind in {kinds.POSITIONAL_ONLY, kinds.VAR_POSITIONAL} or ( + param.kind == kinds.POSITIONAL_OR_KEYWORD and (num < args_idx or not forwards_kwargs) + ): + if not forwards_kwargs and param.kind == kinds.POSITIONAL_OR_KEYWORD: + param = dataclasses.replace(param, kind=kinds.POSITIONAL_ONLY) + args.append(param) + elif param.kind in kwargs_kinds: + kwargs.append(param) return args, kwargs -def replace_args_and_kwargs(params: ParamList, args: ParamList, kwargs: ParamList) -> ParamList: +def replace_args_and_kwargs(params: ParamList, args: ParamList | None, kwargs: ParamList) -> ParamList: + """Replaces ``*args`` and ``**kwargs`` in params. When args is None, the ``*args`` is kept.""" args_idx = get_arg_kind_index(params, kinds.VAR_POSITIONAL) kwargs_idx = get_arg_kind_index(params, kinds.VAR_KEYWORD) - if args_idx >= 0: + if args_idx >= 0 and args is not None: params = params[:args_idx] + args + params[args_idx + 1 :] if kwargs_idx >= 0: kwargs_idx += len(args) - 1 @@ -625,6 +645,8 @@ def __init__( ): super().__init__(**kwargs) self.component, self.parent, _ = get_component_and_parent(function_or_class, method_or_property) + # keyed by id of the call node, which is kept in the value so that the id is not reused + self.call_parameters: dict[int, tuple[ast.AST, ParamList | None]] = {} def log_debug(self, message) -> None: self.logger.debug(f"AST resolver: {message}") @@ -638,7 +660,9 @@ def parse_source_tree(self): tree = ast.parse(source) assert isinstance(tree, ast.Module) and len(tree.body) == 1 self.component_node = tree.body[0] - self.self_name = self.component_node.args.args[0].arg if self.parent else None + args = self.component_node.args + # self can be positional-only, e.g. in a method with a positional-only parameter + self.self_name = (args.posonlyargs + args.args)[0].arg if self.parent else None except Exception as ex: raise SourceNotAvailable(f"Problems getting source code for {self.component}: {ex}") from ex @@ -862,7 +886,57 @@ def get_kwargs_pop_or_get_parameter(self, node, component, parent, doc_params): origin=param_kwargs_pop_or_get + self.get_node_origin(node), ) - def get_parameters_args_and_kwargs(self, kwargs_idx: int) -> tuple[ParamList, ParamList]: + def get_call_parameters(self, node, source) -> ParamList | None: + """Parameters of the component called by node, or None if it could not be determined.""" + if id(node) not in self.call_parameters: + self.call_parameters[id(node)] = (node, self.resolve_call_parameters(node, source)) + params = self.call_parameters[id(node)][1] + return None if params is None else [dataclasses.replace(p) for p in params] + + def resolve_call_parameters(self, node, source) -> ParamList | None: + params = None + if self.parent and ast_is_super_call(node): + if ast_is_supported_super_call(node, self.self_name, self.log_debug): + params = get_mro_parameters( + node.func.attr, + partial(get_signature_parameters, include_var_keyword=True, include_var_positional=True), + self.logger, + ) + else: + get_param_args = self.get_node_component(node, source) + if get_param_args: + params = get_signature_parameters( + *get_param_args, logger=self.logger, include_var_keyword=True, include_var_positional=True + ) + return params + + def get_parameters_args(self, args_name, args_uses, kwargs_calls) -> ParamList | None: + """Parameters that ``*args`` stands for, or None when it is to be kept as a parameter.""" + references = sum( + isinstance(n, ast.Name) and n.id == args_name for s in self.component_node.body for n in ast.walk(s) + ) + if not references: + self.log_debug(f"*{args_name} is not used, so its values would be discarded") + return [] + forward_calls = [(n, s) for n, s in args_uses if isinstance(n, ast.Call)] + if references > len(forward_calls): + self.log_debug(f"*{args_name} is used other than forwarded to calls") + return None + resolved = [] + for node, source in forward_calls: + params = self.get_call_parameters(node, source) + if params is None: + self.log_debug(f"*{args_name} is forwarded to an unresolved call: {ast.unparse(node)}") + return None + params = remove_given_parameters(node, params) + resolved.append(split_args_and_kwargs(params, forwards_kwargs=any(node is n for n in kwargs_calls))[0]) + signatures = [[(p.name, p.kind, p.annotation, p.default) for p in params] for params in resolved] + if any(s != signatures[0] for s in signatures[1:]): + self.log_debug(f"*{args_name} is forwarded to calls that differ in their positional parameters") + return None + return resolved[0] + + def get_parameters_args_and_kwargs(self, kwargs_idx: int) -> tuple[ParamList | None, ParamList]: self.parse_source_tree() args_name = getattr(self.component_node.args.vararg, "arg", None) # kwargs_idx is negative when the **kwargs was already replaced, i.e. an Unpack[TypedDict] @@ -875,6 +949,8 @@ def get_parameters_args_and_kwargs(self, kwargs_idx: int) -> tuple[ParamList, Pa values_found = self.find_values_usage(values_to_find) kwargs_uses = [(v, s) for k, v, s in values_found if k == kwargs_name] if kwargs_name else [] + args_uses = [(v, s) for k, v, s in values_found if k == args_name] if args_name else [] + args = self.get_parameters_args(args_name, args_uses, [v for v, _ in kwargs_uses]) if args_name else [] # a **kwargs not used in the body is discarded by the component, but still accepted kwargs_unresolved = bool(kwargs_name) and not kwargs_uses @@ -894,17 +970,8 @@ def get_parameters_args_and_kwargs(self, kwargs_idx: int) -> tuple[ParamList, Pa kwarg = ast_get_call_kwarg_with_value(node, kwargs_value) if kwarg.arg: self.log_debug(f"kwargs given as keyword parameter not supported: {ast.unparse(node)}") - elif self.parent and ast_is_super_call(node): - if ast_is_supported_super_call(node, self.self_name, self.log_debug): - params = get_mro_parameters( - node.func.attr, # type: ignore[attr-defined] - partial(get_signature_parameters, include_var_keyword=True), - self.logger, - ) else: - get_param_args = self.get_node_component(node, source) - if get_param_args: - params = get_signature_parameters(*get_param_args, logger=self.logger, include_var_keyword=True) + params = self.get_call_parameters(node, source) if params is not None: params = remove_given_parameters(node, params, removed_params) elif self.parent and (self_attr := ast_is_attr_assign(node, self.self_name)): @@ -921,7 +988,8 @@ def get_parameters_args_and_kwargs(self, kwargs_idx: int) -> tuple[ParamList, Pa # a pop/get from kwargs means the parameter is accepted, even if the value is then given explicitly removed_params -= pop_or_get_params params = [p for p in params if p.name not in removed_params] - args, kwargs = split_args_and_kwargs(params) + args_names = {p.name for p in args or []} + kwargs = [p for p in params if p.kind in kwargs_kinds and p.name not in args_names] if kwargs_unresolved and not accepts_unresolved_kwargs(kwargs): kwargs.append(get_unresolved_kwargs_param(self.component, self.parent)) return args, kwargs @@ -1262,6 +1330,7 @@ def get_parameters_by_assumptions( component, parent, method_name = get_component_and_parent(function_or_class, method_name) params, args_idx, kwargs_idx, _, stubs = get_signature_parameters_and_indexes(component, parent, logger) + subparams = None if parent and (args_idx >= 0 or kwargs_idx >= 0): with mro_context(parent): subparams = get_mro_parameters(method_name, get_parameters_by_assumptions, logger) @@ -1269,7 +1338,9 @@ def get_parameters_by_assumptions( args, kwargs = split_args_and_kwargs(subparams) params = replace_args_and_kwargs(params, args, kwargs) - params = replace_args_and_kwargs(params, [], [get_unresolved_kwargs_param(component, parent)]) + # a *args of a function is kept, since where it goes is unknown + args = None if not parent or subparams else [] + params = replace_args_and_kwargs(params, args, [get_unresolved_kwargs_param(component, parent)]) add_stub_types(stubs, params, component) return params @@ -1279,6 +1350,7 @@ def get_signature_parameters( method_or_property: str | None = None, logger: bool | str | dict | logging.Logger = True, include_var_keyword: bool = False, + include_var_positional: bool = False, ) -> ParamList: """Get parameters by inspecting ASTs, stubs or by inheritance assumptions. @@ -1294,6 +1366,8 @@ def get_signature_parameters( logger: Useful for debugging. Only logs at ``DEBUG`` level. include_var_keyword: Whether to include a ``**kwargs`` that could not be resolved, see :func:`get_unresolved_kwargs_param`. + include_var_positional: Whether to include a ``*args``, i.e. one that the component uses + itself or that could not be resolved. """ from ._typehints import is_namedtuple, is_typed_dict @@ -1333,4 +1407,6 @@ def get_signature_parameters( params = apply_partial_method(params, attr) if not include_var_keyword: params = remove_unresolved_kwargs(params) + if not include_var_positional: + params = [p for p in params if p.kind != kinds.VAR_POSITIONAL] return params diff --git a/jsonargparse/_signatures.py b/jsonargparse/_signatures.py index 2a816ee1..1d1d7465 100644 --- a/jsonargparse/_signatures.py +++ b/jsonargparse/_signatures.py @@ -5,6 +5,8 @@ import re from argparse import SUPPRESS, ArgumentParser from collections.abc import Callable +from functools import partial +from operator import methodcaller from typing import Any, Literal, Optional, Union from ._actions import _ActionConfigLoad @@ -16,7 +18,13 @@ is_subclass, is_subclasses_disabled, ) -from ._instantiation import dynamic_class_instantiator +from ._instantiation import ( + CallLayout, + bind_call, + dynamic_class_instantiator, + get_call_arguments, + get_call_layout, +) from ._namespace import Namespace, get_value_and_parent from ._optionals import ( attrs_support, @@ -71,6 +79,8 @@ class SignatureArguments(LoggerProperty): # that keyword arguments other than the resolved ones are also accepted, i.e. the signature has # a **kwargs that the parameter resolvers were unable to resolve. _accepted_kwargs: dict[str | None, set[str] | Literal[True]] + # How the values of the parameters of a signature are given in a call, keyed by its nested_key. + _call_layouts: dict[str | None, CallLayout] def add_class_arguments( self, @@ -175,6 +185,7 @@ def add_method_arguments( as_group: bool = True, as_positional: bool = False, skip: set[str | int] | None = None, + instantiate: bool = True, fail_untyped: FailUntyped = True, sub_configs: bool = False, ) -> list[str]: @@ -189,6 +200,10 @@ def add_method_arguments( as_group: Whether arguments should be added to a new argument group. as_positional: Whether to add required parameters as positional arguments. skip: Names of parameters or number of positionals that should be skipped. + instantiate: Whether :meth:`instantiate <.ArgumentParser.instantiate>` should replace + the group by a callable with the arguments bound: for a static or class method a + :func:`functools.partial`, and for other methods an :func:`operator.methodcaller` + which is called with the instance. fail_untyped: Whether to raise an exception for parameters that don't have a type: True for the required ones, "all" for all of them, False for none. sub_configs: Whether subclass type hints should be loadable from inner config file. @@ -206,6 +221,16 @@ def add_method_arguments( if not hasattr(unaliased_type, method_name) or not callable(getattr(unaliased_type, method_name)): raise ValueError('Expected "method_name" argument to be a callable member of the class.') + call_target: Callable | str = method_name + attr = inspect.getattr_static(get_generic_origin(unaliased_type), method_name) + if isinstance(attr, (staticmethod, classmethod)): + call_target = getattr(unaliased_type, method_name) + elif instantiate and nested_key is not None and any(isinstance(s, int) for s in skip or []): + raise ValueError( + "Skipping positionals of a method that is called with an instance is not supported when the group " + "is instantiated. Use instantiate=False." + ) + return self._add_signature_arguments( class_type, method_name, @@ -215,6 +240,8 @@ def add_method_arguments( skip, fail_untyped, sub_configs=sub_configs, + instantiate=instantiate, + call_target=call_target, ) def add_function_arguments( @@ -224,6 +251,7 @@ def add_function_arguments( as_group: bool = True, as_positional: bool = False, skip: set[str | int] | None = None, + instantiate: bool = True, fail_untyped: FailUntyped = True, sub_configs: bool = False, ) -> list[str]: @@ -237,6 +265,8 @@ def add_function_arguments( as_group: Whether arguments should be added to a new argument group. as_positional: Whether to add required parameters as positional arguments. skip: Names of parameters or number of positionals that should be skipped. + instantiate: Whether :meth:`instantiate <.ArgumentParser.instantiate>` should replace + the group by a :func:`functools.partial` of the function with the arguments bound. fail_untyped: Whether to raise an exception for parameters that don't have a type: True for the required ones, "all" for all of them, False for none. sub_configs: Whether subclass type hints should be loadable from inner config file. @@ -251,6 +281,7 @@ def add_function_arguments( if not callable(function): raise ValueError('Expected "function" argument to be a callable object.') + call_target = function method_name = None if hasattr(function, "__class__") and callable_instances(function.__class__): function = function.__class__ @@ -265,6 +296,8 @@ def add_function_arguments( skip, fail_untyped, sub_configs=sub_configs, + instantiate=instantiate, + call_target=call_target, ) def _add_signature_arguments( @@ -280,6 +313,7 @@ def _add_signature_arguments( instantiate: bool = True, linked_targets: set[str] | None = None, help: str | None = None, + call_target: Callable | str | None = None, ) -> list[str]: """Adds arguments from parameters of objects based on signatures and docstrings. @@ -293,7 +327,9 @@ def _add_signature_arguments( fail_untyped: Whether to raise an exception for parameters that don't have a type: True for the required ones, "all" for all of them, False for none. sub_configs: Whether subclass type hints should be loadable from inner config file. - instantiate: Whether the class group should be instantiated. + instantiate: Whether the group should be instantiated. + call_target: For a function or method, what the group is bound to on instantiation, + a callable or the name of a method to call on an instance. Returns: The list of arguments added. @@ -302,7 +338,13 @@ def _add_signature_arguments( ValueError: When there are parameters without a type that fail_untyped requires to have one. """ validate_fail_untyped(fail_untyped) - params = get_signature_parameters(function_or_class, method_name, logger=self.logger, include_var_keyword=True) + params = get_signature_parameters( + function_or_class, + method_name, + logger=self.logger, + include_var_keyword=True, + include_var_positional=True, + ) parser = self.parser if hasattr(self, "parser") else self parser._accepted_kwargs[nested_key] = get_accepted_kwargs(params) params = remove_unresolved_kwargs(params) @@ -316,6 +358,8 @@ def _add_signature_arguments( self.logger.debug( f"Skipping parameters {names} because {skip_positionals[0]} positionals requested to be skipped." ) + call_layout = get_call_layout(params, open_positionals=skip_positionals[0] if skip_positionals else 0) + parser._call_layouts[nested_key] = call_layout prefix = "--" + (nested_key + "." if nested_key else "") for param in params: @@ -340,6 +384,8 @@ def _add_signature_arguments( doc_group, config_load=len(params) > 0, instantiate=instantiate, + call_layout=call_layout, + call_target=call_target, ) ## Add parameter arguments ## @@ -409,16 +455,18 @@ def _add_signature_parameter( "so it is not included in the parsed namespace unless given." ) # Determine argument characteristics based on parameter kind and default value - if kind == kinds.POSITIONAL_ONLY: - is_required = True # Always required - is_non_positional = False # Can be positional - elif kind == kinds.KEYWORD_ONLY: + if kind == kinds.KEYWORD_ONLY: is_required = default == inspect_empty # Required if no default is_non_positional = True # Must use --flag style - elif kind in {kinds.POSITIONAL_OR_KEYWORD, None}: - # POSITIONAL_OR_KEYWORD or programmatically created parameters without kind + elif kind in {kinds.POSITIONAL_ONLY, kinds.POSITIONAL_OR_KEYWORD, None}: + # None is for programmatically created parameters without kind is_required = default == inspect_empty # Required if no default is_non_positional = False # Can be positional + elif kind == kinds.VAR_POSITIONAL: + is_required = False # Zero values are accepted + is_non_positional = False # Can be positional + if default == inspect_empty: + default = [] else: raise RuntimeError(f"The code should never reach here: kind={kind}") # pragma: no cover if annotation != inspect_empty: @@ -448,10 +496,18 @@ def _add_signature_parameter( "With fail_untyped='all', all parameters must have a supported type." f" Parameter '{name}' from '{src}' does not specify a type." ) - if not is_required: + if kind == kinds.VAR_POSITIONAL: + annotation = Untyped + elif not is_required: # The type of the default is attempted first, so that a value that it accepts is # converted as it would be for a parameter that has the type in its signature. annotation = Union[type(default), Untyped] + is_positional = as_positional and not is_non_positional and (is_required or kind == kinds.VAR_POSITIONAL) + if kind == kinds.VAR_POSITIONAL: + if is_positional: + kwargs["nargs"] = "*" # the type is of each value + else: + annotation = list[annotation] # type: ignore[valid-type] if "help" not in kwargs: kwargs["help"] = param.doc if not is_required: @@ -478,7 +534,7 @@ def _add_signature_parameter( nested_skip: set[str] = set() subclasses_disabled = is_subclasses_disabled(annotation) dest = (nested_key + "." if nested_key else "") + name - args = [dest if is_required and as_positional and not is_non_positional else "--" + dest] + args = [dest if is_positional else "--" + dest] if param.aliases and args[0].startswith("--"): args += self._get_alias_args(param, nested_key, container, subclasses_disabled, src) if param.origin: @@ -641,6 +697,8 @@ def _create_group_if_requested( config_load_type=None, required=False, instantiate=True, + call_layout=None, + call_target=None, ): if required: if nested_key is None: @@ -662,10 +720,16 @@ def _create_group_if_requested( group.add_argument("--" + nested_key, action=_ActionConfigLoad(basetype=config_load_type)) # a subscripted generic is instantiated as its origin class, since the subscript # only says what its type parameters stand for - if inspect.isclass(get_generic_origin(obj)) and nested_key is not None and instantiate: - group.dest = nested_key.replace("-", "_") - group.group_class = get_generic_origin(obj) - group.instantiate_class = group_instantiate_class + if nested_key is not None and instantiate: + if inspect.isclass(get_generic_origin(obj)): + group.dest = nested_key.replace("-", "_") + group.group_class = get_generic_origin(obj) + group.instantiate_class = group_instantiate_class + elif call_target is not None: + group.dest = nested_key.replace("-", "_") + group.call_target = call_target + group.instantiate_class = group_bind_callable + group.call_layout = call_layout return group @@ -676,14 +740,28 @@ def set_group_extra_defaults(parser, nested_key, extras: dict) -> None: action.default = Namespace(**extras) -def group_instantiate_class(group, cfg): +def get_group_values(group, cfg) -> tuple[dict, Namespace, str]: try: value, parent, key = get_value_and_parent(cfg, group.dest) except KeyError: - value = {} - parent = cfg - key = group.dest - parent[key] = dynamic_class_instantiator(group.group_class, **value) + return {}, cfg, group.dest + # only the top level keys, since a value can be a namespace, e.g. a subclass spec kept as is + return dict(value.items(branches=True, nested=False)), parent, key + + +def group_instantiate_class(group, cfg): + values, parent, key = get_group_values(group, cfg) + instantiator = partial(dynamic_class_instantiator, group.group_class) + parent[key] = bind_call(instantiator, group.call_layout, values, component=group.group_class)() + + +def group_bind_callable(group, cfg): + values, parent, key = get_group_values(group, cfg) + if isinstance(group.call_target, str): + args, kwargs = get_call_arguments(group.call_layout, values, group.call_target) + parent[key] = methodcaller(group.call_target, *args, **kwargs) + else: + parent[key] = bind_call(group.call_target, group.call_layout, values) def strip_title(value): diff --git a/jsonargparse/_typehints.py b/jsonargparse/_typehints.py index 1e448939..af519b5f 100644 --- a/jsonargparse/_typehints.py +++ b/jsonargparse/_typehints.py @@ -75,7 +75,7 @@ parser_context, validating_defaults, ) -from ._instantiation import dynamic_class_instantiator +from ._instantiation import bind_call, dynamic_class_instantiator from ._loaders_dumpers import ( basic_json_or_yaml_load, get_load_value_mode, @@ -665,6 +665,8 @@ def __call__(self, *args, **kwargs): raise ValueError("ActionTypeHint does not allow nargs=0.") return ActionTypeHint(**kwargs) parser, cfg, val, opt_str = args + if not self.option_strings and self.nargs == "*" and (val == [] or val is self.default) and self.dest in cfg: + return None # a positional not given in the command line keeps the value from defaults or configs source = command_line_source(self, opt_str) # a nested parse of the value, e.g. of an init arg of a subclass, refers to the same option with parser_context(command_line_option=source.origin), value_source_context(source): @@ -2537,14 +2539,16 @@ def adapt_class_type( # only the top level keys, since a value can be a namespace, e.g. a subclass spec # kept as is for an Any typed parameter, which must not be expanded into kwargs init_kwargs = dict(init_args.items(branches=True, nested=False)) + instantiator = bind_call( + partial(dynamic_class_instantiator, val_class), + parser._call_layouts[None], + {**init_kwargs, **dict_kwargs}, + component=val_class, + ) if partial_skip_args is not None: # an empty set for a factory that takes no arguments - return partial( - dynamic_class_instantiator, - val_class, - **{**init_kwargs, **dict_kwargs}, - ) - return dynamic_class_instantiator(val_class, **{**init_kwargs, **dict_kwargs}) + return instantiator + return instantiator() prev_init_args = prev_val.get("init_args") if isinstance(prev_val, Namespace) else None diff --git a/jsonargparse_tests/test_cli.py b/jsonargparse_tests/test_cli.py index 1b781983..ea203b73 100644 --- a/jsonargparse_tests/test_cli.py +++ b/jsonargparse_tests/test_cli.py @@ -323,6 +323,61 @@ def test_class_property_value(): assert "Description of property" in help_str +# positional-only and var-positional parameters tests + + +def function_positional_only(a: int, /, b: str = "-"): + return a, b + + +def function_var_positional(first: int, *rest: float, flag: bool = False): + return first, rest, flag + + +class PositionalOnlyAndVarPositional: + def __init__(self, x: int, /, *extra: str): + self.x = x + self.extra = extra + + def method(self, a: int, /, *rest: int): + return self.x, self.extra, a, rest + + +def test_function_positional_only(): + assert (1, "-") == auto_cli(function_positional_only, args=["1"]) + + +def test_function_var_positional(): + assert (1, (2.5, 3.0), False) == auto_cli(function_var_positional, args=["1", "2.5", "3"]) + assert (1, (), True) == auto_cli(function_var_positional, args=["1", "--flag=true"]) + config = '--config={"first": 1, "rest": [2.5]}' + assert (1, (2.5,), False) == auto_cli(function_var_positional, args=[config]) + assert (1, (3.0,), False) == auto_cli(function_var_positional, args=[config, "1", "3"]) + assert (1, (2.5,), False) == auto_cli(function_var_positional, as_positional=False, args=[config]) + help_str = get_cli_stdout(function_var_positional, args=["--help"]) + assert "first [rest ...]" in help_str + + +def test_class_positional_only_and_var_positional(): + args = ["--x=1", '--extra=["a"]', "method", "--a=2", "--rest=[3,4]"] + assert (1, ("a",), 2, (3, 4)) == auto_cli(PositionalOnlyAndVarPositional, as_positional=False, args=args) + obj = auto_cli(PositionalOnlyAndVarPositional, args=["1", "a", "b"], return_instance=True) + assert (obj.x, obj.extra) == (1, ("a", "b")) + + +def test_class_var_positional_and_subcommands_unsupported(): + with pytest.raises(ValueError) as ctx: + auto_parser(PositionalOnlyAndVarPositional) + ctx.match("'extra' accepts a variable number of values") + ctx.match("as_positional=False") + + +def test_subcommands_positional_only_and_var_positional(): + components = {"fn": {"po": function_positional_only}, "cls": PositionalOnlyAndVarPositional} + assert (1, "-") == auto_cli(components, as_positional=False, args=["fn", "po", "--a=1"]) + assert (1, (), 2, ()) == auto_cli(components, as_positional=False, args=["cls", "--x=1", "method", "--a=2"]) + + # function and class tests diff --git a/jsonargparse_tests/test_core.py b/jsonargparse_tests/test_core.py index a5649278..d6f813b2 100644 --- a/jsonargparse_tests/test_core.py +++ b/jsonargparse_tests/test_core.py @@ -58,6 +58,15 @@ def test_add_argument_positional_with_default_nargs_star(parser): assert parser.parse_args(["x", "y"]) == Namespace(pos=["x", "y"]) +@pytest.mark.parametrize("default", [None, [0]]) +def test_positional_nargs_star_not_given_keeps_config_value(parser, default): + parser.add_argument("--cfg", action="config") + parser.add_argument("pos", nargs="*", type=int, default=default) + assert parser.parse_args(['--cfg={"pos": [1, 2]}']).pos == [1, 2] + assert parser.parse_args(['--cfg={"pos": [1, 2]}', "3"]).pos == [3] + assert parser.parse_args([]).pos == default + + def test_add_argument_positional_with_default_nargs_question(parser): parser.add_argument("pos", nargs="?", default="abc") assert parser.parse_args([]) == Namespace(pos="abc") diff --git a/jsonargparse_tests/test_dataclasses.py b/jsonargparse_tests/test_dataclasses.py index 8c865d5f..0d57e99b 100644 --- a/jsonargparse_tests/test_dataclasses.py +++ b/jsonargparse_tests/test_dataclasses.py @@ -521,7 +521,7 @@ class DataWithOptionalA: def data_with_optional(a: DataWithOptionalA): - pass # pragma: no cover + return a def test_dataclass_with_optional_default(parser): @@ -529,7 +529,8 @@ def test_dataclass_with_optional_default(parser): cfg = parser.parse_args([]) assert cfg.data == Namespace(a=Namespace(b=Namespace(c=3))) init = parser.instantiate(cfg) - assert init.data.a == DataWithOptionalA() + assert init.data.keywords["a"] == DataWithOptionalA() + assert init.data.keywords["a"] == init.data() @dataclasses.dataclass diff --git a/jsonargparse_tests/test_from_config.py b/jsonargparse_tests/test_from_config.py index e087152d..9d719b64 100644 --- a/jsonargparse_tests/test_from_config.py +++ b/jsonargparse_tests/test_from_config.py @@ -266,6 +266,17 @@ def __init__(self, param: str = "default_value"): assert instance.param == "value_from_dict" +def test_from_config_method_positional_only_and_var_positional(): + class FromConfigMethodPositionals(FromConfigMixin): + def __init__(self, first: int, /, *rest: int, param: str = "default_value"): + self.first = first + self.rest = rest + self.param = param + + instance = FromConfigMethodPositionals.from_config({"first": 1, "rest": [2, 3]}) + assert (instance.first, instance.rest, instance.param) == (1, (2, 3), "default_value") + + def test_from_config_method_default(): from os import PathLike from typing import Literal, Type, TypeVar, Union diff --git a/jsonargparse_tests/test_link_arguments.py b/jsonargparse_tests/test_link_arguments.py index 5c95618d..4e990b38 100644 --- a/jsonargparse_tests/test_link_arguments.py +++ b/jsonargparse_tests/test_link_arguments.py @@ -348,7 +348,7 @@ def test_on_parse_subclass_target_in_optional_list(parser): class LinkedDefaultTarget: - def __init__(self, p1: int = 0, p2: str = "-"): + def __init__(self, p1: int = 0, p2: str = "-"): # pragma: no cover self.p1 = p1 self.p2 = p2 @@ -1229,6 +1229,38 @@ def test_on_instantiate_source_module_type_target_subclass(parser): assert init.user.mod is json +class FitModel: + def __init__(self, size: int = 1): + self.size = size + + +def fit(model: FitModel, *callbacks: str, epochs: int = 1): + return model, callbacks, epochs + + +def test_on_instantiate_target_function_group(parser): + parser.add_function_arguments(fit, "fit") + parser.add_class_arguments(FitModel, "model") + parser.link_arguments("model", "fit.model", apply_on="instantiate") + + cfg = parser.parse_args(["--model.size=3", '--fit.callbacks=["a", "b"]']) + init = parser.instantiate(cfg) + model, callbacks, epochs = init.fit() + assert model is init.model + assert (model.size, callbacks, epochs) == (3, ("a", "b"), 1) + + +def test_on_parse_target_var_positional(parser): + parser.add_argument("--names", type=str, default="a,b") + parser.add_function_arguments(fit, "fit", skip={"model"}) + parser.link_arguments("names", "fit.callbacks", compute_fn=lambda names: names.split(",")) + + cfg = parser.parse_args(["--names=x,y"]) + assert cfg.fit.callbacks == ["x", "y"] + with pytest.raises(ValueError, match='requires a value for parameter "model"'): + parser.instantiate(cfg) + + # link creation failures diff --git a/jsonargparse_tests/test_parameter_resolvers.py b/jsonargparse_tests/test_parameter_resolvers.py index 550743f6..1fde7a99 100644 --- a/jsonargparse_tests/test_parameter_resolvers.py +++ b/jsonargparse_tests/test_parameter_resolvers.py @@ -1197,6 +1197,168 @@ def test_accepts_extra_kwargs_from_assumptions(): assert accepts_extra_kwargs(ClassM1) is False +# var-positional parameters tests + + +def get_params_kinds(component, method=None) -> list[tuple[str, str | None]]: + params = get_params(component, method, include_var_positional=True) + return [(p.name, getattr(p.kind, "name", None)) for p in params] + + +class PositionalTarget: + def __init__(self, x: int, y: int = 1, *, k: int = 0): + pass # pragma: no cover + + +class ForwardsOnlyArgs(PositionalTarget): + def __init__(self, *args): + super().__init__(*args) # pragma: no cover + + +class VarPositionalTarget: + def __init__(self, x: int, *rest: int, k: int = 0): + self.rest = rest # pragma: no cover + + +class ForwardsArgsAndKwargs(VarPositionalTarget): + def __init__(self, *args, z: int = 2, **kwargs): + super().__init__(*args, **kwargs) # pragma: no cover + + +class MethodSelfPositionalOnly: + def method(self, a: int, /, *rest: int): + return rest # pragma: no cover + + +def function_forwards_with_given_positional(*args, **kwargs): + return VarPositionalTarget(1, *args, **kwargs) # pragma: no cover + + +def function_uses_var_positional(*files: str): + return [f.upper() for f in files] # pragma: no cover + + +def function_var_positional_unresolved_forward(*args): + return undefined_function(*args) # noqa: F821 # pragma: no cover + + +def function_var_positional_multiple_forwards(flag: bool = False, *args): # pragma: no cover + if flag: + return function_uses_var_positional(*args) + return function_uses_var_positional(*args) + + +def function_var_positional_different_forwards(flag: bool = False, *args): # pragma: no cover + if flag: + return function_uses_var_positional(*args) + return PositionalTarget(*args) + + +def target_int_positional(x: int = 1): + return x # pragma: no cover + + +def target_str_positional(x: str = "two"): + return x # pragma: no cover + + +def function_var_positional_forwards_differ_in_type(flag: bool = False, *args): # pragma: no cover + if flag: + return target_int_positional(*args) + return target_str_positional(*args) + + +def target_int_positional_other_default(x: int = 3): + return x # pragma: no cover + + +def function_var_positional_forwards_differ_in_default(flag: bool = False, *args): # pragma: no cover + if flag: + return target_int_positional(*args) + return target_int_positional_other_default(*args) + + +def function_var_positional_used_and_forwarded(*args): # pragma: no cover + print(len(args)) + return function_uses_var_positional(*args) + + +def function_var_positional_unused(*args, k: int = 0): + return k # pragma: no cover + + +@pytest.mark.parametrize( + ["component", "method", "expected"], + [ + (ForwardsOnlyArgs, None, [("x", "POSITIONAL_ONLY"), ("y", "POSITIONAL_ONLY")]), + ( + ForwardsArgsAndKwargs, + None, + [("x", "POSITIONAL_OR_KEYWORD"), ("rest", "VAR_POSITIONAL"), ("z", "KEYWORD_ONLY"), ("k", "KEYWORD_ONLY")], + ), + (MethodSelfPositionalOnly, "method", [("a", "POSITIONAL_ONLY"), ("rest", "VAR_POSITIONAL")]), + (function_forwards_with_given_positional, None, [("rest", "VAR_POSITIONAL"), ("k", "KEYWORD_ONLY")]), + (function_uses_var_positional, None, [("files", "VAR_POSITIONAL")]), + ( + function_var_positional_multiple_forwards, + None, + [("flag", "POSITIONAL_OR_KEYWORD"), ("files", "VAR_POSITIONAL")], + ), + ], +) +def test_get_params_var_positional_resolved(component, method, expected): + assert get_params_kinds(component, method) == expected + + +@pytest.mark.parametrize( + ["component", "expected", "message"], + [ + (function_var_positional_unresolved_forward, [("args", "VAR_POSITIONAL")], "forwarded to an unresolved call"), + ( + function_var_positional_different_forwards, + [("flag", "POSITIONAL_OR_KEYWORD"), ("args", "VAR_POSITIONAL")], + "forwarded to calls that differ in their positional parameters", + ), + ( + function_var_positional_forwards_differ_in_type, + [("flag", "POSITIONAL_OR_KEYWORD"), ("args", "VAR_POSITIONAL")], + "forwarded to calls that differ in their positional parameters", + ), + ( + function_var_positional_forwards_differ_in_default, + [("flag", "POSITIONAL_OR_KEYWORD"), ("args", "VAR_POSITIONAL")], + "forwarded to calls that differ in their positional parameters", + ), + ( + function_var_positional_used_and_forwarded, + [("args", "VAR_POSITIONAL")], + "used other than forwarded to calls", + ), + (function_var_positional_unused, [("k", "KEYWORD_ONLY")], "*args is not used"), + ], +) +def test_get_params_var_positional_not_resolved(component, expected, message, logger): + with capture_logs(logger) as logs: + params = get_params(component, logger=logger, include_var_positional=True) + assert [(p.name, getattr(p.kind, "name", None)) for p in params] == expected + assert message in logs.getvalue() + + +def test_get_params_var_positional_not_included_by_default(): + assert get_params(function_uses_var_positional) == [] + assert_params(get_params(ForwardsArgsAndKwargs), ["x", "z", "k"], help=False) + + +def test_get_params_var_positional_from_assumptions(): + with source_unavailable(): + assert get_params_kinds(function_uses_var_positional) == [("files", "VAR_POSITIONAL")] + assert get_params_kinds(ForwardsArgsAndKwargs) == [ + ("z", "KEYWORD_ONLY"), + ("x", "POSITIONAL_OR_KEYWORD"), + ("k", "KEYWORD_ONLY"), + ] + + # failure cases diff --git a/jsonargparse_tests/test_signatures.py b/jsonargparse_tests/test_signatures.py index 687a7943..e611a1c9 100644 --- a/jsonargparse_tests/test_signatures.py +++ b/jsonargparse_tests/test_signatures.py @@ -2,6 +2,8 @@ import functools import json +import operator +import pickle import sys from pathlib import Path from typing import Any, Dict, Generic, List, Optional, Tuple, TypedDict, TypeVar, Union @@ -392,7 +394,8 @@ def test_add_class_skip_parameter_debug_logging(parser, logger): parser.logger = logger with capture_logs(logger) as logs: parser.add_class_arguments(Debug2, skip={"c2_a2"}) - assert 1 == len(logs.getvalue().strip().split("\n")) + assert 2 == len(logs.getvalue().strip().split("\n")) + assert "*args is not used, so its values would be discarded" in logs.getvalue() assert 'Skipping parameter "c2_a2"' in logs.getvalue() assert "because of: Parameter requested to be skipped" in logs.getvalue() @@ -1001,3 +1004,272 @@ def test_add_function_positional_and_keyword_only_parameters(parser): parser.parse_args(["1", "--b=2", "--c=3", "--d=4"]) with pytest.raises(ArgumentError, match="the following arguments are required: c"): parser.parse_args(["1", "2", "--d=4"]) + + +# positional-only and var-positional parameters tests + + +class ClassPositionalOnly: + def __init__(self, a: int, /, b: int = 2): + self.a = a + self.b = b + + +def test_add_class_positional_only_instantiate(parser): + parser.add_class_arguments(ClassPositionalOnly, "cls") + cfg = parser.parse_args(["--cls.a=1"]) + init = parser.instantiate(cfg) + assert isinstance(init.cls, ClassPositionalOnly) + assert (init.cls.a, init.cls.b) == (1, 2) + with pytest.raises(ArgumentError, match="the following arguments are required: cls.a"): + parser.parse_args([]) + + +def func_positional_only_default(a: int = 1, /, b: int = 2): + return a, b + + +def test_add_function_positional_only_default_not_required(parser): + parser.add_function_arguments(func_positional_only_default, "fn") + cfg = parser.parse_args([]) + assert cfg.fn == Namespace(a=1, b=2) + assert parser.instantiate(cfg).fn() == (1, 2) + cfg = parser.parse_args(["--fn.a=3"]) + assert parser.instantiate(cfg).fn() == (3, 2) + + +def test_add_function_positional_only_default_as_positional(parser): + parser.add_function_arguments(func_positional_only_default, as_positional=True) + assert parser.parse_args([]) == Namespace(a=1, b=2) + assert parser.parse_args(["--a=3"]) == Namespace(a=3, b=2) + + +class ClassVarPositional: + def __init__(self, a: int, *args: int, b: int = 0): + """ + Args: + a: a description + args: args description + """ + self.a = a + self.args = args + self.b = b + + +def test_add_class_var_positional(parser): + added_args = parser.add_class_arguments(ClassVarPositional, "cls") + assert added_args == ["cls.a", "cls.args", "cls.b"] + assert parser.get_defaults().cls == Namespace(a=None, args=[], b=0) + assert find_action(parser, "cls.args")._typehint == list[int] + + cfg = parser.parse_args(["--cls.a=1", "--cls.args=[2, 3]", "--cls.b=4"]) + assert cfg.cls == Namespace(a=1, args=[2, 3], b=4) + init = parser.instantiate(cfg) + assert (init.cls.a, init.cls.args, init.cls.b) == (1, (2, 3), 4) + + cfg = parser.parse_args(["--cls.a=1", "--cls.args+=5", "--cls.args+=6"]) + assert cfg.cls.args == [5, 6] + + with pytest.raises(ArgumentError, match='Parser key "cls.args"'): + parser.parse_args(["--cls.a=1", "--cls.args=[x]"]) + + init = parser.instantiate(parser.parse_args(["--cls.a=1"])) + assert (init.cls.a, init.cls.args, init.cls.b) == (1, (), 0) + + +@skip_if_docstring_parser_unavailable +def test_add_class_var_positional_help(parser): + parser.add_class_arguments(ClassVarPositional, "cls") + help_str = get_parser_help(parser) + assert "--cls.args [ITEM,...]" in help_str + assert "args description (type: list[int], default: [])" in help_str + + +def test_add_class_var_positional_skip(parser): + added_args = parser.add_class_arguments(ClassVarPositional, "cls", skip={"args"}) + assert added_args == ["cls.a", "cls.b"] + init = parser.instantiate(parser.parse_args(["--cls.a=1"])) + assert init.cls.args == () + + +class ClassPositionalDefaultThenVarPositional: + def __init__(self, a: int = 1, *args: int): + self.a = a + self.args = args + + +def test_var_positional_skipped_preceding_positional_uses_default(parser): + parser.add_class_arguments(ClassPositionalDefaultThenVarPositional, "cls", skip={"a"}) + init = parser.instantiate(parser.parse_args(["--cls.args=[2, 3]"])) + assert (init.cls.a, init.cls.args) == (1, (2, 3)) + + +def test_var_positional_skipped_preceding_required_positional(parser): + parser.add_class_arguments(ClassVarPositional, "cls", skip={"a"}) + cfg = parser.parse_args(["--cls.args=[2, 3]"]) + with pytest.raises(ValueError, match='requires a value for parameter "a", since it precedes "args"'): + parser.instantiate(cfg) + + +def func_var_positional_untyped(*args, k: int = 0): + return args, k + + +def test_add_function_var_positional_untyped(parser): + parser.add_function_arguments(func_var_positional_untyped, "fn") + assert find_action(parser, "fn.args")._typehint == list[Untyped] + cfg = parser.parse_args(['--fn.args=[1, "x", 2.5]']) + assert cfg.fn.args == [1, "x", 2.5] + init = parser.instantiate(cfg) + assert init.fn() == ((1, "x", 2.5), 0) + + +def test_add_function_var_positional_untyped_fail_untyped_all(parser): + with pytest.raises(ValueError, match="Parameter 'args' from .* does not specify a type"): + parser.add_function_arguments(func_var_positional_untyped, "fn", fail_untyped="all") + + +def func_var_positional_underscore(*_rest: Any, k: int = 0): + return _rest, k # pragma: no cover + + +def test_add_function_var_positional_underscore_skipped(parser, logger): + parser.logger = logger + with capture_logs(logger) as logs: + added_args = parser.add_function_arguments(func_var_positional_underscore, "fn") + assert added_args == ["fn.k"] + assert 'Skipping parameter "_rest"' in logs.getvalue() + + +def func_positional_only_and_var_positional(a: int, /, b: str, *rest: float, c: bool = False): + return a, b, rest, c + + +def test_add_function_positional_only_and_var_positional(parser): + parser.add_function_arguments(func_positional_only_and_var_positional, "fn") + cfg = parser.parse_args(["--fn.a=1", "--fn.b=x", "--fn.rest=[0.5]"]) + assert cfg.fn == Namespace(a=1, b="x", rest=[0.5], c=False) + init = parser.instantiate(cfg) + assert init.fn() == (1, "x", (0.5,), False) + cfg = parser.parse_args(["--fn.a=1", "--fn.b=x"]) + assert parser.instantiate(cfg).fn() == (1, "x", (), False) + + +def test_add_function_skip_positional_var_positional_empty(parser): + parser.add_function_arguments(func_positional_only_and_var_positional, "fn", skip={1}) + init = parser.instantiate(parser.parse_args(["--fn.b=x"])) + assert isinstance(init.fn, functools.partial) + assert init.fn(3) == (3, "x", (), False) + + +@pytest.mark.skipif(sys.version_info < (3, 14), reason="functools.Placeholder introduced in python 3.14") +def test_add_function_skip_positional_var_positional_placeholder(parser): + parser.add_function_arguments(func_positional_only_and_var_positional, "fn", skip={1}) + cfg = parser.parse_args(["--fn.b=x", "--fn.rest=[0.5, 1.5]", "--fn.c=true"]) + init = parser.instantiate(cfg) + assert isinstance(init.fn, functools.partial) + assert init.fn.args == (functools.Placeholder, "x", 0.5, 1.5) + assert init.fn(3) == (3, "x", (0.5, 1.5), True) + assert init.fn(3, 2.5) == (3, "x", (0.5, 1.5, 2.5), True) + assert init.fn(3, c=False) == (3, "x", (0.5, 1.5), False) + assert pickle.loads(pickle.dumps(init.fn))(4) == (4, "x", (0.5, 1.5), True) + with pytest.raises(TypeError, match="missing positional arguments"): + init.fn() + + +@pytest.mark.skipif(sys.version_info >= (3, 14), reason="functools.Placeholder introduced in python 3.14") +def test_add_function_skip_positional_var_positional_before_python_3_14(parser): + parser.add_function_arguments(func_positional_only_and_var_positional, "fn", skip={1}) + cfg = parser.parse_args(["--fn.b=x", "--fn.rest=[0.5, 1.5]"]) + with pytest.raises(ValueError, match="only supported in Python 3.14 or later"): + parser.instantiate(cfg) + + +# instantiate of function and method groups tests + + +def test_add_function_instantiate_partial(parser): + parser.add_function_arguments(func, "fn") + cfg = parser.parse_args(["--fn.a1=x"]) + init = parser.instantiate(cfg) + assert isinstance(init.fn, functools.partial) + assert init.fn.func is func + assert init.fn.keywords == {"a1": "x", "a2": 2.0, "a3": False, "a4": None} + assert init.fn() == "x" + assert pickle.loads(pickle.dumps(init.fn))() == "x" + assert cfg.fn == Namespace(a1="x", a2=2.0, a3=False, a4=None) + + +def test_add_function_instantiate_false(parser): + parser.add_function_arguments(func, "fn", instantiate=False) + cfg = parser.parse_args(["--fn.a1=x"]) + assert parser.instantiate(cfg) == cfg + + +def test_add_function_instantiate_groups_false(parser): + parser.add_function_arguments(func, "fn") + cfg = parser.parse_args(["--fn.a1=x"]) + assert parser.instantiate(cfg, instantiate_groups=False) == cfg + + +def test_add_function_without_nested_key_not_bound(parser): + parser.add_function_arguments(func) + cfg = parser.parse_args(["--a1=x"]) + assert parser.instantiate(cfg) == cfg + + +def test_add_function_callable_instance_instantiate(parser): + instance = Class1(c1_a1="-") + parser.add_function_arguments(instance, "fn") + init = parser.instantiate(parser.parse_args([])) + assert init.fn.func is instance + assert init.fn() == "-" + + +class WithMethodKinds: + def __init__(self, p: int = 0): + self.p = p + + def normal_method(self, a: int, /, *rest: int, k: str = "-"): + return self.p, a, rest, k + + @staticmethod + def static_method(a: int, *rest: int): + return a, rest + + @classmethod + def class_method(cls, a: int): + return cls, a + + +class SubWithMethodKinds(WithMethodKinds): + def normal_method(self, a: int, /, *rest: int, k: str = "-"): + return "sub", a, rest, k + + +def test_add_method_instantiate_normal_method(parser): + parser.add_method_arguments(WithMethodKinds, "normal_method", "m") + cfg = parser.parse_args(["--m.a=1", "--m.rest=[2, 3]"]) + init = parser.instantiate(cfg) + assert isinstance(init.m, operator.methodcaller) + assert init.m(WithMethodKinds(p=5)) == (5, 1, (2, 3), "-") + assert init.m(SubWithMethodKinds()) == ("sub", 1, (2, 3), "-") + assert pickle.loads(pickle.dumps(init.m))(WithMethodKinds()) == (0, 1, (2, 3), "-") + + +def test_add_method_instantiate_static_and_class_methods(parser): + parser.add_method_arguments(WithMethodKinds, "static_method", "s") + parser.add_method_arguments(SubWithMethodKinds, "class_method", "c") + init = parser.instantiate(parser.parse_args(["--s.a=1", "--s.rest=[2]", "--c.a=3"])) + assert isinstance(init.s, functools.partial) + assert init.s() == (1, (2,)) + assert isinstance(init.c, functools.partial) + assert init.c() == (SubWithMethodKinds, 3) + + +def test_add_method_instantiate_skip_positionals(parser): + with pytest.raises(ValueError, match="Skipping positionals of a method that is called with an instance"): + parser.add_method_arguments(WithMethodKinds, "normal_method", "m", skip={1}) + parser.add_method_arguments(WithMethodKinds, "normal_method", "m", skip={1}, instantiate=False) + cfg = parser.parse_args(["--m.k=x"]) + assert parser.instantiate(cfg) == cfg diff --git a/jsonargparse_tests/test_subclasses.py b/jsonargparse_tests/test_subclasses.py index 7efe695d..443ea96f 100644 --- a/jsonargparse_tests/test_subclasses.py +++ b/jsonargparse_tests/test_subclasses.py @@ -2,6 +2,7 @@ import json import os +import sys import textwrap import warnings from abc import ABC, abstractmethod @@ -1624,6 +1625,110 @@ def test_subclass_dict_kwargs_skipped_parameter(parser): parser.parse_args([f"--cls={json.dumps(value)}"]) +# positional-only and var-positional parameters tests + + +class VarPositional: + def __init__(self, *a: int, b: int): + self.a = a + self.b = b + + +def test_subclass_var_positional(parser, tmp_cwd): + parser.add_argument("--cfg", action="config") + parser.add_argument("--a", type=VarPositional) + + config = {"a": {"class_path": f"{__name__}.VarPositional", "init_args": {"a": [1, 2], "b": 3}}} + Path("config.yaml").write_text(json_or_yaml_dump(config)) + cfg = parser.parse_args(["--cfg=config.yaml"]) + assert cfg.a.init_args == Namespace(a=[1, 2], b=3) + init = parser.instantiate(cfg) + assert isinstance(init.a, VarPositional) + assert (init.a.a, init.a.b) == ((1, 2), 3) + + cfg = parser.parse_args(["--cfg=config.yaml", "--a.a+=4"]) + assert cfg.a.init_args.a == [1, 2, 4] + + help_str = get_parse_args_stdout(parser, [f"--a.help={__name__}.VarPositional"]) + assert "--a.a [ITEM,...]" in help_str + + +def test_subclass_var_positional_in_dict_kwargs(parser): + parser.add_argument("--a", type=VarPositional) + value = {"class_path": f"{__name__}.VarPositional", "init_args": {"b": 3}, "dict_kwargs": {"a": [1]}} + cfg = parser.parse_args([f"--a={json.dumps(value)}"]) + assert cfg.a.init_args == Namespace(a=[1], b=3) + assert "dict_kwargs" not in cfg.a + assert parser.instantiate(cfg).a.a == (1,) + + +class PositionalOnly: + def __init__(self, x: int, /, y: int = 1): + self.x = x + self.y = y + + +def test_subclass_positional_only(parser): + parser.add_argument("--p", type=PositionalOnly) + cfg = parser.parse_args([f"--p={__name__}.PositionalOnly", "--p.x=2"]) + init = parser.instantiate(cfg) + assert (init.p.x, init.p.y) == (2, 1) + + +class ForwardsVarPositional(PositionalOnly): + def __init__(self, *args, z: int = 3, **kwargs): + super().__init__(*args, **kwargs) + self.z = z + + +def test_subclass_forwarded_positional_only(parser): + parser.add_argument("--p", type=PositionalOnly) + cfg = parser.parse_args([f"--p={__name__}.ForwardsVarPositional", "--p.x=2", "--p.y=4"]) + assert cfg.p.init_args == Namespace(x=2, z=3, y=4) + init = parser.instantiate(cfg) + assert (init.p.x, init.p.y, init.p.z) == (2, 4, 3) + + +class CallableVarPositional: + def __init__(self, x: int, *layers: int, k: int = 0): + self.x = x + self.layers = layers + self.k = k + + +callable_var_positional_value = { + "class_path": f"{__name__}.CallableVarPositional", + "init_args": {"layers": [2, 3], "k": 4}, +} + + +@pytest.mark.skipif(sys.version_info < (3, 14), reason="functools.Placeholder introduced in python 3.14") +def test_callable_return_class_var_positional(parser): + parser.add_argument("--fn", type=Callable[[int], CallableVarPositional]) + cfg = parser.parse_args([f"--fn={json.dumps(callable_var_positional_value)}"]) + init = parser.instantiate(cfg) + obj = init.fn(1) + assert isinstance(obj, CallableVarPositional) + assert (obj.x, obj.layers, obj.k) == (1, (2, 3), 4) + + +@pytest.mark.skipif(sys.version_info >= (3, 14), reason="functools.Placeholder introduced in python 3.14") +def test_callable_return_class_var_positional_before_python_3_14(parser): + parser.add_argument("--fn", type=Callable[[int], CallableVarPositional]) + cfg = parser.parse_args([f"--fn={json.dumps(callable_var_positional_value)}"]) + with pytest.raises(ValueError, match="only supported in Python 3.14 or later"): + parser.instantiate(cfg) + + +def test_callable_return_class_var_positional_empty(parser): + parser.add_argument("--fn", type=Callable[[int], CallableVarPositional]) + cfg = parser.parse_args([f"--fn={__name__}.CallableVarPositional"]) + init = parser.instantiate(cfg) + assert isinstance(init.fn, partial) + obj = init.fn(1) + assert (obj.x, obj.layers, obj.k) == (1, (), 0) + + # add_subclass_arguments tests diff --git a/jsonargparse_tests/test_subcommands.py b/jsonargparse_tests/test_subcommands.py index d1f3056e..72b554de 100644 --- a/jsonargparse_tests/test_subcommands.py +++ b/jsonargparse_tests/test_subcommands.py @@ -506,6 +506,25 @@ def test_subsubcommands_wrong_add_order(parser): ctx.match("Multiple levels of subcommands must be added in level order") +def test_add_subcommands_after_variable_nargs_positional(subtests): + for nargs in ["*", "+", "..."]: + with subtests.test(nargs=nargs): + parser = ArgumentParser() + parser.add_argument("files", nargs=nargs) + with pytest.raises(ValueError) as ctx: + parser.add_subcommands() + ctx.match("'files' accepts a variable number of values") + + +def test_add_subcommands_after_single_value_positional(parser, subparser): + parser.add_argument("file") + subcommands = parser.add_subcommands() + subcommands.add_subcommand("cmd", subparser) + cfg = parser.parse_args(["x", "cmd"]) + assert cfg.file == "x" + assert cfg.subcommand == "cmd" + + def test_subcommands_custom_instantiator(parser, subparser, subtests, clear_instantiators): subparser.add_argument("--cls", type=CustomInstantiationBase) subcommands = parser.add_subcommands() diff --git a/sphinx/migrate_v5.rst b/sphinx/migrate_v5.rst index 2817c9b4..ef7f43c3 100644 --- a/sphinx/migrate_v5.rst +++ b/sphinx/migrate_v5.rst @@ -65,6 +65,13 @@ changes: absolute path. - **Configs can no longer import and instantiate anything.** See `Subclass specs and import paths`_ below. +- **Function and method groups are instantiated.** For a group added by + ``add_function_arguments`` or ``add_method_arguments`` with a ``nested_key``, + ``instantiate`` gives a :func:`functools.partial` with the arguments bound, or + an :func:`operator.methodcaller` for a method that is called with an instance, + instead of the parsed namespace. Call it, e.g. ``init.fn()`` instead of + ``fn(**init.fn)``, or give ``instantiate=False`` to keep the namespace. Warns + only with ``JSONARGPARSE_DEPRECATION_WARNINGS=all``. CLI / auto_cli