Repository: omni-us/jsonargparse
Pull request #568 (2024-09-10 05:54:40)

For consistency add_subclass_arguments now sets default None instead of SUPPRESS
## What does this PR do?

Fixes https://github.com/Lightning-AI/pytorch-lightning/issue/20103

## 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=568) **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=568&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=omni-us_jsonargparse&pullRequest=568&issueStatuses=ACCEPTED)

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=568&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=omni-us_jsonargparse&pullRequest=568&metric=new_coverage&view=list)  
![](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=568&metric=new_duplicated_lines_density&view=list)  
  
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=omni-us_jsonargparse&pullRequest=568)


codecov[bot]: ## [Codecov](https://app.codecov.io/gh/omni-us/jsonargparse/pull/568?dropdown=coverage&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 99.70%. Comparing base [(`9919b35`)](https://app.codecov.io/gh/omni-us/jsonargparse/commit/9919b35f0d554bf90dced859969229e008a4de91?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) to head [(`fc14b3e`)](https://app.codecov.io/gh/omni-us/jsonargparse/commit/fc14b3e78979ae9daaf9744a97f98ab032c542a5?dropdown=coverage&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     #568      +/-   ##
===========================================
- Coverage   100.00%   99.70%   -0.30%     
===========================================
  Files           22       22              
  Lines         6441     6441              
===========================================
- Hits          6441     6422      -19     
- Misses           0       19      +19     
```

| [Flag](https://app.codecov.io/gh/omni-us/jsonargparse/pull/568/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/568/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `85.66% <100.00%> (-0.07%)` | :arrow_down: |
| [py3.10_all](https://app.codecov.io/gh/omni-us/jsonargparse/pull/568/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `?` | |
| [py3.10_pydantic1](https://app.codecov.io/gh/omni-us/jsonargparse/pull/568/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `?` | |
| [py3.10_pydantic2](https://app.codecov.io/gh/omni-us/jsonargparse/pull/568/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/568/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/568/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/568/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/568/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `?` | |
| [py3.12](https://app.codecov.io/gh/omni-us/jsonargparse/pull/568/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `?` | |
| [py3.12_all](https://app.codecov.io/gh/omni-us/jsonargparse/pull/568/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `?` | |
| [py3.12_types](https://app.codecov.io/gh/omni-us/jsonargparse/pull/568/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/568/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `86.07% <100.00%> (-0.07%)` | :arrow_down: |
| [py3.7_all](https://app.codecov.io/gh/omni-us/jsonargparse/pull/568/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `99.10% <100.00%> (-0.07%)` | :arrow_down: |
| [py3.7_types](https://app.codecov.io/gh/omni-us/jsonargparse/pull/568/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `99.15% <100.00%> (-0.07%)` | :arrow_down: |
| [py3.8](https://app.codecov.io/gh/omni-us/jsonargparse/pull/568/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `86.26% <100.00%> (-0.07%)` | :arrow_down: |
| [py3.8_all](https://app.codecov.io/gh/omni-us/jsonargparse/pull/568/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `99.26% <100.00%> (-0.07%)` | :arrow_down: |
| [py3.8_types](https://app.codecov.io/gh/omni-us/jsonargparse/pull/568/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `99.30% <100.00%> (-0.07%)` | :arrow_down: |
| [py3.9](https://app.codecov.io/gh/omni-us/jsonargparse/pull/568/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/568/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `99.22% <100.00%> (-0.07%)` | :arrow_down: |
| [py3.9_types](https://app.codecov.io/gh/omni-us/jsonargparse/pull/568/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) | `99.23% <100.00%> (-0.07%)` | :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/568?dropdown=coverage&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).

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=568) **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=568&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=omni-us_jsonargparse&pullRequest=568&issueStatuses=ACCEPTED)

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=568&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=omni-us_jsonargparse&pullRequest=568&metric=new_coverage&view=list)  
![](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=568&metric=new_duplicated_lines_density&view=list)  
  
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=omni-us_jsonargparse&pullRequest=568)


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=568) **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=568&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=omni-us_jsonargparse&pullRequest=568&issueStatuses=ACCEPTED)

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=568&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=omni-us_jsonargparse&pullRequest=568&metric=new_coverage&view=list)  
![](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=568&metric=new_duplicated_lines_density&view=list)  
  
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=omni-us_jsonargparse&pullRequest=568)



Patch:
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 3aef282..a9361da 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -34,6 +34,9 @@ Changed
 ^^^^^^^
 - Removed shtab experimental warning (`#561
   <https://github.com/omni-us/jsonargparse/pull/561>`__).
