diff --git a/CHANGELOG.rst b/CHANGELOG.rst index fb34739a..a19aad29 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -21,6 +21,9 @@ Added accept keys that are not in their signature, which are forwarded to the model on instantiation (`#732 `__). +- Support for methods and ``__init__`` defined with ``functools.partialmethod``, + both for adding their parameters and as import paths of callables (`#665 + `__). Changed ^^^^^^^ diff --git a/DOCUMENTATION.rst b/DOCUMENTATION.rst index f403aa9c..1c566b2b 100644 --- a/DOCUMENTATION.rst +++ b/DOCUMENTATION.rst @@ -2255,8 +2255,9 @@ it. Modules commonly import others, e.g. ``import os``, so without this it prevents the object from being used, unlike the check on the given path, which prevents the import from happening at all. An object that has no defining path of its own is denied by the callable it reaches, i.e. the bound function -for a ``functools.partial`` and the defining class for an instance, e.g. -``builtins.help`` is an instance of the ``_sitebuiltins._Helper`` class. +for a ``functools.partial`` or ``partialmethod`` and the defining class for an +instance, e.g. ``builtins.help`` is an instance of the ``_sitebuiltins._Helper`` +class. Entries given are added to the ones denied by default, they don't replace them. For configs that are entirely untrusted, prefer denying everything and allowing diff --git a/jsonargparse/_common.py b/jsonargparse/_common.py index 412de7f7..d9016e09 100644 --- a/jsonargparse/_common.py +++ b/jsonargparse/_common.py @@ -7,6 +7,7 @@ from collections.abc import Callable from contextlib import contextmanager from contextvars import ContextVar +from functools import partialmethod from typing import ( # type: ignore[attr-defined] Generic, TypeVar, @@ -581,6 +582,13 @@ def get_generic_origins(class_or_tuple): return get_generic_origin(class_or_tuple) +def get_partial_method(value) -> partialmethod | None: + """The partialmethod given, or the one that created a method obtained from a class, e.g. ``Class.partial``.""" + if inspect.isfunction(value): + value = getattr(value, "__partialmethod__", getattr(value, "_partialmethod", None)) # python<3.13 + return value if isinstance(value, partialmethod) else None + + def get_unsubscripted_alias_origin(typehint): """Origin class of an unsubscripted typing alias, e.g. typing.List -> list, else None.""" if isinstance(typehint, type) or hasattr(typehint, "__args__"): diff --git a/jsonargparse/_optionals.py b/jsonargparse/_optionals.py index ee399e2f..d4226d67 100644 --- a/jsonargparse/_optionals.py +++ b/jsonargparse/_optionals.py @@ -221,8 +221,13 @@ def get_docstring_parse_options(): def parse_docstring(component, params=False, logger=None): + from ._common import get_partial_method + dp = import_docstring_parser("parse_docstring") options = get_docstring_parse_options() + partial_method = get_partial_method(component) + if partial_method: + component = partial_method.func # documented by the function that it binds try: if params and options["attribute_docstrings"]: return dp.parse_from_object(component, style=options["style"]) @@ -276,7 +281,7 @@ def parse_docs(component, parent, logger): def get_doc_short_description(function_or_class, method_name=None, logger=None): - from ._common import get_generic_origin + from ._common import get_generic_origin, get_partial_method function_or_class = get_generic_origin(function_or_class) # e.g. Strategy[int] documented by Strategy if docstring_parser_support: @@ -287,7 +292,7 @@ def get_doc_short_description(function_or_class, method_name=None, logger=None): if docstring and docstring.short_description: return docstring.short_description init = cls.__dict__.get("__init__") - if init is not None: + if init is not None and not get_partial_method(init): # the class defines its own constructor, so base classes don't describe it docstring = parse_docstring(init, params=False, logger=logger) return docstring.short_description if docstring else None diff --git a/jsonargparse/_parameter_resolvers.py b/jsonargparse/_parameter_resolvers.py index 3dfd054e..cbb0b916 100644 --- a/jsonargparse/_parameter_resolvers.py +++ b/jsonargparse/_parameter_resolvers.py @@ -1,5 +1,6 @@ import ast import dataclasses +import functools import inspect import logging import textwrap @@ -8,7 +9,7 @@ from contextlib import contextmanager, suppress from contextvars import ContextVar from copy import deepcopy -from functools import partial +from functools import partial, partialmethod from importlib import import_module from types import MethodType from typing import Any, Union @@ -105,6 +106,10 @@ def is_method(attr) -> bool: ) +def is_partial_method(attr) -> bool: + return isinstance(attr, partialmethod) + + def is_property(attr) -> bool: return isinstance(attr, property) @@ -449,6 +454,22 @@ def replace_args_and_kwargs(params: ParamList, args: ParamList, kwargs: ParamLis return params +def apply_partial_method(params: ParamList, partial_method: partialmethod) -> ParamList: + """Applies to the parameters the arguments given in a partialmethod, as inspect.signature does.""" + placeholder = getattr(functools, "Placeholder", object()) # python>=3.14 + positionals = [p for p in params if p.kind in {kinds.POSITIONAL_ONLY, kinds.POSITIONAL_OR_KEYWORD}] + given = {p.name for p, arg in zip(positionals, partial_method.args) if arg is not placeholder} + params = [p for p in params if p.name not in given] + to_keyword_only = False + for param in params: + if param.name in partial_method.keywords: + param.default = partial_method.keywords[param.name] + to_keyword_only = True + if to_keyword_only and param.kind == kinds.POSITIONAL_OR_KEYWORD: + param.kind = kinds.KEYWORD_ONLY + return params + + def group_parameters(params_list: list[ParamList]) -> ParamList: if len(params_list) == 1: for param in params_list[0]: @@ -522,13 +543,13 @@ def get_mro_parameters(method_name, get_parameters_fn, logger): remainder = classes[num + 1 :] + [object] if method and not any(method is getattr(c, method_name, None) for c in remainder): current_mro.set((classes, num)) - return get_parameters_fn(cls, method, logger=logger) + return get_parameters_fn(cls, method_name, logger=logger) return [] def get_component_and_parent( function_or_class: Callable | type, - method_or_property: str | Callable | None = None, + method_or_property: str | None = None, ): if is_subclass(function_or_class, ClassFromFunctionBase) and method_or_property in {None, "__init__"}: function_or_class = function_or_class.wrapped_function # type: ignore[union-attr] @@ -539,8 +560,6 @@ def get_component_and_parent( method_or_property = None elif inspect.isclass(get_generic_origin(function_or_class)) and method_or_property is None: method_or_property = "__init__" - elif method_or_property and not isinstance(method_or_property, str): - method_or_property = method_or_property.__name__ parent = component = None if method_or_property: try: @@ -555,6 +574,8 @@ def get_component_and_parent( component = getattr(function_or_class, "__new__") elif is_method(attr): component = attr + elif is_partial_method(attr) and is_method(attr.func): + component = attr.func elif is_property(attr): component = attr.fget elif isinstance(attr, classmethod): @@ -574,7 +595,7 @@ class ParametersVisitor(LoggerProperty, ast.NodeVisitor): def __init__( self, function_or_class: Callable | type, - method_or_property: str | Callable | None = None, + method_or_property: str | None = None, **kwargs, ): super().__init__(**kwargs) @@ -1248,7 +1269,7 @@ def get_signature_parameters( # the generated __new__ of a named tuple doesn't keep the annotations as written, and a # subscripted generic one has no signature, so its parameters come from its fields return get_namedtuple_params(function_or_class, logger, component=function_or_class) - get_component_and_parent(function_or_class, method_or_property) # verify input + component, parent, method_name = get_component_and_parent(function_or_class, method_or_property) params = None for get_parameters in [ get_parameters_from_pydantic_or_attrs, @@ -1269,4 +1290,9 @@ def get_signature_parameters( ) if params is not None: break - return params or [] + params = params or [] + if parent: + attr = inspect.getattr_static(get_generic_origin(parent), method_name) + if is_partial_method(attr) and component is attr.func: + params = apply_partial_method(params, attr) + return params diff --git a/jsonargparse/_util.py b/jsonargparse/_util.py index b02a49cf..43393ce5 100644 --- a/jsonargparse/_util.py +++ b/jsonargparse/_util.py @@ -22,6 +22,7 @@ from ._common import ( check_import_path, get_generic_origin, + get_partial_method, parser_capture, parser_context, ) @@ -248,6 +249,9 @@ def canonical_import_paths(obj) -> set: stack = [obj] while stack: current = stack.pop() + partial_method = get_partial_method(current) + if partial_method: + current = partial_method.func # a method from a partialmethod is denied by the callable it binds canonical = canonical_import_path(current) if canonical: paths.add(canonical) @@ -276,6 +280,16 @@ def register_unresolvable_import_paths(*modules: ModuleType): unresolvable_import_paths[val] = f"{module.__name__}.{val.__name__}" +def get_partial_method_path(partial_method: functools.partialmethod) -> str: + """Import path of a partialmethod, found in the classes of the module that defines the function it binds.""" + module = import_module(partial_method.func.__module__) + for cls in [v for v in vars(module).values() if inspect.isclass(v)]: + name = next((k for k, v in vars(cls).items() if v is partial_method), None) + if name: + return f"{get_import_path(cls)}.{name}" + raise ValueError(f"Not possible to determine the import path for partialmethod {partial_method}.") + + def get_module_var_path(module_path: str, value: Any) -> str | None: module = import_module(module_path) for name, var in vars(module).items(): @@ -320,6 +334,9 @@ def get_import_path(value: Any) -> str | None: remembered = resolved_import_paths.get(value) if remembered: return remembered + partial_method = get_partial_method(value) + if partial_method: + return get_partial_method_path(partial_method) path = None value = get_generic_origin(value) if hasattr(value, "__self__") and inspect.isclass(value.__self__) and inspect.ismethod(value): @@ -367,7 +384,7 @@ def object_path_serializer(value): try: path = get_import_path(value) reimported = import_object(path, check_path=False) - if value is not reimported: + if (get_partial_method(value) or value) is not (get_partial_method(reimported) or reimported): raise ValueError return path except Exception as ex: diff --git a/jsonargparse_tests/test_import_paths.py b/jsonargparse_tests/test_import_paths.py index 6dd0dae5..4232b1f7 100644 --- a/jsonargparse_tests/test_import_paths.py +++ b/jsonargparse_tests/test_import_paths.py @@ -260,6 +260,18 @@ def test_denied_callable_bound_by_a_partial(): import_object(f"{__name__}.system_partial") # partial bound to os.system, defined in posix, nt on Windows +def test_denied_callable_bound_by_a_partialmethod(): + set_parsing_settings(import_path_denylist=[]) + with pytest.raises(ImportDenied, match=f"'{os.system.__module__}'"): + import_object(f"{__name__}.WithSystemPartialMethod.system") # partialmethod of os.system + + +def test_partialmethod_allowed(): + set_parsing_settings(import_path_denylist=[]) + method = import_object(f"{__name__}.WithPartialMethod.partial_method") + assert method(WithPartialMethod()) == "given" + + def test_denied_callable_exposed_by_an_instance(): set_parsing_settings(import_path_denylist=[]) with pytest.raises(ImportDenied, match="'operator'"): @@ -297,6 +309,17 @@ def test_import_object_unaffected_when_allowed(): attr_getter = operator.attrgetter("__globals__") # instance of the denied operator.attrgetter +class WithSystemPartialMethod: + system = functools.partialmethod(os.system, "echo test") # binds a denied callable, defined in posix + + +class WithPartialMethod: + def method(self, value: str): + return value + + partial_method = functools.partialmethod(method, "given") + + def no_module_function(): """Mimics extension functions that have __module__ set to None.""" diff --git a/jsonargparse_tests/test_parameter_resolvers.py b/jsonargparse_tests/test_parameter_resolvers.py index 91245523..65761010 100644 --- a/jsonargparse_tests/test_parameter_resolvers.py +++ b/jsonargparse_tests/test_parameter_resolvers.py @@ -2,7 +2,9 @@ import calendar import inspect +import sys import xml.dom +from functools import partialmethod from random import shuffle from typing import Any, Callable, Dict, List, Optional, Protocol, Union from unittest.mock import patch @@ -37,6 +39,8 @@ def method_a(self, pma1: int, pma2: float, kma1: str = "x"): kma1: help for kma1 """ + partial_method_a = partialmethod(method_a, pma1=1, pma2=0.5) + class ClassB(ClassA): def __init__(self, pkb1: str, kb1: int = 3, kb2: str = "4", **kwargs): @@ -96,6 +100,8 @@ def method_d(self, pmd1: int, *args, kmd1: int = 2, **kws): """ return super().method_a(*args, **kws) # pragma: no cover + partial_method_d = partialmethod(method_d, 3, kma1="y") + @staticmethod def staticmethod_d(ksmd1: str = "z", **kw): """ @@ -836,6 +842,64 @@ def test_get_params_classmethod_instantiate_from_cls(): assert_params(get_params(ClassS1, "classmethod_s"), []) +# partialmethod parameters tests + + +def test_get_params_partialmethod_keywords(): + params = get_params(ClassA, "partial_method_a") + assert_params(params, ["pma1", "pma2", "kma1"]) + assert [p.default for p in params] == [1, 0.5, "x"] + signature = list(inspect.signature(ClassA.partial_method_a).parameters.values())[1:] + assert [(p.name, p.kind, p.default) for p in params] == [(p.name, p.kind, p.default) for p in signature] + with source_unavailable(): + assert params == get_params(ClassA, "partial_method_a") + + +def test_get_params_partialmethod_positional_and_forwarded_kwargs(): + params = get_params(ClassD, "partial_method_d") + assert_params(params, ["kmd1", "pma1", "pma2", "kma1"]) + assert params[-1].default == "y" + with source_unavailable(): + assert_params(get_params(ClassD, "partial_method_d"), ["kmd1"]) + + +class ClassPartialInit(ClassB): + __init__ = partialmethod(ClassB.__init__, "p", kb1=5) + + +def test_get_params_partialmethod_init(): + params = get_params(ClassPartialInit) + assert_params(params, ["kb1", "kb2", "ka1"]) + assert [p.default for p in params] == [5, "4", 1.2] + with source_unavailable(): + assert_params(get_params(ClassPartialInit), ["kb1", "kb2", "ka1", "ka2"]) + + +class ClassPartialInitChild(ClassPartialInit): + def __init__(self, kpc1: int = 0, **kwargs): + """ + Args: + kpc1: help for kpc1 + """ + super().__init__(**kwargs) # pragma: no cover + + +def test_get_params_partialmethod_init_from_super(): + params = get_params(ClassPartialInitChild) + assert_params(params, ["kpc1", "kb1", "kb2", "ka1"]) + assert [p.default for p in params] == [0, 5, "4", 1.2] + + +@pytest.mark.skipif(sys.version_info < (3, 14), reason="functools.Placeholder introduced in python 3.14") +def test_get_params_partialmethod_placeholder(): + from functools import Placeholder + + class ClassPlaceholder(ClassA): + placeholder_method = partialmethod(ClassA.method_a, Placeholder, 0.5) + + assert_params(get_params(ClassPlaceholder, "placeholder_method"), ["pma1", "kma1"]) + + # function method parameters tests diff --git a/jsonargparse_tests/test_signatures.py b/jsonargparse_tests/test_signatures.py index 9c24c1b3..687a7943 100644 --- a/jsonargparse_tests/test_signatures.py +++ b/jsonargparse_tests/test_signatures.py @@ -321,6 +321,18 @@ def test_add_class_with_required_parameters(parser): assert cfg.model == Namespace(m=0.1, n=3) +class WithPartialInit(RequiredParams): + __init__ = functools.partialmethod(RequiredParams.__init__, 1, m=0.5) + + +def test_add_class_partialmethod_init(parser): + parser.add_class_arguments(WithPartialInit, "a") + cfg = parser.parse_args([]) + assert cfg.a == Namespace(m=0.5) + init = parser.instantiate(parser.parse_args(["--a.m=0.7"])) + assert (init.a.n, init.a.m) == (1, 0.7) + + def test_add_class_conditional_kwargs(parser): from jsonargparse_tests.test_parameter_resolvers import ClassG @@ -480,6 +492,18 @@ def test_add_class_group_description_from_base(parser): assert "b1 description" in help_str +class WithPartialInitDocstring(WithDocstringBase): + __init__ = functools.partialmethod(WithDocstringBase.__init__, b1=2) + + +@skip_if_docstring_parser_unavailable +def test_add_class_group_description_partialmethod_init(parser): + parser.add_class_arguments(WithPartialInitDocstring, "w") + help_str = get_parser_help(parser) + assert "WithDocstringBase short description:" in help_str + assert "b1 description (type: int, default: 2)" in help_str + + def test_add_class_custom_instantiator(parser, clear_instantiators): def instantiate(cls, **kwargs): instance = cls(**kwargs) @@ -691,6 +715,21 @@ def test_add_method_normal_and_static(parser): assert f"{key.split('.')[1]} description" != find_action(parser, key).help +class WithPartialMethod(WithMethods): + partial_method = functools.partialmethod(WithMethods.normal_method, a2=3.0) + + +def test_add_method_partialmethod(parser): + added_args = parser.add_method_arguments(WithPartialMethod, "partial_method", "m") + assert added_args == ["m.a1", "m.a2", "m.a3"] + assert parser.get_defaults().m == Namespace(a1="1", a2=3.0, a3=False) + cfg = parser.parse_args(["--m.a1=x"]) + assert "x" == WithPartialMethod().partial_method(**cfg.m) + if docstring_parser_support: + assert "normal_method short description" == parser.groups["m"].title + assert "a2 description" == find_action(parser, "m.a2").help + + class SubWithMethod(WithMethods): def normal_method(self, *args, p2: int = 2, **kwargs): # pragma: no cover p1 = super().normal_method(**kwargs) diff --git a/jsonargparse_tests/test_subclasses.py b/jsonargparse_tests/test_subclasses.py index 9e7f50eb..2c820208 100644 --- a/jsonargparse_tests/test_subclasses.py +++ b/jsonargparse_tests/test_subclasses.py @@ -8,7 +8,7 @@ from calendar import Calendar from copy import deepcopy from dataclasses import dataclass -from functools import partial +from functools import partial, partialmethod from gzip import GzipFile from pathlib import Path from typing import ( @@ -109,6 +109,19 @@ def test_subclass_defaults(parser): assert cfg.cls.init_args.param == "sub_default" +class SubClassPartialInit(BaseClassDefault): + __init__ = partialmethod(BaseClassDefault.__init__, param="partial_default") + + +def test_subclass_partialmethod_init(parser): + parser.add_argument("--cls", type=BaseClassDefault) + cfg = parser.parse_args(["--cls=SubClassPartialInit"]) + assert cfg.cls.init_args.param == "partial_default" + init = parser.instantiate(cfg) + assert isinstance(init.cls, SubClassPartialInit) + assert init.cls.param == "partial_default" + + def test_subclass_init_args_in_subcommand(parser, subparser): subparser.add_subclass_arguments(BaseC, "obj", default=lazy_instance(BaseC)) subcommands = parser.add_subcommands() diff --git a/jsonargparse_tests/test_typehints.py b/jsonargparse_tests/test_typehints.py index ff35ef9d..7736d317 100644 --- a/jsonargparse_tests/test_typehints.py +++ b/jsonargparse_tests/test_typehints.py @@ -13,6 +13,7 @@ from dataclasses import dataclass, field from datetime import date from enum import Enum +from functools import partialmethod from pathlib import Path from textwrap import dedent from types import GenericAlias, MappingProxyType, ModuleType, UnionType @@ -2856,6 +2857,26 @@ def test_callable_function_path(parser): ctx.match("Callable expects a function or a callable class") +class WithPartialMethod: + def method(self, value: int = 1): + return value + + partial_method = partialmethod(method, value=2) + + +def test_callable_partialmethod_path(parser): + path = f"{__name__}.WithPartialMethod.partial_method" + parser.add_argument("--config", action="config") + parser.add_argument("--callable", type=Callable, default=WithPartialMethod.partial_method) + + out = get_parse_args_stdout(parser, ["--print_config"]) + assert json_or_yaml_load(out) == {"callable": path} + + cfg = parser.parse_args([f"--config={out}"]) + assert cfg.callable(WithPartialMethod()) == 2 + assert json_or_yaml_load(parser.dump(cfg)) == {"callable": path} + + def make_closure_callable(): def unbound_closure(): return "closure" # pragma: no cover diff --git a/jsonargparse_tests/test_util.py b/jsonargparse_tests/test_util.py index 62eb9bdf..467dfb67 100644 --- a/jsonargparse_tests/test_util.py +++ b/jsonargparse_tests/test_util.py @@ -1,7 +1,9 @@ from __future__ import annotations +import calendar import logging import os +from functools import partialmethod from importlib import import_module from unittest.mock import patch @@ -180,6 +182,25 @@ def test_get_import_path_classpath_inheritance(): assert get_import_path(ChildClassmethod.class_method) == f"{__name__}.ChildClassmethod.class_method" +class WithPartialMethod: + def method(self, p1: int, p2: str = "x"): + pass # pragma: no cover + + partial_method = partialmethod(method, p2="y") + other_module_partial_method = partialmethod(calendar.Calendar.getfirstweekday) + + +def test_get_import_path_partialmethod(): + path = f"{__name__}.WithPartialMethod.partial_method" + assert get_import_path(WithPartialMethod.partial_method) == path + assert object_path_serializer(WithPartialMethod.partial_method) == path + + +def test_get_import_path_partialmethod_not_found(): + with pytest.raises(ValueError, match="Not possible to determine the import path"): + get_import_path(WithPartialMethod.other_module_partial_method) + + def unresolvable_import(): pass # pragma: no cover