Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

678 select #683

Merged
merged 3 commits into from
Apr 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions news/678.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
OmegaConf.select now returns None when attempting to select a child of a value or None node
6 changes: 3 additions & 3 deletions omegaconf/_impl.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from typing import Any

from omegaconf import MISSING, Container, DictConfig, ListConfig, Node, ValueNode
from omegaconf.errors import ConfigKeyError, InterpolationToMissingValueError
from omegaconf.errors import ConfigTypeError, InterpolationToMissingValueError

from ._utils import _DEFAULT_MARKER_, _get_value

Expand Down Expand Up @@ -94,11 +94,11 @@ def select_node(
throw_on_missing=throw_on_missing,
throw_on_resolution_failure=throw_on_resolution_failure,
)
except ConfigKeyError:
except ConfigTypeError:
if default is not _DEFAULT_MARKER_:
return default
else:
raise
return None

if (
default is not _DEFAULT_MARKER_
Expand Down
7 changes: 2 additions & 5 deletions omegaconf/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
)
from .errors import (
ConfigKeyError,
ConfigTypeError,
InterpolationKeyError,
InterpolationResolutionError,
InterpolationToMissingValueError,
Expand Down Expand Up @@ -399,7 +400,7 @@ def _select_impl(
if ret is not None and not isinstance(ret, Container):
parent_key = ".".join(split[0 : i + 1])
child_key = split[i + 1]
raise ConfigKeyError(
raise ConfigTypeError(
odelalleau marked this conversation as resolved.
Show resolved Hide resolved
f"Error trying to access {key}: node `{parent_key}` "
f"is not a container and thus cannot contain `{child_key}`"
)
Expand Down Expand Up @@ -610,10 +611,6 @@ def _resolve_node_interpolation(
raise InterpolationToMissingValueError(
f"MissingMandatoryValue while resolving interpolation: {exc}"
).with_traceback(sys.exc_info()[2])
except ConfigKeyError as exc:
raise InterpolationKeyError(
f"ConfigKeyError while resolving interpolation: {exc}"
).with_traceback(sys.exc_info()[2])

if parent is None or value is None:
raise InterpolationKeyError(f"Interpolation key '{inter_key}' not found")
Expand Down
5 changes: 4 additions & 1 deletion omegaconf/omegaconf.py
Original file line number Diff line number Diff line change
Expand Up @@ -1047,7 +1047,10 @@ def _select_one(
from .listconfig import ListConfig

ret_key: Union[str, int] = key
assert isinstance(c, (DictConfig, ListConfig)), f"Unexpected type: {c}"
assert isinstance(c, Container), f"Unexpected type: {c}"
if c._is_none():
return None, ret_key

if isinstance(c, DictConfig):
assert isinstance(ret_key, str)
val = c._get_node(ret_key, validate_access=False)
Expand Down
3 changes: 2 additions & 1 deletion tests/interpolation/test_interpolation.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
)
from omegaconf._utils import _ensure_container
from omegaconf.errors import InterpolationKeyError
from omegaconf.errors import InterpolationResolutionError
from omegaconf.errors import InterpolationResolutionError as IRE
from omegaconf.errors import InterpolationValidationError
from tests import MissingDict, MissingList, StructuredWithMissing, SubscriptedList, User
Expand Down Expand Up @@ -85,7 +86,7 @@ def test_merge_with_interpolation() -> None:

def test_non_container_interpolation() -> None:
cfg = OmegaConf.create({"foo": 0, "bar": "${foo.baz}"})
with raises(InterpolationKeyError):
with raises(InterpolationResolutionError):
cfg.bar


Expand Down
4 changes: 2 additions & 2 deletions tests/test_errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -280,9 +280,9 @@ def finalize(self, cfg: Any) -> None:
Expected(
create=lambda: OmegaConf.create({"foo": "${int.missing}", "int": 0}),
op=lambda cfg: getattr(cfg, "foo"),
exception_type=InterpolationKeyError,
exception_type=InterpolationResolutionError,
msg=(
"ConfigKeyError while resolving interpolation: Error trying to access int.missing: "
"ConfigTypeError raised while resolving interpolation: Error trying to access int.missing: "
odelalleau marked this conversation as resolved.
Show resolved Hide resolved
"node `int` is not a container and thus cannot contain `missing`"
),
key="foo",
Expand Down
69 changes: 35 additions & 34 deletions tests/test_select.py
Original file line number Diff line number Diff line change
@@ -1,27 +1,28 @@
import re
from typing import Any, Optional

from _pytest.python_api import RaisesContext
from pytest import mark, param, raises

from omegaconf import MissingMandatoryValue, OmegaConf
from omegaconf import DictConfig, ListConfig, MissingMandatoryValue, OmegaConf
from omegaconf._impl import select_value
from omegaconf._utils import _ensure_container
from omegaconf.errors import ConfigKeyError, InterpolationKeyError
from omegaconf.errors import InterpolationKeyError


@mark.parametrize("struct", [False, True, None])
@mark.parametrize("struct", [False, True])
class TestSelect:
@mark.parametrize(
"register_func",
[OmegaConf.legacy_register_resolver, OmegaConf.register_new_resolver],
)
@mark.parametrize(
"cfg, key, expected",
[
# None returned
param({}, "nope", None, id="dict:none"),
param({}, "not.there", None, id="dict:none"),
param({}, "still.not.there", None, id="dict:none"),
param({"a": 10}, "a.b", None, id="dict:nesting_into_value"),
param({"a": None}, "a.b", None, id="dict:nesting_into_none"),
param({"a": DictConfig(None)}, "a.b", None, id="dict:nesting_into_none"),
param({"a": ListConfig(None)}, "a.b", None, id="dict:nesting_into_none"),
# value returned
param({"c": 1}, "c", 1, id="dict:int"),
param({"a": {"v": 1}}, "a.v", 1, id="dict:int"),
param({"a": {"v": 1}}, "a", {"v": 1}, id="dict:dict"),
Expand All @@ -41,13 +42,38 @@ class TestSelect:
param({"a": {"v": 1}}, "", {"a": {"v": 1}}, id="select_root"),
param({"a": {"b": 1}, "c": "one=${a.b}"}, "c", "one=1", id="inter"),
param({"a": {"b": "one=${n}"}, "n": 1}, "a.b", "one=1", id="inter"),
param({"a": {"b": "one=${func:1}"}}, "a.b", "one=_1_", id="resolver"),
# relative selection
param({"a": {"b": {"c": 10}}}, ".a", {"b": {"c": 10}}, id="relative"),
param({"a": {"b": {"c": 10}}}, ".a.b", {"c": 10}, id="relative"),
],
)
def test_select(
self,
restore_resolvers: Any,
cfg: Any,
key: Any,
expected: Any,
struct: Optional[bool],
) -> None:
cfg = _ensure_container(cfg)
OmegaConf.set_struct(cfg, struct)
if isinstance(expected, RaisesContext):
with expected:
OmegaConf.select(cfg, key)
else:
assert OmegaConf.select(cfg, key) == expected

@mark.parametrize(
"register_func",
[OmegaConf.legacy_register_resolver, OmegaConf.register_new_resolver],
)
@mark.parametrize(
"cfg, key, expected",
[
param({"a": {"b": "one=${func:1}"}}, "a.b", "one=_1_", id="resolver"),
],
)
def test_select_resolver(
self,
restore_resolvers: Any,
cfg: Any,
Expand Down Expand Up @@ -107,31 +133,6 @@ def test_select_default_throw_on_missing(
with raises(MissingMandatoryValue):
OmegaConf.select(cfg, key, default=default, throw_on_missing=True)

@mark.parametrize(
("cfg", "key", "exc"),
[
param(
{"int": 0},
"int.y",
raises(
ConfigKeyError,
match=re.escape(
"Error trying to access int.y: node `int` is not a container "
"and thus cannot contain `y`"
),
),
id="non_container",
),
],
)
def test_select_error(
self, cfg: Any, key: Any, exc: Any, struct: Optional[bool]
) -> None:
cfg = _ensure_container(cfg)
OmegaConf.set_struct(cfg, struct)
with exc:
OmegaConf.select(cfg, key)

@mark.parametrize(
("cfg", "key"),
[
Expand Down