+- For consistency ``add_subclass_arguments`` now sets default ``None`` instead
+  of ``SUPPRESS`` (`lightning#20103
+  <https://github.com/Lightning-AI/pytorch-lightning/issue/20103>`__).
 
 
 v4.32.1 (2024-08-23)
diff --git a/jsonargparse/_formatters.py b/jsonargparse/_formatters.py
index 6b86aa5..9297543 100644
--- a/jsonargparse/_formatters.py
+++ b/jsonargparse/_formatters.py
@@ -25,7 +25,7 @@ from ._actions import (
 )
 from ._common import defaults_cache, parent_parser
 from ._link_arguments import ActionLink
-from ._namespace import Namespace
+from ._namespace import Namespace, NSKeyError
 from ._optionals import import_ruyaml
 from ._type_checking import ArgumentParser, ruyamlCommentedMap
 from ._typehints import ActionTypeHint, type_to_str
@@ -92,7 +92,7 @@ class DefaultHelpFormatter(HelpFormatter):
             for key in parser.required_args:
                 try:
                     default = parser.get_default(key)
-                except KeyError:
+                except NSKeyError:
                     default = None
                 if default is None and f"[--{key} " in usage:
                     usage = re.sub(f"\\[(--{key} [^\\]]+)]", r"\1", usage, count=1)
diff --git a/jsonargparse/_signatures.py b/jsonargparse/_signatures.py
index 639adc6..807a8d4 100644
--- a/jsonargparse/_signatures.py
+++ b/jsonargparse/_signatures.py
@@ -3,7 +3,7 @@
 import dataclasses
 import inspect
 import re
-from argparse import SUPPRESS, ArgumentParser
+from argparse import ArgumentParser
 from contextlib import suppress
 from typing import Any, Callable, List, Optional, Set, Tuple, Type, Union
 
@@ -549,7 +549,7 @@ class SignatureArguments(LoggerProperty):
             }
         )
         if "default" not in kwargs:
-            kwargs["default"] = SUPPRESS
+            kwargs["default"] = None
         self._add_signature_parameter(
             group, None, param, added_args, skip, sub_configs=True, instantiate=instantiate, **kwargs
         )


Test patch:
diff --git a/jsonargparse_tests/conftest.py b/jsonargparse_tests/conftest.py
index 54a3c0a..cfd9e10 100644
--- a/jsonargparse_tests/conftest.py
+++ b/jsonargparse_tests/conftest.py
@@ -70,6 +70,11 @@ skip_if_responses_unavailable = pytest.mark.skipif(
     reason="responses package is required",
 )
 
+skip_if_running_as_root = pytest.mark.skipif(
+    is_posix and os.geteuid() == 0,
+    reason="User is root, permission tests will not work",
+)
+
 if responses_available:
     import responses
 
diff --git a/jsonargparse_tests/test_core.py b/jsonargparse_tests/test_core.py
index eb1b676..17906e5 100644
--- a/jsonargparse_tests/test_core.py
+++ b/jsonargparse_tests/test_core.py
@@ -26,6 +26,7 @@ from jsonargparse import (
     strip_meta,
 )
 from jsonargparse._formatters import get_env_var
