In struct mode, accessing a missing key now raises AttributeError and…

… not KeyError

.pre-commit-config.yaml

@@ -14,4 +14,4 @@ repos:
   - repo: https://github.com/pre-commit/mirrors-isort
     rev: ''  # Use the revision sha / tag you want to point at
     hooks:
-    -   id: isort
+    - id: isort

CONTRIBUTING.md

@@ -23,4 +23,6 @@ Sessions defined in /home/omry/dev/omegaconf/noxfile.py:
 ```
 To run a specific session use `-s`, for example `nox -s lint` will run linting
 
-OmegaConf is formatted with black, to format your code automatically use `black .`
+OmegaConf is formatted with black, to format your code automatically use `black .`
+
+Imports are sorted using isort, use `isort -y` to sort all imports prior to pushing.

docs/source/usage.rst

@@ -448,7 +448,7 @@ It's sometime useful to change this behavior.
     >>> conf.a.cc = 30
     Traceback (most recent call last):
     ...
-    KeyError: 'Accessing unknown key in a struct : a.cc'
+    AttributeError: 'Accessing unknown key in a struct : a.cc'
 
 You can temporarily remove the struct flag from a config object:
 

news/94.bugfix

@@ -0,0 +1 @@
+In struct mode, accessing a missing key now raises AttributeError and not KeyError

omegaconf/dictconfig.py

@@ -160,7 +160,7 @@ def get_node(
         if validate_access:
             try:
                 self._validate_access(key)
-            except KeyError:
+            except (KeyError, AttributeError):
                 if default_value is not None:
                     value = default_value
                 else:
@@ -201,7 +201,7 @@ def __contains__(self, key: Union[str, Enum]) -> bool:
 
         try:
             node: Optional[Node] = self.get_node(str_key)
-        except KeyError:
+        except (KeyError, AttributeError):
             node = None
 
         if node is None:
@@ -274,11 +274,13 @@ def _validate_access(self, key: str) -> None:
             if is_typed and node_open:
                 return
             if is_typed or is_closed:
-                raise KeyError(
-                    "Accessing unknown key in a struct : {}".format(
-                        self.get_full_key(key)
-                    )
+                msg = "Accessing unknown key in a struct : {}".format(
+                    self.get_full_key(key)
                 )
+                if is_closed:
+                    raise AttributeError(msg)
+                else:
+                    raise KeyError(msg)
 
     def _validate_type(self, key: str, value: Any) -> None:
         if self.__dict__["_type"] is not None:

tests/test_base_config.py

@@ -210,7 +210,7 @@ def test_deepcopy_struct(self, src):
         if isinstance(c2, ListConfig):
             c2.append(1000)
         elif isinstance(c2, DictConfig):
-            with pytest.raises(KeyError):
+            with pytest.raises(AttributeError):
                 c2.foo = "bar"
 
 
@@ -238,7 +238,7 @@ def test_deepcopy_and_merge_and_flags():
     )
     OmegaConf.set_struct(c1, True)
     c2 = copy.deepcopy(c1)
-    with pytest.raises(KeyError):
+    with pytest.raises(AttributeError):
         OmegaConf.merge(c2, OmegaConf.from_dotlist(["dataset.bad_key=yes"]))
 
 
@@ -262,7 +262,7 @@ def test_deepcopy_preserves_container_type(cfg):
             "struct",
             False,
             lambda c: c.__setitem__("foo", 1),
-            pytest.raises(KeyError),
+            pytest.raises(AttributeError),
         ),
         (
             {},
@@ -325,7 +325,7 @@ def test_tokenize_with_escapes(string, tokenized):
 
 @pytest.mark.parametrize(
     "src, func, expectation",
-    [({}, lambda c: c.__setattr__("foo", 1), pytest.raises(KeyError))],
+    [({}, lambda c: c.__setattr__("foo", 1), pytest.raises(AttributeError))],
 )
 def test_struct_override(src, func, expectation):
     c = OmegaConf.create(src)

tests/test_basic_ops_dict.py

@@ -485,3 +485,10 @@ def test_get_with_invalid_key():
     cfg = OmegaConf.create()
     with pytest.raises(UnsupportedKeyType):
         cfg[1]
+
+
+def test_hasattr():
+    cfg = OmegaConf.create({"foo": "bar"})
+    OmegaConf.set_struct(cfg, True)
+    assert hasattr(cfg, "foo")
+    assert not hasattr(cfg, "buz")

tests/test_nodes.py

@@ -109,7 +109,7 @@ def test_assigned_value_node_type(input_, expected_type):
 def test_get_node_no_validate_access():
     c = OmegaConf.create({"foo": "bar"})
     OmegaConf.set_struct(c, True)
-    with pytest.raises(KeyError):
+    with pytest.raises(AttributeError):
         c.get_node("zoo", validate_access=True)
 
     assert c.get_node("zoo", validate_access=False) is None

tests/test_struct.py

@@ -15,29 +15,29 @@ def test_struct_set_on_dict():
     c = OmegaConf.create(dict(a=dict()))
     OmegaConf.set_struct(c, True)
     # Throwing when it hits foo, so exception key is a.foo and not a.foo.bar
-    with pytest.raises(KeyError, match=re.escape("a.foo")):
+    with pytest.raises(AttributeError, match=re.escape("a.foo")):
         # noinspection PyStatementEffect
         c.a.foo.bar
 
 
 def test_struct_set_on_nested_dict():
     c = OmegaConf.create(dict(a=dict(b=10)))
     OmegaConf.set_struct(c, True)
-    with pytest.raises(KeyError):
+    with pytest.raises(AttributeError):
         # noinspection PyStatementEffect
         c.foo
 
     assert "a" in c
     assert c.a.b == 10
-    with pytest.raises(KeyError, match=re.escape("a.foo")):
+    with pytest.raises(AttributeError, match=re.escape("a.foo")):
         # noinspection PyStatementEffect
         c.a.foo
 
 
 def test_merge_dotlist_into_struct():
     c = OmegaConf.create(dict(a=dict(b=10)))
     OmegaConf.set_struct(c, True)
-    with pytest.raises(KeyError, match=re.escape("foo")):
+    with pytest.raises(AttributeError, match=re.escape("foo")):
         c.merge_with_dotlist(["foo=1"])
 
 
@@ -46,7 +46,7 @@ def test_merge_config_with_struct(base, merged):
     base = OmegaConf.create(base)
     merged = OmegaConf.create(merged)
     OmegaConf.set_struct(base, True)
-    with pytest.raises(KeyError):
+    with pytest.raises(AttributeError):
         OmegaConf.merge(base, merged)