Fix several issues related to type hints and signature parameters

CHANGELOG.rst

@@ -43,6 +43,9 @@ Added
   Previously adding an argument with these types failed with ``TypeError:
   'member_descriptor' object is not iterable`` (`#945
   <https://github.com/mauvilsa/jsonargparse/pull/945>`__).
+- Support ``Collection``, ``Container`` and ``Reversible``, validated as a list,
+  and ``AbstractSet``, validated as a set (`#950
+  <https://github.com/mauvilsa/jsonargparse/pull/950>`__).
 
 Fixed
 ^^^^^
@@ -81,6 +84,20 @@ Fixed
   as the instances given for a subclass type, i.e. as an import path when the
   value can be imported back, otherwise as a message that says that it was not
   serializable (`#948 <https://github.com/mauvilsa/jsonargparse/pull/948>`__).
+- ``AssertionError`` without a message when adding an argument typed as a
+  subscripted user defined generic class, e.g. ``Optional[Strategy[T]]`` (`#950
+  <https://github.com/mauvilsa/jsonargparse/pull/950>`__).
+- A ``Callable`` type accepting values that are neither callable nor an import
+  path, e.g. a list, which in a union such as ``Union[Callable,
+  list[Callable]]`` prevented the list items from being resolved (`#950
+  <https://github.com/mauvilsa/jsonargparse/pull/950>`__).
+- ``type[T]`` never validating. Now the ``TypeVar`` is replaced by its bound,
+  its constraints, or ``object`` when it has neither, so that the help shows
+  what is accepted, e.g. ``type[object]`` (`#950
+  <https://github.com/mauvilsa/jsonargparse/pull/950>`__).
+- Parameters of a subscripted generic class being dropped when their type is a
+  PEP 604 union, e.g. ``p: int | None`` in a ``Generic[T]`` class added as
+  ``MyClass[int]`` (`#950 <https://github.com/mauvilsa/jsonargparse/pull/950>`__).
 
 Changed
 ^^^^^^^

DOCUMENTATION.rst

@@ -520,7 +520,8 @@ Some notes about this support are:
 - Fully supported types are: ``str``, ``bool`` (more details in
   :ref:`boolean-arguments`), ``int``, ``float``, ``Decimal``, ``complex``,
   ``bytes``/``bytearray`` (Base64 encoding), ``range``, ``list`` (more details
-  in :ref:`list-append`), ``Deque``, ``Iterable``, ``Sequence``, ``Any``,
+  in :ref:`list-append`), ``Deque``, ``Iterable``, ``Sequence``,
+  ``MutableSequence``, ``Collection``, ``Container``, ``Reversible``, ``Any``,
   ``Union``/``Optional`` (more details in :ref:`union-types`), ``Type``,
   ``Enum``, ``PathLike``, ``UUID``, ``timedelta``, restricted types as explained
   in sections :ref:`restricted-numbers` and :ref:`restricted-strings` and path
@@ -541,8 +542,9 @@ Some notes about this support are:
   i.e. it has all the keys of the expected ``TypedDict``, with the same types
   and requiredness.
 
-- ``tuple``, ``set``, ``frozenset`` and ``MutableSet`` are supported even though
-  they can't be represented in JSON distinguishable from a list. Each ``tuple``
+- ``tuple``, ``set``, ``frozenset``, ``AbstractSet`` and ``MutableSet`` are
+  supported even though they can't be represented in JSON distinguishable from
+  a list. Each ``tuple``
   element position can have its own type and will be validated as such.
   ``tuple`` with ellipsis (``tuple[type, ...]``) is also supported. In command
   line arguments, config files and environment variables, tuples and sets are

jsonargparse/_parameter_resolvers.py

@@ -302,14 +302,18 @@ def get_signature_parameters_and_indexes(component, parent, logger):
 
 def replace_generic_type_vars(params: ParamList, parent) -> None:
     if is_generic_class(parent) and parent.__args__ and getattr(parent.__origin__, "__parameters__", None):
+        from ._typehints import rebuild_typehint_args
+
         type_vars = dict(zip(parent.__origin__.__parameters__, parent.__args__))
 
         def replace_type_vars(annotation):
             if annotation in type_vars:
                 return type_vars[annotation]
-            if getattr(annotation, "__args__", None):
-                origin = annotation.__origin__
-                return origin[tuple(replace_type_vars(a) for a in annotation.__args__)]
+            args = getattr(annotation, "__args__", None)
+            # only a tuple, since e.g. types.UnionType has __args__ as a class level slot
+            # descriptor, which is truthy but not the subtypes of an instance
+            if isinstance(args, tuple) and args:
+                return rebuild_typehint_args(annotation, tuple(replace_type_vars(a) for a in args))
             return annotation
 
         for param in params:

jsonargparse/_signatures.py

