Repository: omni-us/jsonargparse
Pull request #435 (2024-01-24 10:40:19)

Fix signature parameter default that doesn't match its type failing on instantiation and serialization
## What does this PR do?

Fixes https://github.com/Lightning-AI/pytorch-lightning/pull/19289#issuecomment-1902618722

## Before submitting

- [x] Did you read the [contributing guideline](https://github.com/omni-us/jsonargparse/blob/main/CONTRIBUTING.rst)?
- [n/a] 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=435) **Quality Gate passed**  
Kudos, no new issues were introduced!

[0 New issues](https://sonarcloud.io/project/issues?id=omni-us_jsonargparse&pullRequest=435&resolved=false&inNewCodePeriod=true)  
[0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=omni-us_jsonargparse&pullRequest=435&resolved=false&inNewCodePeriod=true)  
No data about Coverage  
[0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=omni-us_jsonargparse&pullRequest=435&metric=new_duplicated_lines_density&view=list)  
  
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=omni-us_jsonargparse&pullRequest=435)


codecov[bot]: ## [Codecov](https://app.codecov.io/gh/omni-us/jsonargparse/pull/435?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:
> Comparison is base [(`fd6a44b`)](https://app.codecov.io/gh/omni-us/jsonargparse/commit/fd6a44b9f64a7f8c3ff299ba53dacc6f1699cbca?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) 100.00% compared to head [(`2d11f4b`)](https://app.codecov.io/gh/omni-us/jsonargparse/pull/435?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) 100.00%.


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


```diff
@@            Coverage Diff            @@
##              main      #435   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           21        21           
  Lines         5958      5957    -1     
=========================================
- Hits          5958      5957    -1     
```

| [Flag](https://app.codecov.io/gh/omni-us/jsonargparse/pull/435/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/435/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `88.29% <100.00%> (-0.01%)` | :arrow_down: |
| [py3.10_all](https://app.codecov.io/gh/omni-us/jsonargparse/pull/435/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `98.72% <100.00%> (-0.01%)` | :arrow_down: |
| [py3.10_pydantic1](https://app.codecov.io/gh/omni-us/jsonargparse/pull/435/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `48.31% <100.00%> (+0.02%)` | :arrow_up: |
| [py3.10_pydantic2](https://app.codecov.io/gh/omni-us/jsonargparse/pull/435/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `48.04% <100.00%> (+0.02%)` | :arrow_up: |
| [py3.10_types](https://app.codecov.io/gh/omni-us/jsonargparse/pull/435/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `98.72% <100.00%> (-0.01%)` | :arrow_down: |
| [py3.11](https://app.codecov.io/gh/omni-us/jsonargparse/pull/435/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `88.28% <100.00%> (-0.01%)` | :arrow_down: |
| [py3.11_all](https://app.codecov.io/gh/omni-us/jsonargparse/pull/435/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `98.72% <100.00%> (-0.01%)` | :arrow_down: |
| [py3.11_types](https://app.codecov.io/gh/omni-us/jsonargparse/pull/435/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `98.72% <100.00%> (-0.01%)` | :arrow_down: |
| [py3.12](https://app.codecov.io/gh/omni-us/jsonargparse/pull/435/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `88.28% <100.00%> (-0.01%)` | :arrow_down: |
| [py3.12_all](https://app.codecov.io/gh/omni-us/jsonargparse/pull/435/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `98.72% <100.00%> (-0.01%)` | :arrow_down: |
| [py3.12_types](https://app.codecov.io/gh/omni-us/jsonargparse/pull/435/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `98.72% <100.00%> (-0.01%)` | :arrow_down: |
| [py3.7](https://app.codecov.io/gh/omni-us/jsonargparse/pull/435/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `88.87% <100.00%> (-0.01%)` | :arrow_down: |
| [py3.7_all](https://app.codecov.io/gh/omni-us/jsonargparse/pull/435/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_down: |
| [py3.7_types](https://app.codecov.io/gh/omni-us/jsonargparse/pull/435/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_down: |
| [py3.8](https://app.codecov.io/gh/omni-us/jsonargparse/pull/435/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `89.02% <100.00%> (-0.01%)` | :arrow_down: |
| [py3.8_all](https://app.codecov.io/gh/omni-us/jsonargparse/pull/435/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_down: |
| [py3.8_types](https://app.codecov.io/gh/omni-us/jsonargparse/pull/435/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_down: |
| [py3.9](https://app.codecov.io/gh/omni-us/jsonargparse/pull/435/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `88.87% <100.00%> (-0.01%)` | :arrow_down: |
| [py3.9_all](https://app.codecov.io/gh/omni-us/jsonargparse/pull/435/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `99.37% <100.00%> (-0.01%)` | :arrow_down: |
| [py3.9_types](https://app.codecov.io/gh/omni-us/jsonargparse/pull/435/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `99.37% <100.00%> (-0.01%)` | :arrow_down: |

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/435?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 8eb3936..5c6ac8d 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -19,6 +19,9 @@ Fixed
 ^^^^^
 - Argument links not working for target ``init_args`` in an optional list (`#433
   <https://github.com/omni-us/jsonargparse/issues/433>`__).
+- Signature parameter default that doesn't match its type failing on
+  instantiation and serialization (`lightning#19289 comment
+  <https://github.com/Lightning-AI/pytorch-lightning/pull/19289#issuecomment-1902618722>`__)
 
 
 v4.27.2 (2024-01-18)
diff --git a/jsonargparse/_typehints.py b/jsonargparse/_typehints.py
index 740ac3d..f19a0fe 100644
--- a/jsonargparse/_typehints.py
+++ b/jsonargparse/_typehints.py
@@ -429,9 +429,14 @@ class ActionTypeHint(Action):
         with dump_kwargs_context(dump_kwargs):
             if _is_action_value_list(self):
                 return [
-                    adapt_typehints(v, self._typehint, serialize=True, sub_add_kwargs=sub_add_kwargs) for v in value
+                    adapt_typehints(
+                        v, self._typehint, default=self.default, serialize=True, sub_add_kwargs=sub_add_kwargs
+                    )
+                    for v in value
                 ]
-            return adapt_typehints(value, self._typehint, serialize=True, sub_add_kwargs=sub_add_kwargs)
+            return adapt_typehints(
+                value, self._typehint, default=self.default, serialize=True, sub_add_kwargs=sub_add_kwargs
+            )
 
     def __call__(self, *args, **kwargs):
         """Parses an argument validating against the corresponding type hint.
@@ -496,12 +501,9 @@ class ActionTypeHint(Action):
                 except ValueError as ex:
                     assert ex  # needed due to ruff bug that removes " as ex"
                     try:
-                        if orig_val is not None and orig_val == self.default:
-                            val = orig_val
-                            ex = None
-                        elif isinstance(orig_val, str):
+                        if isinstance(orig_val, str):
                             with change_to_path_dir(config_path):
-                                val = adapt_typehints(orig_val, self._typehint, **kwargs)
+                                val = adapt_typehints(orig_val, self._typehint, default=self.default, **kwargs)
                             ex = None
                     except ValueError:
                         if self._enable_path and config_path is None and isinstance(orig_val, str):
@@ -527,7 +529,9 @@ class ActionTypeHint(Action):
             value = [value]
         sub_add_kwargs = getattr(self, "sub_add_kwargs", {})
         for num, val in enumerate(value):
-            value[num] = adapt_typehints(val, self._typehint, instantiate_classes=True, sub_add_kwargs=sub_add_kwargs)
+            value[num] = adapt_typehints(
+                val, self._typehint, default=self.default, instantiate_classes=True, sub_add_kwargs=sub_add_kwargs
+            )
         return value if islist else value[0]
 
     @staticmethod
@@ -616,7 +620,11 @@ def adapt_typehints(
     list_item=False,
     enable_path=False,
     sub_add_kwargs=None,
+    default=None,
 ):
+    if type(val) in {str, bool, int, float} and val == default:
+        return val
+
     adapt_kwargs = {
         "serialize": serialize,
         "instantiate_classes": instantiate_classes,


Test patch:
diff --git a/jsonargparse_tests/test_signatures.py b/jsonargparse_tests/test_signatures.py
index a58f03c..a9f140e 100644
--- a/jsonargparse_tests/test_signatures.py
+++ b/jsonargparse_tests/test_signatures.py
@@ -407,6 +407,21 @@ def test_add_class_generics(parser):
     assert cfg.p == Namespace(a=5, b=6 + 7j)
 
 
+class UnmatchedDefaultType:
+    def __init__(self, p1: str, p2: bool = "deprecated"):  # type: ignore[assignment]
+        self.p2 = p2
+
+
+def test_add_class_unmatched_default_type(parser):
+    parser.add_class_arguments(UnmatchedDefaultType, "cls")
+    cfg = parser.parse_args(["--cls.p1=x"])
+    assert cfg.cls == Namespace(p1="x", p2="deprecated")
+    init = parser.instantiate_classes(cfg)
+    assert init.cls.p2 == "deprecated"
+    dump = parser.dump(cfg)
+    assert dump == "cls:\n  p1: x\n  p2: deprecated\n"
+
+
 # add_method_arguments tests
 
 
@@ -624,13 +639,3 @@ def test_add_function_param_conflict(parser):
     with pytest.raises(ValueError) as ctx:
         parser.add_function_arguments(func_param_conflict)
     ctx.match("Unable to add parameter 'cfg' from")
-
-
-def func_unmatched_default_type(p1: str, p2: bool = "deprecated"):  # type: ignore[assignment]
-    return p1
-
-
-def test_add_function_unmatched_default_type(parser):
-    parser.add_function_arguments(func_unmatched_default_type)
-    cfg = parser.parse_args(["--p1=x"])
-    assert cfg.p1 == "x"