Repository: omni-us/jsonargparse

Issue:
Improve the error message when a link function fails
<!-- If you like this project, please ⭐ star it https://github.com/omni-us/jsonargparse/ -->

## 🚀 Feature request

When a link function fails, the error message should give a hint that the error happened when trying to apply a link function.

### Motivation

Currently, the only error message that is displayed is the exception in the link function itself. e.g. `argparse.ArgumentError: 'NoneType' object is not subscriptable`. That makes it very hard to understand what the actual problem is.

### Pitch

Include in the error message that the error happened on linking, and ideally also in which function.

### Alternatives

In my particular case, I had the problem when calling `--print_config` for a `LightningCLI`. In that case, there is no need to call the link functions since the linked arguments are not part of the printed config. So this particular error could maybe be avoided by changing the implementation of printing the config.


Discussion:
sonarcloud[bot]: Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=omni-us_jsonargparse&pullRequest=381)

[![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=omni-us_jsonargparse&pullRequest=381&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=omni-us_jsonargparse&pullRequest=381&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=omni-us_jsonargparse&pullRequest=381&resolved=false&types=BUG)  
[![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=omni-us_jsonargparse&pullRequest=381&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=omni-us_jsonargparse&pullRequest=381&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=omni-us_jsonargparse&pullRequest=381&resolved=false&types=VULNERABILITY)  
[![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=omni-us_jsonargparse&pullRequest=381&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=omni-us_jsonargparse&pullRequest=381&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=omni-us_jsonargparse&pullRequest=381&resolved=false&types=SECURITY_HOTSPOT)  
[![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=omni-us_jsonargparse&pullRequest=381&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=omni-us_jsonargparse&pullRequest=381&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=omni-us_jsonargparse&pullRequest=381&resolved=false&types=CODE_SMELL)

[![No Coverage information](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo-16px.png 'No Coverage information')](https://sonarcloud.io/component_measures?id=omni-us_jsonargparse&pullRequest=381) No Coverage information  
[![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=omni-us_jsonargparse&pullRequest=381&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=omni-us_jsonargparse&pullRequest=381&metric=new_duplicated_lines_density&view=list)


codecov[bot]: ## [Codecov](https://app.codecov.io/gh/omni-us/jsonargparse/pull/381?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) Report
Patch coverage: **`100.00%`** and project coverage change: **`-0.02%`** :warning:
> Comparison is base [(`584dca0`)](https://app.codecov.io/gh/omni-us/jsonargparse/commit/584dca0743750bdf37b1a5a373906ad32c5b790d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) 100.00% compared to head [(`c4596e4`)](https://app.codecov.io/gh/omni-us/jsonargparse/pull/381?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) 99.98%.

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


```diff
@@             Coverage Diff             @@
##              main     #381      +/-   ##
===========================================
- Coverage   100.00%   99.98%   -0.02%     
===========================================
  Files           21       21              
  Lines         5712     5729      +17     
===========================================
+ Hits          5712     5728      +16     
- Misses           0        1       +1     
```

| [Flag](https://app.codecov.io/gh/omni-us/jsonargparse/pull/381/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/381/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `87.71% <100.00%> (+0.03%)` | :arrow_up: |
| [py3.10_all](https://app.codecov.io/gh/omni-us/jsonargparse/pull/381/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `?` | |
| [py3.10_types](https://app.codecov.io/gh/omni-us/jsonargparse/pull/381/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `?` | |
| [py3.11](https://app.codecov.io/gh/omni-us/jsonargparse/pull/381/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `?` | |
| [py3.11_all](https://app.codecov.io/gh/omni-us/jsonargparse/pull/381/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `?` | |
| [py3.11_types](https://app.codecov.io/gh/omni-us/jsonargparse/pull/381/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `?` | |
| [py3.7](https://app.codecov.io/gh/omni-us/jsonargparse/pull/381/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `88.25% <100.00%> (+0.03%)` | :arrow_up: |
| [py3.7_all](https://app.codecov.io/gh/omni-us/jsonargparse/pull/381/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `99.68% <100.00%> (+<0.01%)` | :arrow_up: |
| [py3.7_types](https://app.codecov.io/gh/omni-us/jsonargparse/pull/381/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `99.66% <100.00%> (+<0.01%)` | :arrow_up: |
| [py3.8](https://app.codecov.io/gh/omni-us/jsonargparse/pull/381/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `88.40% <100.00%> (+0.03%)` | :arrow_up: |
| [py3.8_all](https://app.codecov.io/gh/omni-us/jsonargparse/pull/381/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `99.73% <100.00%> (+<0.01%)` | :arrow_up: |
| [py3.8_types](https://app.codecov.io/gh/omni-us/jsonargparse/pull/381/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `99.72% <100.00%> (+<0.01%)` | :arrow_up: |
| [py3.9](https://app.codecov.io/gh/omni-us/jsonargparse/pull/381/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `?` | |
| [py3.9_all](https://app.codecov.io/gh/omni-us/jsonargparse/pull/381/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `?` | |
| [py3.9_types](https://app.codecov.io/gh/omni-us/jsonargparse/pull/381/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `?` | |

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.

| [Files Changed](https://app.codecov.io/gh/omni-us/jsonargparse/pull/381?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | Coverage Δ | |
|---|---|---|
| [jsonargparse/\_actions.py](https://app.codecov.io/gh/omni-us/jsonargparse/pull/381?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us#diff-anNvbmFyZ3BhcnNlL19hY3Rpb25zLnB5) | `100.00% <100.00%> (ø)` | |
| [jsonargparse/\_link\_arguments.py](https://app.codecov.io/gh/omni-us/jsonargparse/pull/381?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us#diff-anNvbmFyZ3BhcnNlL19saW5rX2FyZ3VtZW50cy5weQ==) | `100.00% <100.00%> (ø)` | |

... and [1 file with indirect coverage changes](https://app.codecov.io/gh/omni-us/jsonargparse/pull/381/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us)


</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/omni-us/jsonargparse/pull/381?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 637df72..a27be8a 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -15,11 +15,19 @@ paths are considered internals and can change in minor and patch releases.
 v4.25.0 (2023-09-??)
 --------------------
 
+Fixed
+^^^^^
+- ``--print_config`` fails when parser has shallow links.
+- Argument links unnecessarily applied when ``--print_config`` used and parser
+  has subcommands (`#311 <https://github.com/omni-us/jsonargparse/issue/311>`__).
+
 Changed
 ^^^^^^^
 - Provide a more informative error message to remind user to select
   and provide a subcommand when a subcommand is required but not
-  provided (`#371 <https://github.com/omni-us/jsonargparse/pull/371>`__).
+  given (`#371 <https://github.com/omni-us/jsonargparse/pull/371>`__).
+- Now when an argument link ``compute_fn`` fails, the error message will say
+  this (`#311 <https://github.com/omni-us/jsonargparse/issue/311>`__).
 - Removed support for python 3.6.
 
 
diff --git a/jsonargparse/_actions.py b/jsonargparse/_actions.py
index 4f4a1f1..32baccc 100644
--- a/jsonargparse/_actions.py
+++ b/jsonargparse/_actions.py
@@ -283,6 +283,14 @@ class _ActionPrintConfig(Action):
             delattr(parser, "print_config")
             parser.exit()
 
+    @staticmethod
+    def is_print_config_requested(parser):
+        while parser:
+            if hasattr(parser, "print_config"):
+                return True
+            parser = getattr(parser, "parent_parser", None)
+        return False
+
 
 class _ActionConfigLoad(Action):
     def __init__(self, basetype: Optional[Type] = None, **kwargs):
diff --git a/jsonargparse/_link_arguments.py b/jsonargparse/_link_arguments.py
index 7dbc65a..8ccabca 100644
--- a/jsonargparse/_link_arguments.py
+++ b/jsonargparse/_link_arguments.py
@@ -13,6 +13,7 @@ from ._actions import (
     Action,
     ActionConfigFile,
     _ActionConfigLoad,
+    _ActionPrintConfig,
     _ActionSubCommands,
     _find_parent_action,
     filter_default_actions,
@@ -243,9 +244,18 @@ class ActionLink(Action):
     def _check_type(self, value, cfg=None):
         return self.parser._check_value_key(self.target[1], value, self.target[0], cfg)
 
+    def call_compute_fn(self, args):
+        try:
+            assert callable(self.compute_fn)
+            return self.compute_fn(*args)
+        except Exception as ex:
+            link = self.option_strings[0]
+            args = ", ".join(str(a) for a in args)
+            raise ValueError(f"Call to compute_fn of link '{link}' with args ({args}) failed: {ex}") from ex
+
     @staticmethod
     def apply_parsing_links(parser: "ArgumentParser", cfg: Namespace) -> None:
-        if apply_config_skip.get():
+        if apply_config_skip.get() or _ActionPrintConfig.is_print_config_requested(parser):
             return
 
         subcommand, subparser = _ActionSubCommands.get_subcommand(parser, cfg, fail_no_subcommand=False)
@@ -294,7 +304,7 @@ class ActionLink(Action):
                     ):
                         args[n] = args[n].as_dict()
                 # Compute value
-                value = action.compute_fn(*args)
+                value = action.call_compute_fn(args)
             ActionLink.set_target_value(action, value, cfg, parser.logger)
 
     @staticmethod
@@ -334,7 +344,7 @@ class ActionLink(Action):
             elif action.compute_fn is None:
                 value = source_objects[0]
             else:
-                value = action.compute_fn(*source_objects)
+                value = action.call_compute_fn(source_objects)
             ActionLink.set_target_value(action, value, cfg, parser.logger)
             applied_links.add(action)
 
@@ -384,6 +394,8 @@ class ActionLink(Action):
     def strip_link_target_keys(parser, cfg):
         def del_target_key(target_key):
             cfg.pop(target_key, None)
+            if "." not in target_key:
+                return
             parent_key, _ = split_key_leaf(target_key)
             if "." in target_key and parent_key in cfg and not cfg[parent_key]:
                 del cfg[parent_key]


Test patch:
diff --git a/jsonargparse_tests/test_link_arguments.py b/jsonargparse_tests/test_link_arguments.py
index 37dc722..bbdd619 100644
--- a/jsonargparse_tests/test_link_arguments.py
+++ b/jsonargparse_tests/test_link_arguments.py
@@ -15,7 +15,7 @@ from jsonargparse import (
     lazy_instance,
 )
 from jsonargparse._optionals import docstring_parser_support
-from jsonargparse_tests.conftest import get_parser_help
+from jsonargparse_tests.conftest import get_parse_args_stdout, get_parser_help
 
 # tests for links applied on parse
 
@@ -28,6 +28,38 @@ def test_on_parse_help_target_lacking_type_and_help(parser):
     assert "Target argument 'b' lacks type and help" in help_str
 
 
+def test_on_parse_shallow_print_config(parser):
+    parser.add_argument("--cfg", action=ActionConfigFile)
+    parser.add_argument("--a", type=int, default=0)
+    parser.add_argument("--b", type=str)
+    parser.link_arguments("a", "b")
+    out = get_parse_args_stdout(parser, ["--print_config"])
+    assert yaml.safe_load(out) == {"a": 0}
+
+
+def test_on_parse_subcommand_failing_compute_fn(parser, subparser, subtests):
+    def to_str(value):
+        if not value:
+            raise ValueError("value is empty")
+        return str(value)
+
+    subparser.add_argument("--a", type=int, default=0)
+    subparser.add_argument("--b", type=str)
+    subparser.link_arguments("a", "b", to_str)
+    subparser.add_argument("--config", action=ActionConfigFile)
+    subcommands = parser.add_subcommands()
+    subcommands.add_subcommand("sub", subparser)
+
+    with subtests.test("parse_args"):
+        with pytest.raises(ValueError) as ctx:
+            parser.parse_args(["sub"])
+        ctx.match("Call to compute_fn of link 'to_str.*failed: value is empty")
+
+    with subtests.test("print_config"):
+        out = get_parse_args_stdout(parser, ["sub", "--print_config"])
+        assert yaml.safe_load(out) == {"a": 0}
+
+
 def test_on_parse_compute_fn_single_arguments(parser, subtests):
     def a_prod(a):
         return a["v1"] * a["v2"]
@@ -402,6 +434,32 @@ def test_on_instantiate_link_all_group_arguments():
     assert "Group 'x': All arguments are derived from links" in help_str
 
 
+class FailingComputeFn1:
+    def __init__(self, a: int = 0):
+        self.a = a
+
+
+class FailingComputeFn2:
+    def __init__(self, b: str):
+        self.b = b
+
+
+def test_on_instantiate_failing_compute_fn(parser):
+    def to_str(value):
+        if not value:
+            raise ValueError("value is empty")
+        return str(value)
+
+    parser.add_class_arguments(FailingComputeFn1, "c1")
+    parser.add_class_arguments(FailingComputeFn2, "c2")
+    parser.link_arguments("c1.a", "c2.b", compute_fn=to_str, apply_on="instantiate")
+
+    with pytest.raises(ValueError) as ctx:
+        cfg = parser.parse_args([])
+        parser.instantiate_classes(cfg)
+    ctx.match("Call to compute_fn of link 'to_str.*failed: value is empty")
+
+
 def test_on_instantiate_link_from_subclass_with_compute_fn():
     parser = get_parser_subclasses_link_on_instantiate()
     cfg = parser.parse_args(