+from jsonargparse._namespace import NSKeyError
 from jsonargparse._optionals import jsonnet_support, jsonschema_support, ruyaml_support
 from jsonargparse.typing import Path_fc, Path_fr, path_type
 from jsonargparse_tests.conftest import (
@@ -39,6 +40,7 @@ from jsonargparse_tests.conftest import (
     skip_if_fsspec_unavailable,
     skip_if_not_posix,
     skip_if_responses_unavailable,
+    skip_if_running_as_root,
 )
 
 
@@ -329,6 +331,7 @@ def test_parse_path_defaults(parser, tmp_cwd):
 
 
 @skip_if_not_posix
+@skip_if_running_as_root
 def test_parse_path_file_not_readable(parser, tmp_cwd):
     config_path = Path("config.json")
     config_path.touch()
@@ -760,9 +763,7 @@ def test_default_config_files(parser, subtests, tmp_cwd):
     with subtests.test("get_default"):
         assert parser.get_default("op1") == "from default config file"
         parser.add_subclass_arguments(Calendar, "cal")
-        with pytest.raises(KeyError) as ctx:
-            parser.get_default("cal")
-        ctx.match("does not specify a default")
+        assert parser.get_default("cal") is None
 
     with subtests.test("set invalid"):
         with pytest.raises(ValueError) as ctx:
@@ -792,6 +793,7 @@ def test_default_config_file_invalid_value(parser, tmp_cwd):
 
 
 @skip_if_not_posix
+@skip_if_running_as_root
 def test_default_config_file_unreadable(parser, tmp_cwd):
     default_config_file = Path("defaults.yaml")
     default_config_file.write_text("op1: from yaml\n")
@@ -885,6 +887,37 @@ def test_set_get_defaults_multiple(parser, subparser, subtests):
         pytest.raises(KeyError, lambda: parser.get_default("v4"))
 
 
+def test_set_get_default_suppress_optional(parser):
+    parser.add_argument("--p1", type=int, default=SUPPRESS)
+    with pytest.raises(NSKeyError, match="does not specify a default"):
+        parser.get_default("p1")
+    assert parser.get_defaults() == Namespace()
+    help_str = get_parser_help(parser)
+    assert "--p1 P1     (type: int)" in help_str
+
+
+def test_set_get_default_suppress_required(parser):
+    parser.add_argument("--p2", type=int, default=SUPPRESS, required=True)
+    with pytest.raises(NSKeyError, match="does not specify a default"):
+        parser.get_default("p2")
+    assert parser.get_defaults() == Namespace()
+    help_str = get_parser_help(parser)
+    assert "--p2 P2     (required, type: int)" in help_str
+
+
+def test_set_get_default_suppress_default_config_file(parser, tmp_cwd):
+    default_config_file = tmp_cwd / "defaults.yaml"
+    default_config_file.write_text("p3: 1.2\n")
+
+    parser.default_config_files = [default_config_file]
+    parser.add_argument("--p3", type=float, default=SUPPRESS)
+    parser.add_argument("--p4", type=int, default=SUPPRESS)
+
+    assert parser.get_default("p3") == 1.2
+    with pytest.raises(NSKeyError, match="does not specify a default"):
+        parser.get_default("p4")
+
+
 def test_add_config_action_class(parser):
     parser.add_argument("--cfg", action=ActionConfigFile)
     parser.add_argument("--op", type=int)
diff --git a/jsonargparse_tests/test_subclasses.py b/jsonargparse_tests/test_subclasses.py
index 2297bff..3d5d814 100644
--- a/jsonargparse_tests/test_subclasses.py
+++ b/jsonargparse_tests/test_subclasses.py
@@ -1338,9 +1338,9 @@ def test_add_subclass_required_group(parser):
 def test_add_subclass_not_required_group(parser):
     parser.add_subclass_arguments(Calendar, "cal", required=False)
     cfg = parser.parse_args([])
-    assert cfg == Namespace()
+    assert cfg == Namespace(cal=None)
     init = parser.instantiate_classes(cfg)
-    assert init == Namespace()
+    assert init == Namespace(cal=None)
 
 
 class ListUnionA:
@@ -1723,8 +1723,8 @@ def test_subclass_error_indentation_invalid_init_arg(parser):
       Given value: abc
     """
     ).strip()
-    expected = textwrap.indent(expected, "    ")
-    assert expected in err
+    expected = textwrap.indent(expected, "        ")
+    assert "\n".join(expected.splitlines()) in "\n".join(err.splitlines())
 
 
 class ErrorIndentation2:
diff --git a/jsonargparse_tests/test_util.py b/jsonargparse_tests/test_util.py
index 1abd85f..629fb61 100644
--- a/jsonargparse_tests/test_util.py
+++ b/jsonargparse_tests/test_util.py
@@ -47,6 +47,7 @@ from jsonargparse_tests.conftest import (
     skip_if_fsspec_unavailable,
     skip_if_requests_unavailable,
     skip_if_responses_unavailable,
+    skip_if_running_as_root,
 )
 
 if responses_available:
@@ -136,6 +137,7 @@ def test_path_equality_operator(paths):
     assert Path("123", "fc") != 123
 
 
+@skip_if_running_as_root
 def test_path_file_access_mode(paths):
     Path(paths.file_rw, "frw")
     Path(paths.file_r, "fr")
@@ -150,6 +152,7 @@ def test_path_file_access_mode(paths):
     pytest.raises(TypeError, lambda: Path("file_ne", "fr"))
 
 
+@skip_if_running_as_root
 def test_path_dir_access_mode(paths):
     Path(paths.dir_rwx, "drwx")
     Path(paths.dir_rx, "drx")
@@ -166,6 +169,7 @@ def test_path_get_content(paths):
     assert "file contents" == Path(f"file://{paths.tmp_path}/{paths.file_r}", "ur").get_content()
 
 
+@skip_if_running_as_root
 def test_path_create_mode(paths):
     Path(paths.file_rw, "fcrw")
     Path(paths.tmp_path / "file_c", "fc")