Repository: omni-us/jsonargparse
Pull request #469 (2024-03-12 09:00:06)

Fix: Optional callable that returns a class instance with a lambda default, produces an invalid string default
## Before submitting

- [x] Did you read the [contributing guideline](https://github.com/omni-us/jsonargparse/blob/main/CONTRIBUTING.rst)?
- [x] Did you update **the documentation**? (readme and public docstrings)
- [x] Did you write **unit tests** such that there is 100% coverage on related code? (required for bug fixes and new features)
- [x] Did you verify that new and existing **tests pass locally**?
- [x] Did you make sure that all changes preserve **backward compatibility**?
- [x] Did you update **the CHANGELOG**? (not for typos, docs, test updates, or minor internal changes/refactors)


sonarcloud[bot]: ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=omni-us_jsonargparse&pullRequest=469) **Quality Gate passed**  
Issues  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 New issues](https://sonarcloud.io/project/issues?id=omni-us_jsonargparse&pullRequest=469&resolved=false&inNewCodePeriod=true)  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/component_measures?id=omni-us_jsonargparse&pullRequest=469&metric=new_accepted_issues&view=list)

Measures  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=omni-us_jsonargparse&pullRequest=469&resolved=false&inNewCodePeriod=true)  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/no-data-16px.png '') No data about Coverage  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=omni-us_jsonargparse&pullRequest=469&metric=new_duplicated_lines_density&view=list)  
  
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=omni-us_jsonargparse&pullRequest=469)


codecov[bot]: ## [Codecov](https://app.codecov.io/gh/omni-us/jsonargparse/pull/469?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 100.00%. Comparing base [(`b98df25`)](https://app.codecov.io/gh/omni-us/jsonargparse/commit/b98df253420cc093857bd2e8d06895112e03d968?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) to head [(`245da37`)](https://app.codecov.io/gh/omni-us/jsonargparse/pull/469?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us).


<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff            @@
##              main      #469   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           21        21           
  Lines         5997      6004    +7     
=========================================
+ Hits          5997      6004    +7     
```

| [Flag](https://app.codecov.io/gh/omni-us/jsonargparse/pull/469/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | Coverage Δ | |
|---|---|---|
| [py3.10](https://app.codecov.io/gh/omni-us/jsonargparse/pull/469/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `88.39% <100.00%> (+0.01%)` | :arrow_up: |
| [py3.10_all](https://app.codecov.io/gh/omni-us/jsonargparse/pull/469/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `98.73% <100.00%> (+<0.01%)` | :arrow_up: |
| [py3.10_pydantic1](https://app.codecov.io/gh/omni-us/jsonargparse/pull/469/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `48.33% <72.72%> (+0.02%)` | :arrow_up: |
| [py3.10_pydantic2](https://app.codecov.io/gh/omni-us/jsonargparse/pull/469/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `48.06% <72.72%> (+0.02%)` | :arrow_up: |
| [py3.10_types](https://app.codecov.io/gh/omni-us/jsonargparse/pull/469/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `98.73% <100.00%> (+<0.01%)` | :arrow_up: |
| [py3.11](https://app.codecov.io/gh/omni-us/jsonargparse/pull/469/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `88.37% <100.00%> (+0.01%)` | :arrow_up: |
| [py3.11_all](https://app.codecov.io/gh/omni-us/jsonargparse/pull/469/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `98.73% <100.00%> (+<0.01%)` | :arrow_up: |
| [py3.11_types](https://app.codecov.io/gh/omni-us/jsonargparse/pull/469/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `98.73% <100.00%> (+<0.01%)` | :arrow_up: |
| [py3.12](https://app.codecov.io/gh/omni-us/jsonargparse/pull/469/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `88.37% <100.00%> (+0.01%)` | :arrow_up: |
| [py3.12_all](https://app.codecov.io/gh/omni-us/jsonargparse/pull/469/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `98.73% <100.00%> (+<0.01%)` | :arrow_up: |
| [py3.12_types](https://app.codecov.io/gh/omni-us/jsonargparse/pull/469/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `98.73% <100.00%> (+<0.01%)` | :arrow_up: |
| [py3.7](https://app.codecov.io/gh/omni-us/jsonargparse/pull/469/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `88.95% <100.00%> (+0.01%)` | :arrow_up: |
| [py3.7_all](https://app.codecov.io/gh/omni-us/jsonargparse/pull/469/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `99.49% <100.00%> (+<0.01%)` | :arrow_up: |
| [py3.7_types](https://app.codecov.io/gh/omni-us/jsonargparse/pull/469/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `99.47% <100.00%> (+<0.01%)` | :arrow_up: |
| [py3.8](https://app.codecov.io/gh/omni-us/jsonargparse/pull/469/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `89.10% <100.00%> (+0.01%)` | :arrow_up: |
| [py3.8_all](https://app.codecov.io/gh/omni-us/jsonargparse/pull/469/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `99.51% <100.00%> (+<0.01%)` | :arrow_up: |
| [py3.8_types](https://app.codecov.io/gh/omni-us/jsonargparse/pull/469/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `99.50% <100.00%> (+<0.01%)` | :arrow_up: |
| [py3.9](https://app.codecov.io/gh/omni-us/jsonargparse/pull/469/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `88.95% <100.00%> (+0.01%)` | :arrow_up: |
| [py3.9_all](https://app.codecov.io/gh/omni-us/jsonargparse/pull/469/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `99.38% <100.00%> (+<0.01%)` | :arrow_up: |
| [py3.9_types](https://app.codecov.io/gh/omni-us/jsonargparse/pull/469/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `99.38% <100.00%> (+<0.01%)` | :arrow_up: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us#carryforward-flags-in-the-pull-request-comment) to find out more.


</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/omni-us/jsonargparse/pull/469?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us).


Patch:
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 817924b..87a789b 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -22,6 +22,8 @@ Fixed
   <https://github.com/omni-us/jsonargparse/issues/454>`__).
 - Subsubcommand parse failing when ``default_env=True`` (`#465
   <https://github.com/omni-us/jsonargparse/issues/465>`__).