@@ -341,15 +341,25 @@ def _add_signature_parameter(
         name = param.name
         kind = param.kind
         annotation = param.annotation
-        register_pydantic_types(annotation)  # before the check of what can be validated
+        src = get_parameter_origins(param.component, param.parent)
+        skip_message = f'Skipping parameter "{name}" from "{src}" because of: '
+        # Before anything is done with the annotation, so that a type that jsonargparse
+        # is unable to handle can be worked around by skipping the parameter.
+        if skip and name in skip:
+            self.logger.debug(skip_message + "Parameter requested to be skipped.")
+            return
         unvalidated: list = []
-        unvalidatable_replaced = replace_unvalidatable_typehints(annotation, unvalidated)
+        try:
+            register_pydantic_types(annotation)  # before the check of what can be validated
+            unvalidatable_replaced = replace_unvalidatable_typehints(annotation, unvalidated)
+        except Exception as ex:
+            raise ValueError(f'Unable to add parameter "{name}" from "{src}": {ex}') from ex
         if unvalidated:
             reasons = " ".join(f"{u.name}: {u.reason}." for u in unvalidated)
             self.logger.debug(
-                f'Parameter "{name}" from "{get_parameter_origins(param.component, param.parent)}" has '
-                f"a type that can't be fully validated: {annotation}. {reasons} These parts are shown "
-                "in the help as Unvalidated<...> and accept any value without validation."
+                f'Parameter "{name}" from "{src}" has a type that can\'t be fully validated: '
+                f"{annotation}. {reasons} These parts are shown in the help as Unvalidated<...> "
+                "and accept any value without validation."
             )
             annotation = unvalidatable_replaced
         if default == inspect_empty:
@@ -380,8 +390,6 @@ def _add_signature_parameter(
             is_non_positional = False  # Can be positional
         else:
             raise RuntimeError(f"The code should never reach here: kind={kind}")  # pragma: no cover
-        src = get_parameter_origins(param.component, param.parent)
-        skip_message = f'Skipping parameter "{name}" from "{src}" because of: '
         if annotation != inspect_empty:
             # Checked before linked_targets and fail_untyped adjust is_required, since the wrappers
             # are meant to agree with the requiredness that the signature itself defines.
@@ -405,9 +413,6 @@ def _add_signature_parameter(
             is_required_link_target = True
         if not is_required and name[0] == "_":
             return
-        elif skip and name in skip:
-            self.logger.debug(skip_message + "Parameter requested to be skipped.")
-            return
         if is_factory_class(default):
             default = param.parent.__dataclass_fields__[name].default_factory()
         if annotation == inspect_empty and not is_required:

jsonargparse/_typehints.py

@@ -20,8 +20,11 @@
 from operator import or_
 from types import FunctionType, GenericAlias, MappingProxyType, ModuleType, UnionType
 from typing import (
+    AbstractSet,
     Any,
     Callable,
+    Collection,
+    Container,
     Deque,
     Dict,
     ForwardRef,
@@ -34,6 +37,7 @@
     MutableSequence,
     MutableSet,
     NoReturn,
+    Reversible,
     Sequence,
     Set,
     Tuple,
@@ -54,6 +58,7 @@
     remove_actions,
 )
 from ._common import (
+    get_generic_origin,
     get_parsing_setting,
     get_unaliased_type,
     is_generic_class,
@@ -134,10 +139,16 @@ def _capture_typing_extension_shadows(name: str, *collections) -> None:
     FrozenSet,
     Deque,
     deque,
+    Collection,
+    Container,
     Iterable,
+    Reversible,
     Sequence,
     MutableSequence,
+    abc.Collection,
+    abc.Container,
     abc.Iterable,
+    abc.Reversible,
     abc.Sequence,
     abc.MutableSequence,
     Tuple,
@@ -146,7 +157,9 @@ def _capture_typing_extension_shadows(name: str, *collections) -> None:
     FrozenSet,
     set,
     frozenset,
+    AbstractSet,
     MutableSet,
+    abc.Set,
     abc.MutableSet,
     Dict,
     dict,
@@ -175,16 +188,22 @@ def _capture_typing_extension_shadows(name: str, *collections) -> None:
 
 leaf_or_root_types = leaf_types.union(root_types)
 
-tuple_set_origin_types = {Tuple, tuple, Set, set, frozenset, MutableSet, abc.Set, abc.MutableSet}
+tuple_set_origin_types = {Tuple, tuple, Set, set, frozenset, AbstractSet, MutableSet, abc.Set, abc.MutableSet}
 sequence_origin_types = {
     List,
     list,
     Deque,
     deque,
+    Collection,
+    Container,
     Iterable,
+    Reversible,
     Sequence,
     MutableSequence,
+    abc.Collection,
+    abc.Container,
     abc.Iterable,
+    abc.Reversible,
     abc.Sequence,
     abc.MutableSequence,
 }
@@ -339,7 +358,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(typehint)
+            self._typehint = sort_unions_in_typehint(replace_type_vars_in_type_subtype(typehint))
             self._enable_path = False if is_pathlike(typehint) else enable_path
         elif "_typehint" not in kwargs:
             raise ValueError("Expected typehint keyword argument.")
@@ -1453,6 +1472,10 @@ def adapt_typehints(
                         partial_skip_args=partial_skip_args,
                         prev_val=prev_val,
                     )
+                elif not callable(val):
+                    # e.g. a list, which without this would be accepted unchanged, silently
+                    # preventing the resolution by other subtypes when part of a union
+                    raise ImportError(f"Expected an import path or a subclass spec, but got {val_input}")
             except (ImportError, AttributeError, ArgumentError) as ex:
                 raise_unexpected_value(f"Type {typehint} expects a function or a callable class: {ex}", val, ex)
 
@@ -1838,7 +1861,9 @@ def yield_class_types(typehint, is_single, also_lists=False, callable_return=Fal
         for subtype in typehint.__args__:
             yield from yield_class_types(subtype, **kwargs)
     if is_single(typehint, typehint_origin):
-        yield typehint
+        # a subscripted user defined generic, e.g. Strategy[T], is yielded as its origin
+        # class, since the consumers use the class itself, e.g. to look up its subclasses
+        yield get_generic_origin(typehint)
 
 
 def get_subclass_types(typehint, also_lists=False, callable_return=False):
@@ -2157,6 +2182,33 @@ 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.
+
+    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]``.
+    """
+    if get_typehint_origin(typehint) in literal_types:
+        return typehint  # the args of a Literal are values, not types
+    args = getattr(typehint, "__args__", None)
+    # only a tuple, since e.g. types.UnionType and types.GenericAlias have __args__ as a
+    # class level slot descriptor, which is truthy but not the subtypes of an instance
+    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,)
+    else:
+        new_args = tuple(replace_type_vars_in_type_subtype(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)
+
+
 def sort_unions_in_typehint(typehint):
     """Returns the type hint with the subtypes of all its unions sorted, including nested ones.
 

jsonargparse_tests/test_signatures.py

@@ -342,6 +342,23 @@ def test_add_class_skip_parameter_debug_logging(parser, logger):
     assert "because of: Parameter requested to be skipped" in logs.getvalue()
 
 
+class UnusableType:
+    def __init__(self, a1: int = 1, a2: Optional[Namespace] = None):
+        pass  # pragma: no cover
+
+
+def test_add_class_skip_parameter_with_unusable_type(parser):
+    parser.add_class_arguments(UnusableType, "c", skip={"a2"})
+    assert parser.parse_args([]) == Namespace(c=Namespace(a1=1))
+
+
+def test_add_class_parameter_with_unusable_type_error_context(parser):
+    with pytest.raises(ValueError) as ctx:
+        parser.add_class_arguments(UnusableType, "c")
+    ctx.match('Unable to add parameter "a2" from ".*UnusableType.__init__"')
+    ctx.match("jsonargparse.Namespace is only intended for parsing results")
+
+
 class WithinSubcommand:
     def __init__(self, a: int = 1):
         self.a = a
@@ -427,6 +444,21 @@ def test_add_class_generics(parser):
     assert cfg.p == Namespace(a=5, b=6 + 7j)
 
 
+class WithGenericsPep604Union(Generic[X]):
+    def __init__(self, a: X | None = None, b: int | None = None):  # pragma: no cover
+        self.a = a
+        self.b = b
+
+
+def test_add_class_generics_pep604_union(parser):
+    parser.add_class_arguments(WithGenericsPep604Union[int], "p")
+    cfg = parser.parse_args(["--p.a=5", "--p.b=6"])
+    assert cfg.p == Namespace(a=5, b=6)
+    help_str = get_parser_help(parser)
+    assert "--p.a A" in help_str
+    assert "--p.b B" in help_str
+
+
 class UnmatchedDefaultType:
     def __init__(self, p1: str, p2: bool = "deprecated"):  # type: ignore[assignment]
         self.p2 = p2

jsonargparse_tests/test_subclasses.py

@@ -9,7 +9,7 @@
 from dataclasses import dataclass
 from gzip import GzipFile
 from pathlib import Path
-from typing import Any, Dict, Iterable, List, Mapping, Optional, Protocol, Union
+from typing import Any, Dict, Generic, Iterable, List, Mapping, Optional, Protocol, Type, TypeVar, Union
 from unittest.mock import patch
 from uuid import NAMESPACE_OID
 
@@ -2234,6 +2234,26 @@ def test_subclass_class_name_ambiguous(parser, option):
         parser.parse_args([f"{option}=LocaleTextCalendar"])
 
 
+T = TypeVar("T")
+
+
+class GenericStrategy(Generic[T]):
+    def __init__(self, schema: Optional[Type[T]] = None):
+        self.schema = schema
+
+
+def test_subclass_generic_alias_in_union(parser):
+    parser.add_argument("--op", type=Optional[GenericStrategy[T]])
+    help_str = get_parser_help(parser)
+    assert "Show the help for the given subclass of GenericStrategy" in help_str
+    help_str = get_parse_args_stdout(parser, [f"--op.help={__name__}.GenericStrategy"])
+    assert "--op.schema" in help_str
+    cfg = parser.parse_args([f"--op={__name__}.GenericStrategy"])
+    assert cfg.op.class_path == f"{__name__}.GenericStrategy"
+    init = parser.instantiate(cfg)
+    assert isinstance(init.op, GenericStrategy)
+
+
 def test_subclass_help_not_subclass(parser):
     parser.add_argument("--op", type=BaseC)
     with pytest.raises(ArgumentError) as ctx: