Skip to content

Commit

Permalink
In struct mode, accessing a missing key now raises AttributeError and…
Browse files Browse the repository at this point in the history
… not KeyError
  • Loading branch information
omry committed Dec 29, 2019
1 parent 81ddbc9 commit 10c63d4
Show file tree
Hide file tree
Showing 7 changed files with 27 additions and 21 deletions.
2 changes: 1 addition & 1 deletion docs/source/usage.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down
1 change: 1 addition & 0 deletions news/94.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
In struct mode, accessing a missing key now raises AttributeError and not KeyError
14 changes: 8 additions & 6 deletions omegaconf/dictconfig.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down
9 changes: 4 additions & 5 deletions tests/test_base_config.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import copy

import pytest

from omegaconf import (
MISSING,
DictConfig,
Expand Down Expand Up @@ -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"


Expand Down Expand Up @@ -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"]))


Expand All @@ -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),
),
(
{},
Expand Down Expand Up @@ -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)
Expand Down
8 changes: 7 additions & 1 deletion tests/test_basic_ops_dict.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
from typing import Any

import pytest # type: ignore

from omegaconf import (
AnyNode,
BaseContainer,
Expand Down Expand Up @@ -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")
3 changes: 1 addition & 2 deletions tests/test_nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
from typing import Any

import pytest

from omegaconf import (
AnyNode,
BooleanNode,
Expand Down Expand Up @@ -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
Expand Down
11 changes: 5 additions & 6 deletions tests/test_struct.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import re

import pytest

from omegaconf import OmegaConf


Expand All @@ -15,29 +14,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"])


Expand All @@ -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)


Expand Down

0 comments on commit 10c63d4

Please sign in to comment.