+- Optional callable that returns a class instance with a lambda default,
+  produces an invalid string default.
 
 
 v4.27.5 (2024-02-12)
diff --git a/DOCUMENTATION.rst b/DOCUMENTATION.rst
index c9055cb..41a2b4d 100644
--- a/DOCUMENTATION.rst
+++ b/DOCUMENTATION.rst
@@ -1060,7 +1060,10 @@ Then a parser and behavior could be:
     >>> optimizer.params, optimizer.lr
     ([1, 2, 3], 0.05)
 
-See :ref:`ast-resolver` for limitations of lambda defaults.
+See :ref:`ast-resolver` for limitations of lambda defaults in signatures.
+Providing a lambda default to :py:meth:`.ArgumentParser.add_argument` does not
+work since there is no AST resolving. In this case, a dict with ``class_path``
+and ``init_args`` can be used as default.
 
 
 .. _registering-types:
diff --git a/jsonargparse/_parameter_resolvers.py b/jsonargparse/_parameter_resolvers.py
index 71045ef..f0f1f85 100644
--- a/jsonargparse/_parameter_resolvers.py
+++ b/jsonargparse/_parameter_resolvers.py
@@ -334,14 +334,15 @@ ast_literals = {ast.dump(ast.parse(v, mode="eval").body): partial(ast.literal_ev
 def is_param_subclass_instance_default(param: ParamData) -> bool:
     if is_dataclass_like(type(param.default)):
         return False
-    from ._typehints import ActionTypeHint, get_subclass_types
+    from ._typehints import ActionTypeHint, get_optional_arg, get_subclass_types
 
-    class_types = get_subclass_types(param.annotation)
+    annotation = get_optional_arg(param.annotation)
+    class_types = get_subclass_types(annotation)
     return (class_types and isinstance(param.default, class_types)) or (
         is_lambda(param.default)
-        and ActionTypeHint.is_callable_typehint(param.annotation, all_subtypes=False)
-        and param.annotation.__args__
-        and ActionTypeHint.is_subclass_typehint(param.annotation.__args__[-1], all_subtypes=False)
+        and ActionTypeHint.is_callable_typehint(annotation, all_subtypes=False)
+        and annotation.__args__
+        and ActionTypeHint.is_subclass_typehint(annotation.__args__[-1], all_subtypes=False)
     )
 
 
diff --git a/jsonargparse/_typehints.py b/jsonargparse/_typehints.py
index 3b14ea6..eb6f050 100644
--- a/jsonargparse/_typehints.py
+++ b/jsonargparse/_typehints.py
@@ -281,6 +281,7 @@ class ActionTypeHint(Action):
 
     @staticmethod
     def is_return_subclass_typehint(typehint):
+        typehint = get_optional_arg(typehint)
         typehint_origin = get_typehint_origin(typehint)
         if typehint_origin in callable_origin_types:
             return_type = get_callable_return_type(typehint)
@@ -578,7 +579,7 @@ class ActionTypeHint(Action):
             enum = self._typehint
             return list(enum.__members__.keys())
         elif is_optional(self._typehint, Enum):
-            enum = self._typehint.__args__[0]
+            enum = get_optional_arg(self._typehint)
             return list(enum.__members__.keys()) + ["null"]
         elif is_optional(self._typehint, Path):
             files_completer = get_files_completer()
@@ -954,6 +955,7 @@ def get_callable_return_type(typehint):
 def get_subclass_types(typehint, callable_return=True):
     subclass_types = None
     if callable_return and ActionTypeHint.is_callable_typehint(typehint, all_subtypes=False) and typehint.__args__:
+        typehint = get_optional_arg(typehint)
         typehint = typehint.__args__[-1]
     if ActionTypeHint.is_subclass_typehint(typehint, all_subtypes=False):
         if get_typehint_origin(typehint) == Union:
@@ -1180,16 +1182,22 @@ def is_ellipsis_tuple(typehint):
     return typehint.__origin__ in {Tuple, tuple} and len(typehint.__args__) > 1 and typehint.__args__[1] == Ellipsis
 
 
-def is_optional(annotation, ref_type=object):
+def is_optional(annotation, ref_type=None):
     """Checks whether a type annotation is an optional for one type class."""
     return (
         get_typehint_origin(annotation) == Union
         and len(annotation.__args__) == 2
         and any(NoneType == a for a in annotation.__args__)
-        and any(is_subclass(a, ref_type) for a in annotation.__args__)
+        and (ref_type is None or all(is_subclass(a, ref_type) for a in annotation.__args__ if a != NoneType))
     )
 
 
+def get_optional_arg(annotation, ref_type=None):
+    if is_optional(annotation, ref_type):
+        annotation = next(a for a in annotation.__args__ if a != NoneType)
+    return annotation
+
+
 def is_enum_type(annotation):
     return is_subclass(annotation, Enum) or (
         get_typehint_origin(annotation) == Union and any(is_subclass(a, Enum) for a in annotation.__args__)


Test patch:
diff --git a/jsonargparse_tests/test_typehints.py b/jsonargparse_tests/test_typehints.py
index cc640e5..26f9c7d 100644
--- a/jsonargparse_tests/test_typehints.py
+++ b/jsonargparse_tests/test_typehints.py
@@ -786,6 +786,43 @@ def test_callable_args_return_type_union_of_classes(parser, subtests):
             assert f"{__name__}.{name}" in help_str
 
 
+def optional_callable_args_return_type_class(
+    scheduler: Optional[Callable[[Optimizer], StepLR]] = lambda o: StepLR(o, last_epoch=1)
+):
+    return scheduler
+
+
+def test_optional_callable_args_return_type_class(parser, subtests):
+    parser.add_function_arguments(optional_callable_args_return_type_class)
+    optimizer = Optimizer([])
+
+    with subtests.test("default"):
+        cfg = parser.get_defaults()
+        init = parser.instantiate_classes(cfg)
+        scheduler = init.scheduler(optimizer)
+        assert isinstance(scheduler, StepLR)
+        assert scheduler.last_epoch == 1
+        assert scheduler.optimizer is optimizer
+
+    with subtests.test("parse"):
+        value = {
+            "class_path": "StepLR",
+            "init_args": {
+                "last_epoch": 2,
+            },
+        }
+        cfg = parser.parse_args([f"--scheduler={value}"])
+        init = parser.instantiate_classes(cfg)
+        scheduler = init.scheduler(optimizer)
+        assert isinstance(scheduler, StepLR)
+        assert scheduler.last_epoch == 2
+        assert scheduler.optimizer is optimizer
+
+    with subtests.test("parse null"):
+        cfg = parser.parse_args(["--scheduler=null"])
+        assert cfg.scheduler is None
+
+
 class CallableSubconfig:
     def __init__(self, o: Callable[[int], Optimizer]):
         self.o = o