Repository: omni-us/jsonargparse
Pull request #201 (2022-11-29 06:19:28)

Fix Namespace method names and key clash problem
## What does this PR do?

With this change now it is possible to set/get values in a namespace having the same name as a method of the Namespace class.

## Before submitting

- [x] Did you read the [contributing guideline](https://github.com/omni-us/jsonargparse/blob/master/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]: 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=201)

[![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=201&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=201&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=omni-us_jsonargparse&pullRequest=201&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=201&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=201&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=omni-us_jsonargparse&pullRequest=201&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=201&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=201&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=omni-us_jsonargparse&pullRequest=201&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=201&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=201&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=omni-us_jsonargparse&pullRequest=201&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=201) 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=201&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=omni-us_jsonargparse&pullRequest=201&metric=new_duplicated_lines_density&view=list)



Patch:
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index a39d92d..763c7ee 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -36,6 +36,8 @@ Fixed
 - ``init_args`` not discarded for nested subclasses provided through command
   line `pytorch-lightning#15796
   <https://github.com/Lightning-AI/lightning/issues/15796>`__.
+- Unable to set/get values in ``Namespace`` when key is the same as a method
+  name.
 
 Changed
 ^^^^^^^
diff --git a/jsonargparse/namespace.py b/jsonargparse/namespace.py
index d405189..458df94 100644
--- a/jsonargparse/namespace.py
+++ b/jsonargparse/namespace.py
@@ -2,7 +2,7 @@
 
 import argparse
 from contextlib import contextmanager
-from typing import Any, Callable, Dict, Iterator, List, Optional, overload, Tuple, Union
+from typing import Any, Callable, Dict, Iterator, List, Optional, overload, Set, Tuple, Union
 
 
 __all__ = [
@@ -127,6 +127,7 @@ class Namespace(argparse.Namespace):
         key_split = split_key(key)
         if any(k == '' for k in key_split):
             raise KeyError(f'Empty nested key: "{key}".')
+        key_split = [add_clash_mark(k) for k in key_split]
         leaf_key = key_split[-1]
         parent_ns: Namespace = self
         parent_key = ''
@@ -170,7 +171,7 @@ class Namespace(argparse.Namespace):
         if '.' in name:
             self.__setitem__(name, value)
         else:
-            super().__setattr__(name, value)
+            super().__setattr__(add_clash_mark(name), value)
 
     def __setitem__(self, key: str, item: Any) -> None:
         """Sets an item to a possibly nested namespace."""
@@ -216,7 +217,7 @@ class Namespace(argparse.Namespace):
                 val = {k: v.as_dict() for k, v in val.items()}
             elif isinstance(val, list) and val != [] and all(isinstance(v, Namespace) for v in val):
                 val = [v.as_dict() for v in val]
-            dic[key] = val
+            dic[del_clash_mark(key)] = val
         return dic
 
     def as_flat(self) -> argparse.Namespace:
@@ -229,11 +230,12 @@ class Namespace(argparse.Namespace):
     def items(self, branches: bool = False) -> Iterator[Tuple[str, Any]]:
         """Returns a generator of all leaf (key, value) items, optionally including branches."""
         for key, val in vars(self).items():
+            key = del_clash_mark(key)
             if isinstance(val, Namespace):
                 if branches:
                     yield key, val
                 for subkey, subval in val.items():
-                    yield key+'.'+subkey, subval
+                    yield key+'.'+del_clash_mark(subkey), subval
             else:
                 yield key, val
 
@@ -306,6 +308,22 @@ class Namespace(argparse.Namespace):
         return parent_ns.__dict__.pop(leaf_key, default)
 
 
+clash_names: Set[str] = set(dir(Namespace))
+clash_mark = '\u200B'
+
+
+def add_clash_mark(key: str) -> str:
+    if key in clash_names:
+        key = clash_mark + key
+    return key
+
+
+def del_clash_mark(key: str) -> str:
+    if key[0] == clash_mark:
+        key = key[1:]
+    return key
+
+
 # Temporal to provide backward compatibility in pytorch-lightning
 import yaml
 yaml.SafeDumper.add_representer(Namespace, lambda d, x: d.represent_mapping('tag:yaml.org,2002:map', x.as_dict()))


Test patch:
diff --git a/jsonargparse_tests/test_namespace.py b/jsonargparse_tests/test_namespace.py
index 7a01194..90fa263 100755
--- a/jsonargparse_tests/test_namespace.py
+++ b/jsonargparse_tests/test_namespace.py
@@ -209,6 +209,54 @@ class NamespaceTests(unittest.TestCase):
         val = func(**kwargs)
         self.assertEqual(val, (4, 2, 5))
 
+    def test_shallow_clashing_keys(self):
+        ns = Namespace()
+        self.assertFalse('get' in ns)
+        exec('ns.get = 1')
+        self.assertTrue('get' in ns)
+        self.assertEqual(ns.get('get'), 1)
+        self.assertEqual(dict(ns.items()), {'get': 1})
+        ns['pop'] = 2
+        self.assertEqual(ns['pop'], 2)
+        self.assertEqual(ns.as_dict(), {'get': 1, 'pop': 2})
+        self.assertEqual(ns.pop('get'), 1)
+        self.assertEqual(dict(**ns), {'pop': 2})
+        self.assertEqual(ns.as_flat(), argparse.Namespace(pop=2))
+        del ns['pop']
+        self.assertEqual(ns, Namespace())
+        self.assertEqual(namespace_to_dict(Namespace(update=3)), {'update': 3})
+
+    def test_leaf_clashing_keys(self):
+        ns = Namespace()
+        ns['x.get'] = 1
+        self.assertTrue('x.get' in ns)
+        self.assertEqual(ns.get('x.get'), 1)
+        self.assertEqual(ns['x.get'], 1)
+        self.assertEqual(ns['x']['get'], 1)
+        self.assertEqual(ns.as_dict(), {'x': {'get': 1}})
+        self.assertEqual(dict(ns.items()), {'x.get': 1})
+        self.assertEqual(str(ns.as_flat()), "Namespace(**{'x.get': 1})")
+        self.assertEqual(ns.pop('x.get'), 1)
+        self.assertIs(ns.get('x.get'), None)
+
+    def test_shallow_branch_clashing_keys(self):
+        ns = Namespace(get=Namespace(x=2))
+        self.assertTrue('get.x' in ns)
+        self.assertEqual(ns.get('get.x'), 2)
+        self.assertEqual(ns['get.x'], 2)
+        self.assertEqual(ns['get'], Namespace(x=2))
+        self.assertEqual(ns.as_dict(), {'get': {'x': 2}})
+        self.assertEqual(dict(ns.items()), {'get.x': 2})
+        self.assertEqual(ns.pop('get.x'), 2)
+
+    def test_nested_branch_clashing_keys(self):
+        ns = Namespace()
+        ns['x.get.y'] = 3
+        self.assertTrue('x.get.y' in ns)
+        self.assertEqual(ns.get('x.get.y'), 3)
+        self.assertEqual(ns.as_dict(), {'x': {'get': {'y': 3}}})
+        self.assertEqual(ns.pop('x.get.y'), 3)
+
 
 if __name__ == '__main__':
     unittest.main(verbosity=2)