From 10c63d47706a446e57018ab4e1cf67423efbb779 Mon Sep 17 00:00:00 2001 From: omry Date: Sat, 28 Dec 2019 18:07:42 -0800 Subject: [PATCH] In struct mode, accessing a missing key now raises AttributeError and not KeyError --- docs/source/usage.rst | 2 +- news/94.bugfix | 1 + omegaconf/dictconfig.py | 14 ++++++++------ tests/test_base_config.py | 9 ++++----- tests/test_basic_ops_dict.py | 8 +++++++- tests/test_nodes.py | 3 +-- tests/test_struct.py | 11 +++++------ 7 files changed, 27 insertions(+), 21 deletions(-) create mode 100644 news/94.bugfix diff --git a/docs/source/usage.rst b/docs/source/usage.rst index a9508d10f..c6105eb13 100644 --- a/docs/source/usage.rst +++ b/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: diff --git a/news/94.bugfix b/news/94.bugfix new file mode 100644 index 000000000..eeb6a4d4c --- /dev/null +++ b/news/94.bugfix @@ -0,0 +1 @@ +In struct mode, accessing a missing key now raises AttributeError and not KeyError \ No newline at end of file diff --git a/omegaconf/dictconfig.py b/omegaconf/dictconfig.py index efe418419..a26a476bc 100644 --- a/omegaconf/dictconfig.py +++ b/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: diff --git a/tests/test_base_config.py b/tests/test_base_config.py index 06932d330..b64560ba5 100644 --- a/tests/test_base_config.py +++ b/tests/test_base_config.py @@ -1,7 +1,6 @@ import copy import pytest - from omegaconf import ( MISSING, DictConfig, @@ -210,7 +209,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 +237,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 +261,7 @@ def test_deepcopy_preserves_container_type(cfg): "struct", False, lambda c: c.__setitem__("foo", 1), - pytest.raises(KeyError), + pytest.raises(AttributeError), ), ( {}, @@ -325,7 +324,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) diff --git a/tests/test_basic_ops_dict.py b/tests/test_basic_ops_dict.py index d8f9da226..2ca9ea067 100644 --- a/tests/test_basic_ops_dict.py +++ b/tests/test_basic_ops_dict.py @@ -4,7 +4,6 @@ from typing import Any import pytest # type: ignore - from omegaconf import ( AnyNode, BaseContainer, @@ -485,3 +484,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") diff --git a/tests/test_nodes.py b/tests/test_nodes.py index 214bcef41..e0412df63 100644 --- a/tests/test_nodes.py +++ b/tests/test_nodes.py @@ -3,7 +3,6 @@ from typing import Any import pytest - from omegaconf import ( AnyNode, BooleanNode, @@ -109,7 +108,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 diff --git a/tests/test_struct.py b/tests/test_struct.py index b4daa3fb7..c188c88ab 100644 --- a/tests/test_struct.py +++ b/tests/test_struct.py @@ -1,7 +1,6 @@ import re import pytest - from omegaconf import OmegaConf @@ -15,7 +14,7 @@ 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 @@ -23,13 +22,13 @@ def test_struct_set_on_dict(): 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 @@ -37,7 +36,7 @@ def test_struct_set_on_nested_dict(): 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 +45,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)