register_type replaces instead of failing, applies to subscripted types, and warns for callable defaults that can't be imported back

CHANGELOG.rst

@@ -82,6 +82,14 @@ Fixed
   ``collections.abc`` spelling of the same types was. A bare one didn't validate
   and a composed one, e.g. ``Optional[Hashable]``, raised ``Unsupported type
   hint`` (`#963 <https://github.com/mauvilsa/jsonargparse/pull/963>`__).
+- A ``Callable`` default that can't be imported back, was silently dumped as a
+  non-importable ``<locals>`` import path. Now the default is kept as the object
+  and dumping it gives the not serializable message and a warning (`#966
+  <https://github.com/mauvilsa/jsonargparse/pull/966>`__).
+- Types registered with ``register_type`` were ignored when the type is
+  subscripted, e.g. ``os.PathLike[str]`` for a registered ``PathLike``. Now the
+  registration of the unsubscripted type is used (`#966
+  <https://github.com/mauvilsa/jsonargparse/pull/966>`__).
 
 Changed
 ^^^^^^^
@@ -101,6 +109,12 @@ Changed
   makes the type optional, a ``NotRequired`` parameter without a default and a
   parameter that is the target of a link (`#965
   <https://github.com/mauvilsa/jsonargparse/pull/965>`__).
+- ``register_type`` no longer fails when the type is already registered. The new
+  registration now replaces the previous one and a debug log informs about it,
+  naming the module of each registration. This way a new type registered by
+  jsonargparse doesn't break code that already registers it. The previous
+  behavior is available with ``fail_already_registered=True`` (`#966
+  <https://github.com/mauvilsa/jsonargparse/pull/966>`__).
 
 Deprecated
 ^^^^^^^^^^

DOCUMENTATION.rst

@@ -1186,6 +1186,13 @@ for example ``datetime``:
     parser.add_argument("--datetime", type=datetime)
     parser.parse_args(["--datetime=2008-09-03T20:56:35"])
 
+Registering an already registered type replaces the previous one, jsonargparse's
+own registrations included. A debug log names the module of each, useful when
+two packages register the same type. Give ``fail_already_registered=True`` to
+fail instead. A generic class is registered unsubscripted, and the registration
+also applies to its subscripted forms, e.g. ``os.PathLike[str]``. The type
+arguments are not validated, since the deserializer gets the complete value.
+
 .. note::
 
     Registering is only intended for simple types. By default, any class used as

jsonargparse/_typehints.py

@@ -392,7 +392,11 @@ def normalize_default(self, default):
         elif is_module_type(self._typehint) and isinstance(default, ModuleType):
             default = default.__name__
         elif is_callable_type(self._typehint) and callable(default) and not inspect.isclass(default):
-            default = get_import_path(default)
+            try:
+                default = object_path_serializer(default)
+            except ValueError:
+                # kept as is when it can't be imported back, e.g. a closure, so that dump warns
+                pass
         elif ActionTypeHint.is_return_subclass_typehint(self._typehint) and inspect.isclass(default):
             default = {"class_path": get_import_path(default)}
         elif is_subclass_type and not allow_default_instance.get():
@@ -974,8 +978,7 @@ class UntypedType(UnvalidatedType):
     """
 
     def __init__(self):
-        self.reason = untyped_reason
-        self.name = ""
+        super().__init__("", reason=untyped_reason)  # no type in the source code, thus an empty name
 
     def __repr__(self):
         return "Untyped"
@@ -1558,7 +1561,7 @@ def adapt_typehints(
                 val, partial_skip_args = adapt_partial_callable_class(typehint, val)
                 val = adapt_class_type(val, True, False, sub_add_kwargs, partial_skip_args=partial_skip_args)
             else:
-                val = object_path_serializer(val)
+                val = serialize_as_import_path(val)
         else:
             adapted = adapt_subconfig_path(val, typehint, adapt_kwargs)
             if adapted is not not_a_subconfig_path:
@@ -1627,7 +1630,7 @@ def adapt_typehints(
     elif inspect.isclass(typehint_origin):
         if is_instance_or_supports_protocol(val, typehint):
             if serialize:
-                val = serialize_class_instance(val)
+                val = serialize_as_import_path(val)
             return val
         if serialize and isinstance(val, str):
             return val
@@ -2834,14 +2837,14 @@ def typehint_metavar(typehint):
     return metavar
 
 
-def serialize_class_instance(val):
-    with suppress(Exception):
-        import_path = get_import_path(val)
-        if import_path and import_object(import_path, check_path=False) is val:
-            return import_path
-    val = f"Unable to serialize instance {val}"
-    warning(val)
-    return val
+def serialize_as_import_path(val):
+    """Serializes an object as its import path, warning when it can't be imported back."""
+    try:
+        return object_path_serializer(val)
+    except ValueError:
+        val = f"Unable to serialize instance {val}"
+        warning(val)
+        return val
 
 
 def typehint_from_value(val):
