diff --git a/CHANGELOG.rst b/CHANGELOG.rst index fb1ab783..cbef856d 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -18,10 +18,14 @@ v4.51.0 (unreleased) Added ^^^^^ - ``validate_subclass_spec_in_any`` setting in ``set_parsing_settings`` so that - when a value for an ``Any`` typed argument looks like a subclass spec but is - not a valid one, the parsing fails instead of silently ignoring it. When - disabled (the default), a debug log now informs about the ignored invalid - subclass spec (`#938 `__). + when a value looks like a subclass spec but is not a valid one, the parsing + fails instead of silently ignoring it. Applies to types that accept any value, + i.e. ``Any``, ``Unvalidated<...>`` and dicts that don't validate their values, + e.g. ``dict[str, Any]``. For dicts the spec is only validated, since the value + is kept as a dict. When disabled (the default), a debug log now informs about + the ignored invalid subclass spec (`#938 + `__, `#953 + `__). - Items of a list of classes and values of a dict of classes can now be given as paths to sub-config files, instead of this only being supported for the value of an entire argument (`#940 @@ -47,6 +51,10 @@ Added - ``shtab`` completion scripts now include the fields of a dataclass-like type that is not added as a group, e.g. ``Optional[SomeDataclass]`` (`#952 `__). +- A ``TypeVar`` used as the type itself, i.e. not only as the subtype of a + ``type[...]``, is now replaced by what it stands for: its PEP 696 ``default``, + its constraints or its bound. Previously the value was accepted without any + validation (`#953 `__). Fixed ^^^^^ @@ -123,6 +131,25 @@ Fixed argument of a subcommand, e.g. ``APP_SUB__DATA='{"p1": 2}'`` failing with ``Not a valid subclass of ...`` (`#952 `__). +- ``dump``, and thus ``--print_config``, not being able to serialize a value + that was given as an import path to an instance, unless the instance happened + to be defined in the module of its class. Instead of the import path it wrote + a message saying that the instance was not serializable, making the dump not + round-trippable. Now the import path that a value was resolved from is + remembered and dumped back (`#953 + `__). +- Secrets round-tripping through ``--print_config`` as the mask ``**********``, + silently making the mask the actual secret. Now parsing the mask as a + ``SecretStr``, both jsonargparse's and pydantic's, fails (`#953 + `__). +- A generic ``Protocol`` never being implementable, since the ``TypeVar`` of the + protocol and the type in the implementation could never be equal. Now a + ``TypeVar`` in either of them matches any type, as static type checkers do + (`#953 `__). +- Classes having no parameters at all when a decorator wraps ``__new__``, e.g. + decorators that mark a class as deprecated or experimental. The parameters + were resolved from the wrapper instead of from ``__init__`` (`#953 + `__). Changed ^^^^^^^ diff --git a/DOCUMENTATION.rst b/DOCUMENTATION.rst index fc1daf77..e16da2ba 100644 --- a/DOCUMENTATION.rst +++ b/DOCUMENTATION.rst @@ -584,7 +584,10 @@ Some notes about this support are: - ``pydantic.SecretStr`` type is supported with the expected behavior of not serializing the actual value. There is also ``jsonargparse.typing.SecretStr`` - to support the same behavior without the need of a dependency. + to support the same behavior without the need of a dependency. Since dumps + only have the mask ``**********`` instead of the actual secret, parsing this + mask as a secret fails, so that a config bootstrapped with ``--print_config`` + is not used with the mask as the secret. - ``pydantic.FilePath`` and ``pydantic.DirectoryPath`` types are supported, running the corresponding pydantic validation when parsing. Arguments with @@ -1187,6 +1190,12 @@ Parsing complex-valued points would be: >>> parser.parse_args(["--point.x=(1+2j)"]).point Namespace(x=(1+2j), y=0.0) +A ``TypeVar`` can't be used to validate, so when it is used as a type, e.g. +``options: Optional[OptionsT] = None``, it is replaced by what it stands for: +its PEP 696 ``default``, its constraints or its bound, in that order. When it +has none of these, the value is accepted without validation and the help shows +it as ``Unvalidated<...>``. + .. _callable-type: @@ -2250,11 +2259,16 @@ be accepted. In this case the config would be like: ``class_path`` and ``init_args`` if the corresponding parameter has type ``Any``, or when ``fail_untyped=False`` which defaults to type ``Any``. - If such a value looks like a subclass spec (has a ``class_path``) but cannot - be parsed as one, e.g. because the class fails to import, by default it is - left unchanged and a debug message is logged. Set + If a value looks like a subclass spec (has a ``class_path``) but cannot be + parsed as one, e.g. because the class fails to import, by default it is left + unchanged and a debug message is logged. Set ``validate_subclass_spec_in_any=True`` in :func:`.set_parsing_settings` to - make the parsing fail in this case instead. + make the parsing fail instead. Apart from ``Any`` and ``Unvalidated<...>``, + this also applies to dicts that don't validate their values, e.g. + ``dict[str, Any]``. For dicts the spec is only validated, since the value is + kept as a dict, which matters for unions such as ``Union[SomeClass, + dict[str, Any]]``, where a spec rejected by the class member would otherwise + be silently swallowed by the dict member. .. note:: diff --git a/jsonargparse/_common.py b/jsonargparse/_common.py index 0deabc15..aefbb24a 100644 --- a/jsonargparse/_common.py +++ b/jsonargparse/_common.py @@ -158,12 +158,14 @@ def set_parsing_settings( validate_defaults: Whether default values must be valid according to the argument type. The default is ``False``, meaning no default validation, like in argparse. - validate_subclass_spec_in_any: If ``True``, when a value for an ``Any`` - typed argument looks like a subclass spec (i.e. a dict with a - ``class_path`` key) it is required to be a valid one, otherwise the - parsing fails. By default, this is ``False``, meaning that an - invalid subclass spec is ignored (a debug log is emitted) and the - original value is kept. + validate_subclass_spec_in_any: If ``True``, when a value for a type that + accepts any value, i.e. ``Any``, ``Unvalidated<...>`` or a dict that + doesn't validate its values, looks like a subclass spec (i.e. a dict + with a ``class_path`` key), it is required to be a valid one, + otherwise the parsing fails. For dicts the spec is only validated, + since the value is kept as a dict. By default, this is ``False``, + meaning that an invalid subclass spec is ignored (a debug log is + emitted) and the original value is kept. config_read_mode_urls_enabled: Whether to read config files from URLs using requests package. Default is ``False``. config_read_mode_fsspec_enabled: Whether to read config files from diff --git a/jsonargparse/_core.py b/jsonargparse/_core.py index 3ccbcf6a..3f09ad16 100644 --- a/jsonargparse/_core.py +++ b/jsonargparse/_core.py @@ -95,6 +95,7 @@ from ._typehints import ( ActionTypeHint, is_subclass_spec, + replace_type_vars, strip_required_typehint, subclasses_disabled_remove_class_path, ) @@ -154,6 +155,7 @@ def add_argument(self, *args, sub_configs: bool = False, **kwargs): else: is_required = bool(kwargs.get("required", False)) kwargs["type"] = strip_required_typehint(kwargs["type"], is_required, f'"{arg_name}"') + kwargs["type"] = replace_type_vars(kwargs["type"]) if is_subclasses_disabled(kwargs["type"]): nested_key = args[0].lstrip("-") self.add_class_arguments(kwargs.pop("type"), nested_key, sub_configs=sub_configs, **kwargs) diff --git a/jsonargparse/_parameter_resolvers.py b/jsonargparse/_parameter_resolvers.py index 087e60a1..ebfa7298 100644 --- a/jsonargparse/_parameter_resolvers.py +++ b/jsonargparse/_parameter_resolvers.py @@ -459,10 +459,13 @@ def group_parameters(params_list: list[ParamList]) -> ParamList: def has_dunder_new_method(cls, attr_name): classes = inspect.getmro(get_generic_origin(cls))[1:] + # unwrapped, since a decorator that wraps __new__, e.g. to mark a class as + # deprecated or experimental, doesn't change the parameters that it accepts + dunder_new = inspect.unwrap(cls.__new__) return ( attr_name == "__init__" - and cls.__new__ is not object.__new__ - and not any(cls.__new__ is c.__new__ for c in classes) + and dunder_new is not object.__new__ + and not any(dunder_new is inspect.unwrap(c.__new__) for c in classes) ) diff --git a/jsonargparse/_signatures.py b/jsonargparse/_signatures.py index 1177be1f..4c985bc6 100644 --- a/jsonargparse/_signatures.py +++ b/jsonargparse/_signatures.py @@ -32,6 +32,7 @@ is_optional, is_subclass_container_typehint, not_required_types, + replace_type_vars, replace_unvalidatable_typehints, sequence_origin_types, strip_required_typehint, @@ -350,7 +351,8 @@ def _add_signature_parameter( return unvalidated: list = [] try: - register_pydantic_types(annotation) # before the check of what can be validated + annotation = replace_type_vars(annotation) # before the check of what can be validated + register_pydantic_types(annotation) unvalidatable_replaced = replace_unvalidatable_typehints(annotation, unvalidated) except Exception as ex: raise ValueError(f'Unable to add parameter "{name}" from "{src}": {ex}') from ex diff --git a/jsonargparse/_typehints.py b/jsonargparse/_typehints.py index fa57c432..85684ff7 100644 --- a/jsonargparse/_typehints.py +++ b/jsonargparse/_typehints.py @@ -345,6 +345,7 @@ def __init__(self, typehint: type | None = None, enable_path: bool = False, **kw ValueError: If a parameter is invalid. """ if typehint is not None: + typehint = replace_type_vars(typehint) if not self.is_supported_typehint(typehint, full=True): raise ValueError(f"Unsupported type hint {typehint}.") if get_typehint_origin(typehint) == Union: @@ -358,7 +359,7 @@ def __init__(self, typehint: type | None = None, enable_path: bool = False, **kw kwargs["logger"].debug(f"Discarding unsupported subtypes {discard} from {typehint}") subtypes = tuple(t for t, s in zip(typehint.__args__, subtype_supported) if s) typehint = Union[subtypes] - self._typehint = sort_unions_in_typehint(replace_type_vars_in_type_subtype(typehint)) + self._typehint = sort_unions_in_typehint(typehint) self._enable_path = False if is_pathlike(typehint) else enable_path elif "_typehint" not in kwargs: raise ValueError("Expected typehint keyword argument.") @@ -1184,7 +1185,7 @@ def adapt_typehints( elif isinstance(val, str): with suppress(*get_loader_exceptions()): val, _ = parse_value_or_config(val, enable_path=False, simple_types=True) - val = adapt_classes_any(val, serialize, instantiate_classes, sub_add_kwargs, logger) + val = adapt_classes_any(val, typehint, serialize, instantiate_classes, sub_add_kwargs, logger) if serialize: val = serialize_unvalidated(val) @@ -1360,6 +1361,8 @@ def adapt_typehints( # Dict, Mapping elif typehint_origin in mapping_origin_types: + if not serialize and not instantiate_classes: + validate_subclass_spec_in_mapping(val, typehint, subtypehints, sub_add_kwargs, logger) if isinstance(val, NestedArg): if isinstance(prev_val, dict): if isinstance(val.key, str) and "." in val.key: @@ -1625,13 +1628,44 @@ def get_protocol_method_signature(class_type, name, logger): return (params[1:] if skip_self else params), get_return_type(method, logger) +def type_var_wildcard_matches(proto_annotation, value_annotation) -> bool: + """Whether two types are equal, a TypeVar in either of them matching any type. + + A generic protocol is written in terms of its own TypeVars and an + implementation in terms of its own or of concrete types, so a TypeVar is + compared as a wildcard, like a static type checker does. Otherwise a generic + protocol could never be implemented. + """ + if isinstance(proto_annotation, TypeVar) or isinstance(value_annotation, TypeVar): + return True + if proto_annotation == value_annotation: + return True + proto_origin = get_typehint_origin(proto_annotation) + if proto_origin is None or proto_origin != get_typehint_origin(value_annotation): + return False + proto_args = getattr(proto_annotation, "__args__", None) + value_args = getattr(value_annotation, "__args__", None) + if not isinstance(proto_args, tuple) or not isinstance(value_args, tuple) or len(proto_args) != len(value_args): + return False + if proto_origin is Union: + # the subtypes of a union are unordered, so each one must match some unmatched other one + unmatched = list(value_args) + for proto_arg in proto_args: + match = next((v for v in unmatched if type_var_wildcard_matches(proto_arg, v)), None) + if match is None: + return False + unmatched.remove(match) + return True + return all(type_var_wildcard_matches(p, v) for p, v in zip(proto_args, value_args)) + + def protocol_type_matches(proto_annotation, value_annotation, value_any_accepted: bool = False) -> bool: """Whether a type in an implementation is accepted for the corresponding type in a protocol.""" if proto_annotation is inspect.Parameter.empty or proto_annotation == Any: return True if value_any_accepted and (value_annotation is inspect.Parameter.empty or value_annotation == Any): return True - return proto_annotation == value_annotation + return type_var_wildcard_matches(proto_annotation, value_annotation) def protocol_var_param_matches(proto_param, value_var_param) -> bool: @@ -2160,32 +2194,53 @@ def subclasses_disabled_remove_class_path(value): return value -def adapt_classes_any(val, serialize, instantiate_classes, sub_add_kwargs, logger=None): +def adapt_classes_any(val, typehint, serialize, instantiate_classes, sub_add_kwargs, logger=None): if is_subclass_spec(val): orig_val = val val = subclass_spec_as_namespace(val) init_args = val.get("init_args") if init_args and not instantiate_classes: for subkey, subval in init_args.items(branches=True, nested=False): - init_args[subkey] = adapt_classes_any(subval, serialize, instantiate_classes, sub_add_kwargs, logger) + init_args[subkey] = adapt_classes_any( + subval, typehint, serialize, instantiate_classes, sub_add_kwargs, logger + ) val["init_args"] = init_args try: val = adapt_class_type(val, serialize, instantiate_classes, sub_add_kwargs) except Exception as ex: + type_str = type_to_str(typehint) if get_parsing_setting("validate_subclass_spec_in_any"): - raise ValueError(f"Invalid subclass spec given as value for an Any type: {ex}") from ex + raise ValueError(f"Invalid subclass spec given as value for type {type_str}: {ex}") from ex if logger: - logger.debug(f"Ignoring invalid subclass spec given as value for an Any type: {ex}", exc_info=ex) + logger.debug(f"Ignoring invalid subclass spec given as value for type {type_str}: {ex}", exc_info=ex) return orig_val elif isinstance(val, list): for num, subval in enumerate(val): - val[num] = adapt_classes_any(subval, serialize, instantiate_classes, sub_add_kwargs, logger) + val[num] = adapt_classes_any(subval, typehint, serialize, instantiate_classes, sub_add_kwargs, logger) elif isinstance(val, dict): for key, subval in val.items(): - val[key] = adapt_classes_any(subval, serialize, instantiate_classes, sub_add_kwargs, logger) + val[key] = adapt_classes_any(subval, typehint, serialize, instantiate_classes, sub_add_kwargs, logger) return val +def validate_subclass_spec_in_mapping(val, typehint, subtypehints, sub_add_kwargs, logger) -> None: + """Raises if the value of a mapping that doesn't validate its values is an invalid subclass spec. + + Only done when the ``validate_subclass_spec_in_any`` setting is enabled. + Unlike for ``Any``, the value is only validated and kept as a mapping, since + an instance would not correspond to the type. Building the class is the + responsibility of a class type, e.g. a member of the union that the mapping + is part of. + """ + if not get_parsing_setting("validate_subclass_spec_in_any") or not is_subclass_spec(val): + return + if type(typehint) in typed_dict_meta_types: + return + if subtypehints is not None and not (subtypehints[1] == Any or isinstance(subtypehints[1], UnvalidatedType)): + return + adapt_classes_any(deepcopy(val), typehint, False, False, sub_add_kwargs, logger) + + def union_subtype_sort_key(subtype) -> int: """Rank of a union subtype, sorted by which is attempted first when parsing. @@ -2203,14 +2258,42 @@ def union_subtype_sort_key(subtype) -> int: return 0 -def replace_type_vars_in_type_subtype(typehint): - """Returns the type hint with the TypeVar subtype of all its type[...] replaced, including nested ones. +def get_type_var_default(type_var): + """Returns the PEP 696 default of a TypeVar, or None when it has none.""" + if getattr(type_var, "has_default", lambda: False)(): + default = type_var.__default__ + if default is not None: + return default + return None + + +def replace_type_var(type_var, in_type_subtype: bool): + """Returns the type that a TypeVar stands for, or the TypeVar when there is none. + + What a TypeVar stands for is given by its PEP 696 default, its constraints + or its bound. As the subtype of a ``type[...]`` it additionally stands for + ``object``, i.e. any class, since there a TypeVar is always a class. + """ + default = get_type_var_default(type_var) + if default is not None: + return default + if type_var.__constraints__: + return Union[type_var.__constraints__] + if type_var.__bound__: + return type_var.__bound__ + return object if in_type_subtype else type_var + + +def replace_type_vars(typehint): + """Returns the type hint with all its TypeVars replaced, including nested ones. Done when an argument is added, since a TypeVar can't be used to validate. - What a TypeVar stands for is given by its bound or its constraints, and - ``object`` when it has neither, i.e. any class. The help then shows what is - accepted, e.g. ``type[object]`` instead of ``type[~T]``. + The help then shows what is accepted, e.g. ``type[object]`` instead of + ``type[~T]``. A TypeVar that stands for nothing is left as is, so that it + becomes an ``Unvalidated<...>``. """ + if isinstance(typehint, TypeVar): + return replace_type_var(typehint, in_type_subtype=False) if get_typehint_origin(typehint) in literal_types: return typehint # the args of a Literal are values, not types args = getattr(typehint, "__args__", None) @@ -2219,12 +2302,9 @@ def replace_type_vars_in_type_subtype(typehint): if not isinstance(args, tuple) or not args: return typehint if get_typehint_origin(typehint) in {Type, type} and isinstance(args[0], TypeVar): - if args[0].__constraints__: - new_args = (Union[args[0].__constraints__],) - else: - new_args = (args[0].__bound__ or object,) + new_args = (replace_type_var(args[0], in_type_subtype=True),) else: - new_args = tuple(replace_type_vars_in_type_subtype(a) for a in args) + new_args = tuple(replace_type_vars(a) for a in args) if all(new is old for new, old in zip(new_args, args)): return typehint return rebuild_typehint_args(typehint, new_args) diff --git a/jsonargparse/_util.py b/jsonargparse/_util.py index 51ede66f..18a41633 100644 --- a/jsonargparse/_util.py +++ b/jsonargparse/_util.py @@ -4,6 +4,7 @@ import os import textwrap import warnings +import weakref from argparse import ArgumentError from collections import namedtuple from collections.abc import Callable, Iterator @@ -197,7 +198,11 @@ def import_object(name: str): raise ex name_module, name_object1 = name_module.rsplit(".", 1) parent = getattr(__import__(name_module, fromlist=[name_object1]), name_object1) - return getattr(parent, name_object) + obj = getattr(parent, name_object) + if not (inspect.isclass(obj) or inspect.ismodule(obj)): + # an instance doesn't know where it was imported from, so it is remembered to make it serializable + resolved_import_paths.add(obj, name) + return obj unresolvable_import_paths: dict[Any, str] = {} @@ -226,8 +231,42 @@ def get_module_var_path(module_path: str, value: Any) -> str | None: return None +class ResolvedImportPaths: + """Remembers the import path that instances were resolved from. + + The import path of an instance can't be derived from the object itself, so + without this a value given as an import path to an instance would not be + serializable, even though it came from an import path. Entries are keyed by + id, weak references being used to discard an entry when its instance is + garbage collected, thus avoiding that an id is reused for another object. + """ + + def __init__(self) -> None: + self._paths: dict[int, tuple[Any, str]] = {} + + def add(self, instance: Any, import_path: str) -> None: + key = id(instance) + try: + ref = weakref.ref(instance, lambda _: self._paths.pop(key, None)) + except TypeError: + return # instance doesn't support weak references + self._paths[key] = (ref, import_path) + + def get(self, instance: Any) -> str | None: + entry = self._paths.get(id(instance)) + if entry and entry[0]() is instance: + return entry[1] + return None + + +resolved_import_paths = ResolvedImportPaths() + + def get_import_path(value: Any) -> str | None: """Returns the shortest dot import path for the given object.""" + remembered = resolved_import_paths.get(value) + if remembered: + return remembered path = None value = get_generic_origin(value) if hasattr(value, "__self__") and inspect.isclass(value.__self__) and inspect.ismethod(value): diff --git a/jsonargparse/typing.py b/jsonargparse/typing.py index fca2ffeb..f19ef029 100644 --- a/jsonargparse/typing.py +++ b/jsonargparse/typing.py @@ -648,6 +648,20 @@ def range_deserializer(value): register_type(range, serializer=range_serializer, deserializer=range_deserializer) +secret_str_mask = "**********" + + +def fail_if_secret_str_mask(value) -> None: + """Fails when the value is the mask that secrets serialize to. + + Dumps of secrets don't include the actual value, so parsing back a dump + would silently give the mask as the secret. Better to fail so that the mask + is replaced by the real secret. + """ + if value == secret_str_mask: + raise ValueError(f"Refusing to parse the mask {secret_str_mask!r} as a secret, give the actual value instead") + + class SecretStr: """Holds a secret string that serializes to ``**********``.""" @@ -655,7 +669,7 @@ def __init__(self, value: str): self._value = value def __str__(self) -> str: - return "**********" + return secret_str_mask def __len__(self) -> int: return len(self._value) @@ -671,8 +685,20 @@ def get_secret_value(self) -> str: return self._value -register_type(SecretStr) -register_type_on_first_use("pydantic.SecretStr") +def secret_str_deserializer(value) -> SecretStr: + fail_if_secret_str_mask(value) + return SecretStr(value) + + +def pydantic_secret_str_deserializer(value): + from pydantic import SecretStr as PydanticSecretStr + + fail_if_secret_str_mask(value) + return PydanticSecretStr(value) + + +register_type(SecretStr, deserializer=secret_str_deserializer) +register_type_on_first_use("pydantic.SecretStr", deserializer=pydantic_secret_str_deserializer) def pydantic_deserializer(class_type): diff --git a/jsonargparse_tests/test_parsing_settings.py b/jsonargparse_tests/test_parsing_settings.py index 6be4684f..bc9bf9da 100644 --- a/jsonargparse_tests/test_parsing_settings.py +++ b/jsonargparse_tests/test_parsing_settings.py @@ -1,12 +1,14 @@ +import json import re from dataclasses import dataclass -from typing import Any, List, Literal, Optional +from typing import Any, Dict, List, Literal, Optional, TypedDict, Union from unittest.mock import patch import pytest from jsonargparse import SUPPRESS, ActionYesNo, ArgumentError, Namespace, Unset, set_parsing_settings from jsonargparse._common import _UnsetType, get_parsing_setting +from jsonargparse._typehints import UnvalidatedType from jsonargparse_tests.conftest import capture_logs, get_parse_args_stdout, get_parser_help, json_or_yaml_load from jsonargparse_tests.test_typehints import Optimizer @@ -489,14 +491,14 @@ def test_validate_subclass_spec_in_any_disabled_ignored_with_debug_log(parser, l with capture_logs(logger) as logs: cfg = parser.parse_args(['--any={"class_path": "nonexistent.Foo"}']) assert cfg.any == {"class_path": "nonexistent.Foo"} - assert "Ignoring invalid subclass spec given as value for an Any type" in logs.getvalue() + assert "Ignoring invalid subclass spec given as value for type Any" in logs.getvalue() def test_validate_subclass_spec_in_any_enabled_fails(parser): set_parsing_settings(validate_subclass_spec_in_any=True) parser.add_argument("--any", type=Any) - with pytest.raises(ArgumentError, match="Invalid subclass spec given as value for an Any type"): + with pytest.raises(ArgumentError, match="Invalid subclass spec given as value for type Any"): parser.parse_args(['--any={"class_path": "nonexistent.Foo"}']) @@ -514,3 +516,97 @@ def test_validate_subclass_spec_in_any_enabled_non_subclass_dict_kept(parser): cfg = parser.parse_args(['--any={"a": 0, "b": 1}']) assert cfg.any == {"a": 0, "b": 1} + + +# validate_subclass_spec_in_any for dict types that accept any value + +unvalidated_dict_type = Dict[str, UnvalidatedType("some.SomeType")] # type: ignore[misc,valid-type] +any_dict_types = [dict, Dict, Dict[str, Any], unvalidated_dict_type] + + +@pytest.mark.parametrize("dict_type", any_dict_types) +def test_validate_subclass_spec_in_any_dict_disabled_kept(parser, dict_type): + parser.add_argument("--dict", type=dict_type) + + cfg = parser.parse_args(['--dict={"class_path": "nonexistent.Foo"}']) + assert cfg.dict == {"class_path": "nonexistent.Foo"} + + +@pytest.mark.parametrize("dict_type", any_dict_types) +def test_validate_subclass_spec_in_any_dict_enabled_fails(parser, dict_type): + set_parsing_settings(validate_subclass_spec_in_any=True) + parser.add_argument("--dict", type=dict_type) + + with pytest.raises(ArgumentError, match="Invalid subclass spec given as value for type"): + parser.parse_args(['--dict={"class_path": "nonexistent.Foo"}']) + + +@pytest.mark.parametrize("dict_type", any_dict_types) +def test_validate_subclass_spec_in_any_dict_enabled_valid_kept_as_dict(parser, dict_type): + set_parsing_settings(validate_subclass_spec_in_any=True) + parser.add_argument("--dict", type=dict_type) + spec = {"class_path": f"{__name__}.AnySubclass", "init_args": {"p": 3}} + + cfg = parser.parse_args([f"--dict={json.dumps(spec)}"]) + assert cfg.dict == spec + + assert json_or_yaml_load(parser.dump(cfg)) == {"dict": spec} + assert parser.instantiate(cfg).dict == spec + + +@pytest.mark.parametrize("dict_type", any_dict_types) +def test_validate_subclass_spec_in_any_dict_enabled_non_subclass_dict_kept(parser, dict_type): + set_parsing_settings(validate_subclass_spec_in_any=True) + parser.add_argument("--dict", type=dict_type) + + cfg = parser.parse_args(['--dict={"a": 0, "b": 1}']) + assert cfg.dict == {"a": 0, "b": 1} + + +def function_unresolvable_dict_subtype(d: Dict[str, "MisspelledType"] = {}): # type: ignore[name-defined] # noqa: F821 + return d # pragma: no cover + + +def test_validate_subclass_spec_in_any_enabled_unresolvable_dict_subtype_fails(parser): + set_parsing_settings(validate_subclass_spec_in_any=True) + parser.add_function_arguments(function_unresolvable_dict_subtype, "fn") + + with pytest.raises(ArgumentError, match=r"Invalid subclass spec given as value for type Dict\[str, Unvalidated<"): + parser.parse_args(['--fn.d={"class_path": "nonexistent.Foo"}']) + + +def test_validate_subclass_spec_in_any_enabled_validated_dict_unaffected(parser): + set_parsing_settings(validate_subclass_spec_in_any=True) + parser.add_argument("--dict", type=Dict[str, str]) + + cfg = parser.parse_args(['--dict={"class_path": "nonexistent.Foo"}']) + assert cfg.dict == {"class_path": "nonexistent.Foo"} + + +class SpecTypedDict(TypedDict): + class_path: str + + +def test_validate_subclass_spec_in_any_enabled_typed_dict_unaffected(parser): + set_parsing_settings(validate_subclass_spec_in_any=True) + parser.add_argument("--dict", type=SpecTypedDict) + + cfg = parser.parse_args(['--dict={"class_path": "nonexistent.Foo"}']) + assert cfg.dict == {"class_path": "nonexistent.Foo"} + + +def test_validate_subclass_spec_in_any_disabled_union_dict_swallows(parser): + parser.add_argument("--union", type=Optional[Union[AnySubclass, Dict[str, Any]]]) + + cfg = parser.parse_args([f'--union={{"class_path": "{__name__}.AnySubclass", "init_args": {{"nope": 1}}}}']) + assert cfg.union == {"class_path": f"{__name__}.AnySubclass", "init_args": {"nope": 1}} + + +def test_validate_subclass_spec_in_any_enabled_union_dict_fails(parser): + set_parsing_settings(validate_subclass_spec_in_any=True) + parser.add_argument("--union", type=Optional[Union[AnySubclass, Dict[str, Any]]]) + + with pytest.raises(ArgumentError) as ctx: + parser.parse_args([f'--union={{"class_path": "{__name__}.AnySubclass", "init_args": {{"nope": 1}}}}']) + ctx.match("Does not validate against any of the Union subtypes") + ctx.match("Invalid subclass spec given as value for type Dict") diff --git a/jsonargparse_tests/test_pydantic.py b/jsonargparse_tests/test_pydantic.py index aac5c483..17088668 100644 --- a/jsonargparse_tests/test_pydantic.py +++ b/jsonargparse_tests/test_pydantic.py @@ -62,6 +62,14 @@ def test_pydantic_secret_str(parser): assert "secret" not in parser.dump(cfg) +@skip_if_pydantic_v1_on_v2 +def test_pydantic_secret_str_mask_not_parsed(parser): + parser.add_argument("--password", type=pydantic.SecretStr) + dumped = parser.dump(parser.parse_args(["--password=secret"])) + with pytest.raises(ArgumentError, match="Refusing to parse the mask"): + parser.parse_string(dumped) + + if annotated and pydantic_support > 1: @pydantic.dataclasses.dataclass(frozen=True) diff --git a/jsonargparse_tests/test_signatures.py b/jsonargparse_tests/test_signatures.py index cacff913..5c8bb6e1 100644 --- a/jsonargparse_tests/test_signatures.py +++ b/jsonargparse_tests/test_signatures.py @@ -1,5 +1,6 @@ from __future__ import annotations +import functools import json import sys from pathlib import Path @@ -243,6 +244,42 @@ def test_add_class_implemented_with_new(parser): assert cfg.a == Namespace(a1=4, a2=2.3) +def wrap_new(cls): + """Decorator like the ones used to mark a class as experimental or deprecated.""" + original_new = cls.__new__ + + @functools.wraps(original_new) + def __new__(cls_, /, *args, **kwargs): # pragma: no cover + return original_new(cls_) + + cls.__new__ = staticmethod(__new__) + return cls + + +@wrap_new +class WithWrappedNew: + def __init__(self, w1: int = 1, w2: float = 2.3): + pass # pragma: no cover + + +@wrap_new +class WithWrappedNewSubclass(WithWrappedNew): + def __init__(self, w3: str = "x", **kwargs): + super().__init__(**kwargs) # pragma: no cover + + +def test_add_class_decorator_wrapped_new(parser): + parser.add_class_arguments(WithWrappedNew, "a") + cfg = parser.parse_args(["--a.w1=4"]) + assert cfg.a == Namespace(w1=4, w2=2.3) + + +def test_add_class_decorator_wrapped_new_subclass(parser): + parser.add_class_arguments(WithWrappedNewSubclass, "a") + cfg = parser.parse_args(["--a.w3=y"]) + assert cfg.a == Namespace(w3="y", w1=1, w2=2.3) + + class RequiredParams: def __init__(self, n: int, m: float): self.n = n diff --git a/jsonargparse_tests/test_subclasses.py b/jsonargparse_tests/test_subclasses.py index fd3eb756..8e9c7415 100644 --- a/jsonargparse_tests/test_subclasses.py +++ b/jsonargparse_tests/test_subclasses.py @@ -7,9 +7,24 @@ from calendar import Calendar from copy import deepcopy from dataclasses import dataclass +from functools import partial from gzip import GzipFile from pathlib import Path -from typing import Any, Dict, Generic, Iterable, List, Mapping, Optional, Protocol, Type, TypeVar, Union +from typing import ( + Any, + Callable, + Dict, + Generic, + Iterable, + List, + Mapping, + Optional, + Protocol, + Tuple, + Type, + TypeVar, + Union, +) from unittest.mock import patch from uuid import NAMESPACE_OID @@ -442,6 +457,37 @@ def test_importable_instances(parser): assert dump == {"dtype": f"{__name__}.float32"} +calendar_instance = Calendar(firstweekday=3) + + +def test_importable_instance_class_from_another_module(parser): + parser.add_argument("--cal", type=Calendar) + cfg = parser.parse_args([f"--cal={__name__}.calendar_instance"]) + assert cfg.cal is calendar_instance + dump = json_or_yaml_load(parser.dump(cfg)) + assert dump == {"cal": f"{__name__}.calendar_instance"} + assert parser.parse_string(parser.dump(cfg)).cal is calendar_instance + + +def test_importable_instances_in_list(parser): + parser.add_argument("--cals", type=Optional[List[Calendar]]) + cfg = parser.parse_args([f'--cals=["{__name__}.calendar_instance"]']) + assert cfg.cals == [calendar_instance] + dump = json_or_yaml_load(parser.dump(cfg)) + assert dump == {"cals": [f"{__name__}.calendar_instance"]} + + +callable_instance = partial(sorted, reverse=True) + + +def test_importable_callable_instance_dump(parser): + parser.add_argument("--fn", type=Optional[Callable]) + cfg = parser.parse_args([f"--fn={__name__}.callable_instance"]) + assert cfg.fn is callable_instance + dump = json_or_yaml_load(parser.dump(cfg)) + assert dump == {"fn": f"{__name__}.callable_instance"} + + # custom instantiation tests @@ -1880,6 +1926,72 @@ def test_implements_protocol_type_hints(expected, protocol, value): assert implements_protocol(value, protocol) is expected +ProtoVar = TypeVar("ProtoVar") +ProtoVarContra = TypeVar("ProtoVarContra", contravariant=True) +ImplVar = TypeVar("ImplVar") + + +class GenericInterface(Protocol[ProtoVar]): + def run(self, x: Optional[ProtoVar] = None) -> List[ProtoVar]: ... + + +class GenericImplementsOwnTypeVar: + def run(self, x: Optional[ImplVar] = None) -> List[ImplVar]: + return [] # pragma: no cover + + +class GenericImplementsConcrete: + def run(self, x: Optional[int] = None) -> List[int]: + return [] # pragma: no cover + + +class GenericNotImplements: + def run(self, x: Optional[int] = None) -> int: + return 0 # pragma: no cover + + +class GenericNotAcceptsNone: + def run(self, x: Union[int, str] = 0) -> List[int]: + return [] # pragma: no cover + + +class GenericPairInterface(Protocol[ProtoVarContra]): + def run(self, x: Tuple[ProtoVarContra, ProtoVarContra]) -> None: ... + + +class GenericPairImplements: + def run(self, x: Tuple[str, str]) -> None: ... + + +class GenericPairNotImplements: + def run(self, x: Tuple[int, int, int]) -> None: ... + + +@pytest.mark.parametrize( + "expected, protocol, value", + [ + (True, GenericInterface, GenericImplementsOwnTypeVar), + (True, GenericInterface, GenericImplementsConcrete), + (False, GenericInterface, GenericNotImplements), + (False, GenericInterface, GenericNotAcceptsNone), + (True, GenericPairInterface, GenericPairImplements), + (False, GenericPairInterface, GenericPairNotImplements), + ], +) +def test_implements_generic_protocol(expected, protocol, value): + assert implements_protocol(value, protocol) is expected + + +def test_parse_implements_generic_protocol(parser): + parser.add_argument("--cls", type=GenericInterface) + cfg = parser.parse_args([f"--cls={__name__}.GenericImplementsOwnTypeVar"]) + assert cfg.cls.class_path == f"{__name__}.GenericImplementsOwnTypeVar" + init = parser.instantiate(cfg) + assert isinstance(init.cls, GenericImplementsOwnTypeVar) + with pytest.raises(ArgumentError, match="does not implement protocol"): + parser.parse_args([f"--cls={__name__}.GenericNotImplements"]) + + class MultipleMethodsInterface(Protocol): def one(self, a: int) -> None: ... diff --git a/jsonargparse_tests/test_typehints.py b/jsonargparse_tests/test_typehints.py index 1852120a..7aafbbd1 100644 --- a/jsonargparse_tests/test_typehints.py +++ b/jsonargparse_tests/test_typehints.py @@ -354,6 +354,62 @@ def test_type_typehint_constrained_typevar_arg(parser): assert f"(type: {expected}, default: null)" in get_parser_help(parser) +# typevar as the type itself tests + + +class TypeVarOptions(TypedDict, total=False): + temperature: float + + +BoundTypedDictVar = TypeVar("BoundTypedDictVar", bound=TypeVarOptions) + + +def test_typevar_bound_argument(parser): + parser.add_argument("--options", type=Optional[BoundTypedDictVar]) + assert parser.parse_args(['--options={"temperature": 0.5}']).options == {"temperature": 0.5} + pytest.raises(ArgumentError, lambda: parser.parse_args(['--options={"unknown": 1}'])) + assert f"(type: {type_to_str(Optional[TypeVarOptions])}, default: null)" in get_parser_help(parser) + + +def function_typevar_bound(options: Optional[BoundTypedDictVar] = None): + pass # pragma: no cover + + +def test_typevar_bound_signature_parameter(parser): + parser.add_function_arguments(function_typevar_bound, "x") + assert parser.parse_args(['--x.options={"temperature": 0.5}']).x.options == {"temperature": 0.5} + pytest.raises(ArgumentError, lambda: parser.parse_args(['--x.options={"unknown": 1}'])) + assert "Unvalidated" not in get_parser_help(parser) + + +def test_typevar_constrained_argument(parser): + parser.add_argument("--val", type=ConstrainedVar) + assert parser.parse_args(["--val=1"]).val == 1 + assert parser.parse_args(["--val=a"]).val == "a" + assert f"(type: {type_to_str(Union[int, str])}, default: null)" in get_parser_help(parser) + + +def function_typevar_unbound(val: Optional[UnboundVar] = None): + pass # pragma: no cover + + +def test_typevar_unbound_signature_parameter(parser): + parser.add_function_arguments(function_typevar_unbound, "x") + assert parser.parse_args(['--x.val={"any": 1}']).x.val == {"any": 1} + assert "Unvalidated" in get_parser_help(parser) + + +@pytest.mark.skipif(not typing_extensions_support, reason="typing_extensions package is required") +def test_typevar_default_argument(parser): + from typing_extensions import TypeVar as TypeVarExt + + default_var = TypeVarExt("default_var", bound=Mapping[str, Any], default=TypeVarOptions) + parser.add_argument("--options", type=Optional[default_var]) + assert parser.parse_args(['--options={"temperature": 0.5}']).options == {"temperature": 0.5} + pytest.raises(ArgumentError, lambda: parser.parse_args(['--options={"unknown": 1}'])) + assert f"(type: {type_to_str(Optional[TypeVarOptions])}, default: null)" in get_parser_help(parser) + + # enum tests diff --git a/jsonargparse_tests/test_typing.py b/jsonargparse_tests/test_typing.py index 3b3907c9..486f2ed3 100644 --- a/jsonargparse_tests/test_typing.py +++ b/jsonargparse_tests/test_typing.py @@ -466,6 +466,14 @@ def test_secret_str_parsing(parser): assert "secret" not in parser.dump(cfg) +def test_secret_str_mask_not_parsed(parser): + parser.add_argument("--password", type=SecretStr) + cfg = parser.parse_args(["--password=secret"]) + dumped = parser.dump(cfg) + with pytest.raises(ArgumentError, match="Refusing to parse the mask"): + parser.parse_string(dumped) + + def test_top_level_compatibility_not_in_public_api(): import jsonargparse as ja diff --git a/pyproject.toml b/pyproject.toml index 1a6415a7..944bb73f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -190,7 +190,7 @@ legacy_tox_ini = """ [tox] envlist = py{310,311,312,313,314}-{all-extras,no-extras,argparse},omegaconf,pydantic-v1,without-pyyaml,without-future-annotations labels = - coverage = py{310,311,312,313,314}-{all-extras,no-extras},pydantic-v1,without-pyyaml,without-future-annotations + coverage = py{310,311,312,313,314}-{all-extras,no-extras},omegaconf,pydantic-v1,without-pyyaml,without-future-annotations skip_missing_interpreters = true [testenv]