Fix dump round-tripping, generic protocols, TypeVar types and extend subclass spec validation

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 <https://github.com/mauvilsa/jsonargparse/pull/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
+  <https://github.com/mauvilsa/jsonargparse/pull/938>`__, `#953
+  <https://github.com/mauvilsa/jsonargparse/pull/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
   <https://github.com/mauvilsa/jsonargparse/pull/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 <https://github.com/mauvilsa/jsonargparse/pull/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
   <https://github.com/mauvilsa/jsonargparse/pull/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
+  <https://github.com/mauvilsa/jsonargparse/pull/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
+  <https://github.com/mauvilsa/jsonargparse/pull/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 <https://github.com/mauvilsa/jsonargparse/pull/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
+  <https://github.com/mauvilsa/jsonargparse/pull/953>`__).
 
 Changed
 ^^^^^^^

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::
 

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

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)

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)
     )
 
 

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

jsonargparse/_typehints.py

@@ -345,6 +345,7 @@
             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 @@
                     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 @@
         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 @@
 
     # 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 @@
     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 @@
     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 @@
     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 @@
     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)