@@ -2885,7 +2888,7 @@ def serialize_unvalidated(val, adapt_kwargs):
         return val
     typehint = typehint_from_value(val)
     if typehint is None:
-        return serialize_class_instance(val)
+        return serialize_as_import_path(val)
     if isinstance(val, dict):
         adapt_val = dict(val)  # adapt_typehints serializes the items in place, so give it a copy
     elif isinstance(val, list):

jsonargparse/typing.py

@@ -9,7 +9,7 @@
 from collections.abc import Callable
 from typing import Any, TypeAlias, get_type_hints
 
-from ._common import ClassType, is_final_class, is_subclass, path_dump_preserve_relative
+from ._common import ClassType, get_settings_logger, is_final_class, is_subclass, path_dump_preserve_relative
 from ._deprecated import renamed_parameter_warning
 from ._namespace import Namespace
 from ._optionals import final, is_alias_type, pydantic_support
@@ -436,22 +436,26 @@ def __init__(self, v, **k):
 
 
 class RegisteredType:
+    _eq_attrs = ["class_type", "serializer", "base_deserializer", "deserializer_exceptions", "type_check"]
+
     def __init__(
         self,
         class_type: _TypeClass,
         serializer: Callable,
         deserializer: Callable | None,
         deserializer_exceptions: type[Exception] | tuple[type[Exception], ...],
         type_check: Callable,
+        module: str,
     ):
         self.class_type = class_type
         self.serializer = serializer
         self.base_deserializer = class_type if deserializer is None else deserializer
         self.deserializer_exceptions = deserializer_exceptions
         self.type_check = type_check
+        self.module = module
 
     def __eq__(self, other):
-        return all(getattr(self, k) == getattr(other, k) for k in ["class_type", "serializer", "base_deserializer"])
+        return all(getattr(self, k) == getattr(other, k) for k in self._eq_attrs)
 
     def is_value_of_type(self, value):
         return self.type_check(value, self.class_type)
@@ -466,6 +470,14 @@ def deserializer(self, value):
             raise ex2 from ex
 
 
+def get_registrant_module() -> str:
+    """Returns the name of the module that called the caller of this function."""
+    frame: Any = sys._getframe(2)
+    while frame.f_globals.get("__name__") == "jsonargparse._deprecated":  # skip the deprecation decorator
+        frame = frame.f_back
+    return frame.f_globals.get("__name__", "unknown")
+
+
 @renamed_parameter_warning({"type_class": "class_type"})
 def register_type(
     class_type: _TypeClass,
@@ -477,32 +489,40 @@ def register_type(
         AttributeError,
     ),
     type_check: Callable = lambda v, t: v.__class__ == t,
-    fail_already_registered: bool = True,
+    fail_already_registered: bool = False,
     uniqueness_key: tuple | None = None,
 ) -> None:
     """Registers a new type for use in jsonargparse parsers.
 
     Args:
-        class_type: The class to be registered. Python 3.12+ also supports
-            ``TypeAliasType`` aliases.
+        class_type: The class to be registered. A generic class is registered
+            unsubscripted and its registration also applies to its subscripted
+            forms. Python 3.12+ also supports ``TypeAliasType`` aliases.
         serializer: Function that converts an instance of the class to a basic type.
         deserializer: Function that converts a basic type to an instance of the
             class. Default instantiates ``class_type``.
         deserializer_exceptions: Exceptions that deserializer raises when it fails.
         type_check: Function to check if a value is of ``class_type``. Gets as arguments the value and ``class_type``.
-        fail_already_registered: Whether to fail if type has already been registered.
+        fail_already_registered: Whether to fail instead of replacing a previous registration of the type.
         uniqueness_key: Key to determine uniqueness of type.
     """
     if sys.version_info[:2] < (3, 12) and not inspect.isclass(class_type):
         raise ValueError(f"Expected class_type to be a class, got {type(class_type)}")
     elif sys.version_info[:2] >= (3, 12) and not (inspect.isclass(class_type) or is_alias_type(class_type)):
         raise ValueError(f"Expected class_type to be a class or a type alias, got {type(class_type)}")
-    type_handler = RegisteredType(class_type, serializer, deserializer, deserializer_exceptions, type_check)
-    fail_already_registered = globals().get("_fail_already_registered", fail_already_registered)
-    if not uniqueness_key and fail_already_registered and get_registered_type(class_type):
-        if type_handler == registered_type_handlers[class_type]:
+    module = get_registrant_module()
+    type_handler = RegisteredType(class_type, serializer, deserializer, deserializer_exceptions, type_check, module)
+    previous = None if uniqueness_key else get_registered_type(class_type)
+    if previous:
+        if previous == type_handler:
             return
-        raise ValueError(f'Type "{class_type}" already registered with different serializer and/or deserializer.')
+        if fail_already_registered:
+            raise ValueError(f'Type "{class_type}" already registered with different serializer and/or deserializer.')
+        class_type_name = getattr(class_type, "__name__", str(class_type))
+        get_settings_logger().debug(
+            f"Type {class_type_name!r} registered by module {module!r} replaced the previous "
+            f"registration by module {previous.module!r}"
+        )
     registered_type_handlers[class_type] = type_handler
     if uniqueness_key is not None:
         registered_types[uniqueness_key] = class_type
@@ -524,7 +544,13 @@ def get_registered_type(class_type) -> RegisteredType | None:
             import_path = get_import_path(class_type)
             if import_path in registration_pending:
                 registration_pending.pop(import_path)()
-    return registered_type_handlers.get(class_type)
+    type_handler = registered_type_handlers.get(class_type)
+    if type_handler is None:
+        # a subscripted generic is handled by the registration of its origin, e.g. MyMapping[str, int]
+        origin = getattr(class_type, "__origin__", None)
+        if inspect.isclass(origin):
+            type_handler = get_registered_type(origin)
+    return type_handler
 
 
 def add_type(class_type: type, uniqueness_key: tuple | None, type_check: Callable | None = None):
@@ -538,8 +564,6 @@ def add_type(class_type: type, uniqueness_key: tuple | None, type_check: Callabl
     register_type(class_type, class_type._type, **kwargs)  # type: ignore[attr-defined]
 
 
-_fail_already_registered = False
-
 PositiveInt = restricted_number_type("PositiveInt", int, (">", 0), docstring="int restricted to be >0")
 NonNegativeInt = restricted_number_type("NonNegativeInt", int, (">=", 0), docstring="int restricted to be ≥0")
 PositiveFloat = restricted_number_type("PositiveFloat", float, (">", 0), docstring="float restricted to be >0")
@@ -766,6 +790,3 @@ def register_pydantic_types(typehint):
     if isinstance(args, tuple):
         for arg in args:
             register_pydantic_types(arg)
-
-
-del _fail_already_registered

jsonargparse_tests/test_paths.py

@@ -52,6 +52,11 @@ def test_os_pathlike(parser, file_r):
     assert file_r == parser.parse_args([f"--path={file_r}"]).path
 
 
+def test_os_pathlike_subscripted(parser, file_r):
+    parser.add_argument("--path", type=os.PathLike[str])
+    assert file_r == parser.parse_args([f"--path={file_r}"]).path
+
+
 # base path tests
 
 

jsonargparse_tests/test_typehints.py

@@ -2429,6 +2429,28 @@ def test_callable_function_path(parser):
     ctx.match("Callable expects a function or a callable class")
 
 
+def make_closure_callable():
+    def unbound_closure():
+        return "closure"
+
+    return unbound_closure
+
+
+closure_callable = make_closure_callable()
+
+
+def test_callable_default_not_importable(parser):
+    # the import path of a closure includes a <locals> part, so it can't be imported back
+    parser.add_argument("--callable", type=Callable, default=closure_callable)
+
+    cfg = parser.parse_args([])
+    assert cfg.callable is closure_callable
+
+    with assert_dump_warnings("Unable to serialize instance <function"):
+        dump = json_or_yaml_load(parser.dump(cfg))
+    assert dump["callable"].startswith("Unable to serialize instance <function")
+
+
 def test_callable_list_of_function_paths(parser):
     parser.add_argument("--callables", type=List[Callable])