From 78265e41f9223b3ca2abb915d2680c2aa7805692 Mon Sep 17 00:00:00 2001 From: Omry Yadan Date: Sun, 4 Apr 2021 17:03:28 -0700 Subject: [PATCH 1/6] OmegaConf.select is relative to the node it's called on --- omegaconf/omegaconf.py | 4 ++++ tests/test_select.py | 19 +++++-------------- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/omegaconf/omegaconf.py b/omegaconf/omegaconf.py index 1f6f76713..4e3afe68e 100644 --- a/omegaconf/omegaconf.py +++ b/omegaconf/omegaconf.py @@ -679,6 +679,10 @@ def select( ) -> Any: try: try: + if cfg._parent is not None: + # select is relative to the node its called on. + # if called on none-root, prepend dot. + key = f".{key}" cfg, key = cfg._resolve_key_and_root(key) _root, _last_key, value = cfg._select_impl( key, diff --git a/tests/test_select.py b/tests/test_select.py index 11c093156..1e5b1eec3 100644 --- a/tests/test_select.py +++ b/tests/test_select.py @@ -198,10 +198,11 @@ def test_select_relative_from_nested_node(self, struct: Optional[bool]) -> None: {"a": {"b": {"c": 10}}, "z": 10}, ) OmegaConf.set_struct(cfg, struct) - assert OmegaConf.select(cfg.a, ".") == {"b": {"c": 10}} - assert OmegaConf.select(cfg.a, "..") == {"a": {"b": {"c": 10}}, "z": 10} - assert OmegaConf.select(cfg.a, "..a") == {"b": {"c": 10}} - assert OmegaConf.select(cfg.a, "..z") == 10 + assert OmegaConf.select(cfg.a, "") == {"b": {"c": 10}} + assert OmegaConf.select(cfg.a, "b") == {"c": 10} + assert OmegaConf.select(cfg.a, ".") == {"a": {"b": {"c": 10}}, "z": 10} + assert OmegaConf.select(cfg.a, ".a") == {"b": {"c": 10}} + assert OmegaConf.select(cfg.a, ".z") == 10 @mark.parametrize( @@ -261,13 +262,3 @@ def test_select_resolves_interpolation(cfg: Any, key: str, expected: Any) -> Non OmegaConf.select(cfg, key) else: assert OmegaConf.select(cfg, key) == expected - - -def test_select_relative_from_nested_node() -> None: - cfg = OmegaConf.create( - {"a": {"b": {"c": 10}}, "z": 10}, - ) - assert OmegaConf.select(cfg.a, ".") == {"b": {"c": 10}} - assert OmegaConf.select(cfg.a, "..") == {"a": {"b": {"c": 10}}, "z": 10} - assert OmegaConf.select(cfg.a, "..a") == {"b": {"c": 10}} - assert OmegaConf.select(cfg.a, "..z") == 10 From 87335fd29dc7e5c29a1e96c776a9339833bf7398 Mon Sep 17 00:00:00 2001 From: Omry Yadan Date: Mon, 5 Apr 2021 12:20:42 -0700 Subject: [PATCH 2/6] keys relative by default --- omegaconf/omegaconf.py | 7 ++++--- tests/test_select.py | 22 ++++++++++++++-------- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/omegaconf/omegaconf.py b/omegaconf/omegaconf.py index 4e3afe68e..fe5f0ddb2 100644 --- a/omegaconf/omegaconf.py +++ b/omegaconf/omegaconf.py @@ -679,10 +679,11 @@ def select( ) -> Any: try: try: - if cfg._parent is not None: - # select is relative to the node its called on. - # if called on none-root, prepend dot. + # keys are interpreted relative by default. + # If provided key is not relative, prepend the first . + if not key.startswith("."): key = f".{key}" + cfg, key = cfg._resolve_key_and_root(key) _root, _last_key, value = cfg._select_impl( key, diff --git a/tests/test_select.py b/tests/test_select.py index 1e5b1eec3..30ccf1070 100644 --- a/tests/test_select.py +++ b/tests/test_select.py @@ -194,15 +194,21 @@ def test_select_deprecated(self, struct: Optional[bool]) -> None: cfg.select("foo") def test_select_relative_from_nested_node(self, struct: Optional[bool]) -> None: - cfg = OmegaConf.create( - {"a": {"b": {"c": 10}}, "z": 10}, - ) + input_cfg: Any = {"a": {"b": {"c": 10}}, "z": 10} + cfg = OmegaConf.create(input_cfg) OmegaConf.set_struct(cfg, struct) - assert OmegaConf.select(cfg.a, "") == {"b": {"c": 10}} - assert OmegaConf.select(cfg.a, "b") == {"c": 10} - assert OmegaConf.select(cfg.a, ".") == {"a": {"b": {"c": 10}}, "z": 10} - assert OmegaConf.select(cfg.a, ".a") == {"b": {"c": 10}} - assert OmegaConf.select(cfg.a, ".z") == 10 + # absolute keys are relative to the calling node + assert OmegaConf.select(cfg.a, "") == input_cfg["a"] + assert OmegaConf.select(cfg.a, "b") == input_cfg["a"]["b"] + assert OmegaConf.select(cfg.a, "b.c") == input_cfg["a"]["b"]["c"] + # relative keys + assert OmegaConf.select(cfg.a, ".") == input_cfg["a"] + assert OmegaConf.select(cfg.a, ".b") == input_cfg["a"]["b"] + assert OmegaConf.select(cfg.a, ".b.c") == input_cfg["a"]["b"]["c"] + assert OmegaConf.select(cfg.a, "..") == input_cfg + assert OmegaConf.select(cfg.a, "..a") == input_cfg["a"] + assert OmegaConf.select(cfg.a, "..a.b") == input_cfg["a"]["b"] + assert OmegaConf.select(cfg.a, "..z") == input_cfg["z"] @mark.parametrize( From f14bc88dd6f3734c9cad6b365285741f108e35ce Mon Sep 17 00:00:00 2001 From: Omry Yadan Date: Mon, 5 Apr 2021 12:29:23 -0700 Subject: [PATCH 3/6] absolute key flag --- omegaconf/omegaconf.py | 10 +++---- tests/test_select.py | 63 +++++++++++++++++++++++++++++++++--------- 2 files changed, 55 insertions(+), 18 deletions(-) diff --git a/omegaconf/omegaconf.py b/omegaconf/omegaconf.py index fe5f0ddb2..71a8cc652 100644 --- a/omegaconf/omegaconf.py +++ b/omegaconf/omegaconf.py @@ -676,14 +676,14 @@ def select( default: Any = _DEFAULT_MARKER_, throw_on_resolution_failure: bool = True, throw_on_missing: bool = False, + absolute_key: bool = False, ) -> Any: try: try: - # keys are interpreted relative by default. - # If provided key is not relative, prepend the first . - if not key.startswith("."): - key = f".{key}" - + if not absolute_key: + # keys are interpreted relative. If provided key is not relative, prepend the first `.` + if not key.startswith("."): + key = f".{key}" cfg, key = cfg._resolve_key_and_root(key) _root, _last_key, value = cfg._select_impl( key, diff --git a/tests/test_select.py b/tests/test_select.py index 30ccf1070..db8f6082b 100644 --- a/tests/test_select.py +++ b/tests/test_select.py @@ -193,22 +193,59 @@ def test_select_deprecated(self, struct: Optional[bool]) -> None: ): cfg.select("foo") - def test_select_relative_from_nested_node(self, struct: Optional[bool]) -> None: - input_cfg: Any = {"a": {"b": {"c": 10}}, "z": 10} - cfg = OmegaConf.create(input_cfg) + def test_select_from_nested_node_relative_key_interpretation( + self, struct: Optional[bool] + ) -> None: + inp: Any = { + "a": { + "b": { + "c": 10, + } + }, + "z": 10, + } + cfg = OmegaConf.create(inp) OmegaConf.set_struct(cfg, struct) # absolute keys are relative to the calling node - assert OmegaConf.select(cfg.a, "") == input_cfg["a"] - assert OmegaConf.select(cfg.a, "b") == input_cfg["a"]["b"] - assert OmegaConf.select(cfg.a, "b.c") == input_cfg["a"]["b"]["c"] + assert OmegaConf.select(cfg.a, "") == inp["a"] + assert OmegaConf.select(cfg.a, "b") == inp["a"]["b"] + assert OmegaConf.select(cfg.a, "b.c") == inp["a"]["b"]["c"] + # relative keys + assert OmegaConf.select(cfg.a, ".") == inp["a"] + assert OmegaConf.select(cfg.a, ".b") == inp["a"]["b"] + assert OmegaConf.select(cfg.a, ".b.c") == inp["a"]["b"]["c"] + assert OmegaConf.select(cfg.a, "..") == inp + assert OmegaConf.select(cfg.a, "..a") == inp["a"] + assert OmegaConf.select(cfg.a, "..a.b") == inp["a"]["b"] + assert OmegaConf.select(cfg.a, "..z") == inp["z"] + + def test_select_from_nested_node_absolute_key_interpretation( + self, struct: Optional[bool] + ) -> None: + inp: Any = { + "a": { + "b": { + "c": 10, + } + }, + "z": 10, + } + cfg = OmegaConf.create(inp) + OmegaConf.set_struct(cfg, struct) + # absolute keys are relative to the config root + assert OmegaConf.select(cfg.a, "", absolute_key=True) == inp + assert OmegaConf.select(cfg.a, "a", absolute_key=True) == inp["a"] + assert OmegaConf.select(cfg.a, "a.b", absolute_key=True) == inp["a"]["b"] + assert OmegaConf.select(cfg.a, "a.b.c", absolute_key=True) == inp["a"]["b"]["c"] + assert OmegaConf.select(cfg.a, "z", absolute_key=True) == inp["z"] # relative keys - assert OmegaConf.select(cfg.a, ".") == input_cfg["a"] - assert OmegaConf.select(cfg.a, ".b") == input_cfg["a"]["b"] - assert OmegaConf.select(cfg.a, ".b.c") == input_cfg["a"]["b"]["c"] - assert OmegaConf.select(cfg.a, "..") == input_cfg - assert OmegaConf.select(cfg.a, "..a") == input_cfg["a"] - assert OmegaConf.select(cfg.a, "..a.b") == input_cfg["a"]["b"] - assert OmegaConf.select(cfg.a, "..z") == input_cfg["z"] + assert OmegaConf.select(cfg.a, ".", absolute_key=True) == inp["a"] + assert OmegaConf.select(cfg.a, ".b", absolute_key=True) == inp["a"]["b"] + assert OmegaConf.select(cfg.a, ".b.c", absolute_key=True) == inp["a"]["b"]["c"] + assert OmegaConf.select(cfg.a, "..", absolute_key=True) == inp + assert OmegaConf.select(cfg.a, "..a", absolute_key=True) == inp["a"] + assert OmegaConf.select(cfg.a, "..a.b", absolute_key=True) == inp["a"]["b"] + assert OmegaConf.select(cfg.a, "..z", absolute_key=True) == inp["z"] @mark.parametrize( From 470c7e321c1d74b134d6165ba44d820174e0afd2 Mon Sep 17 00:00:00 2001 From: Omry Yadan Date: Mon, 5 Apr 2021 12:48:41 -0700 Subject: [PATCH 4/6] better tests --- omegaconf/omegaconf.py | 20 ++++++-- tests/test_select.py | 114 ++++++++++++++++++++++------------------- 2 files changed, 76 insertions(+), 58 deletions(-) diff --git a/omegaconf/omegaconf.py b/omegaconf/omegaconf.py index 71a8cc652..0b8a20327 100644 --- a/omegaconf/omegaconf.py +++ b/omegaconf/omegaconf.py @@ -678,12 +678,24 @@ def select( throw_on_missing: bool = False, absolute_key: bool = False, ) -> Any: + """ + :param cfg: Config node to select from + :param key: Key to select + :param default: Default value to return if key is not found + :param throw_on_resolution_failure: Raise an exception if an interpolation + resolution error occurs, otherwise return None + :param throw_on_missing: Raise an exception if an attempt to select a missing key (with the value '???') + is made, otherwise return None + :param absolute_key: True to treat non-relative keys as relative to the config root + False (default) to treat non-relative keys as relative to cfg + :return: selected value or None if not found. + """ try: try: - if not absolute_key: - # keys are interpreted relative. If provided key is not relative, prepend the first `.` - if not key.startswith("."): - key = f".{key}" + # keys are interpreted relative. If provided key is not relative, prepend the first `.` + if not absolute_key and not key.startswith("."): + key = f".{key}" + cfg, key = cfg._resolve_key_and_root(key) _root, _last_key, value = cfg._select_impl( key, diff --git a/tests/test_select.py b/tests/test_select.py index db8f6082b..0ea9d0444 100644 --- a/tests/test_select.py +++ b/tests/test_select.py @@ -193,60 +193,6 @@ def test_select_deprecated(self, struct: Optional[bool]) -> None: ): cfg.select("foo") - def test_select_from_nested_node_relative_key_interpretation( - self, struct: Optional[bool] - ) -> None: - inp: Any = { - "a": { - "b": { - "c": 10, - } - }, - "z": 10, - } - cfg = OmegaConf.create(inp) - OmegaConf.set_struct(cfg, struct) - # absolute keys are relative to the calling node - assert OmegaConf.select(cfg.a, "") == inp["a"] - assert OmegaConf.select(cfg.a, "b") == inp["a"]["b"] - assert OmegaConf.select(cfg.a, "b.c") == inp["a"]["b"]["c"] - # relative keys - assert OmegaConf.select(cfg.a, ".") == inp["a"] - assert OmegaConf.select(cfg.a, ".b") == inp["a"]["b"] - assert OmegaConf.select(cfg.a, ".b.c") == inp["a"]["b"]["c"] - assert OmegaConf.select(cfg.a, "..") == inp - assert OmegaConf.select(cfg.a, "..a") == inp["a"] - assert OmegaConf.select(cfg.a, "..a.b") == inp["a"]["b"] - assert OmegaConf.select(cfg.a, "..z") == inp["z"] - - def test_select_from_nested_node_absolute_key_interpretation( - self, struct: Optional[bool] - ) -> None: - inp: Any = { - "a": { - "b": { - "c": 10, - } - }, - "z": 10, - } - cfg = OmegaConf.create(inp) - OmegaConf.set_struct(cfg, struct) - # absolute keys are relative to the config root - assert OmegaConf.select(cfg.a, "", absolute_key=True) == inp - assert OmegaConf.select(cfg.a, "a", absolute_key=True) == inp["a"] - assert OmegaConf.select(cfg.a, "a.b", absolute_key=True) == inp["a"]["b"] - assert OmegaConf.select(cfg.a, "a.b.c", absolute_key=True) == inp["a"]["b"]["c"] - assert OmegaConf.select(cfg.a, "z", absolute_key=True) == inp["z"] - # relative keys - assert OmegaConf.select(cfg.a, ".", absolute_key=True) == inp["a"] - assert OmegaConf.select(cfg.a, ".b", absolute_key=True) == inp["a"]["b"] - assert OmegaConf.select(cfg.a, ".b.c", absolute_key=True) == inp["a"]["b"]["c"] - assert OmegaConf.select(cfg.a, "..", absolute_key=True) == inp - assert OmegaConf.select(cfg.a, "..a", absolute_key=True) == inp["a"] - assert OmegaConf.select(cfg.a, "..a.b", absolute_key=True) == inp["a"]["b"] - assert OmegaConf.select(cfg.a, "..z", absolute_key=True) == inp["z"] - @mark.parametrize( "cfg,key,expected", @@ -305,3 +251,63 @@ def test_select_resolves_interpolation(cfg: Any, key: str, expected: Any) -> Non OmegaConf.select(cfg, key) else: assert OmegaConf.select(cfg, key) == expected + + +inp: Any = {"a": {"b": {"c": 10}}, "z": 10} + + +class TestSelectFromNestedNode: + # all selects are performed on cfg.a: + @mark.parametrize( + ("key", "expected"), + [ + # relative keys + (".", inp["a"]), + (".b", inp["a"]["b"]), + (".b.c", inp["a"]["b"]["c"]), + ("..", inp), + ("..a", inp["a"]), + ("..a.b", inp["a"]["b"]), + ("..z", inp["z"]), + ], + ) + def test_select_from_nested_node_with_a_relative_key( + self, key: str, expected: Any + ) -> None: + cfg = OmegaConf.create(inp) + # select returns the same result when a key is relative independent of absolute_key flag. + assert OmegaConf.select(cfg.a, key, absolute_key=False) == expected + assert OmegaConf.select(cfg.a, key, absolute_key=True) == expected + + # all selects are performed on cfg.a: + @mark.parametrize( + ("key", "expected"), + [ + # absolute keys are relative to the calling node + ("", inp["a"]), + ("b", inp["a"]["b"]), + ("b.c", inp["a"]["b"]["c"]), + ], + ) + def test_select_from_nested_node_relative_key_interpretation( + self, key: str, expected: Any + ) -> None: + cfg = OmegaConf.create(inp) + assert OmegaConf.select(cfg.a, key, absolute_key=False) == expected + + @mark.parametrize( + ("key", "expected"), + [ + # absolute keys are relative to the config root + ("", inp), + ("a", inp["a"]), + ("a.b", inp["a"]["b"]), + ("a.b.c", inp["a"]["b"]["c"]), + ("z", inp["z"]), + ], + ) + def test_select_from_nested_node_absolute_key_interpretation( + self, key: str, expected: Any + ) -> None: + cfg = OmegaConf.create(inp) + assert OmegaConf.select(cfg.a, key, absolute_key=True) == expected From 76844767bcea688adc43d2bf0c309a53d2241cae Mon Sep 17 00:00:00 2001 From: Omry Yadan Date: Mon, 5 Apr 2021 13:04:00 -0700 Subject: [PATCH 5/6] minot comment update --- tests/test_select.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/test_select.py b/tests/test_select.py index 0ea9d0444..f3268e8ab 100644 --- a/tests/test_select.py +++ b/tests/test_select.py @@ -257,10 +257,10 @@ def test_select_resolves_interpolation(cfg: Any, key: str, expected: Any) -> Non class TestSelectFromNestedNode: - # all selects are performed on cfg.a: @mark.parametrize( ("key", "expected"), [ + # all selects are performed on cfg.a: # relative keys (".", inp["a"]), (".b", inp["a"]["b"]), @@ -279,10 +279,10 @@ def test_select_from_nested_node_with_a_relative_key( assert OmegaConf.select(cfg.a, key, absolute_key=False) == expected assert OmegaConf.select(cfg.a, key, absolute_key=True) == expected - # all selects are performed on cfg.a: @mark.parametrize( ("key", "expected"), [ + # all selects are performed on cfg.a: # absolute keys are relative to the calling node ("", inp["a"]), ("b", inp["a"]["b"]), @@ -298,6 +298,7 @@ def test_select_from_nested_node_relative_key_interpretation( @mark.parametrize( ("key", "expected"), [ + # all selects are performed on cfg.a: # absolute keys are relative to the config root ("", inp), ("a", inp["a"]), From d724965b98380ec554961b0eb2935f028cb5bb2f Mon Sep 17 00:00:00 2001 From: Omry Yadan Date: Mon, 5 Apr 2021 13:27:45 -0700 Subject: [PATCH 6/6] better comment --- omegaconf/omegaconf.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/omegaconf/omegaconf.py b/omegaconf/omegaconf.py index 0b8a20327..662fb6122 100644 --- a/omegaconf/omegaconf.py +++ b/omegaconf/omegaconf.py @@ -692,7 +692,10 @@ def select( """ try: try: - # keys are interpreted relative. If provided key is not relative, prepend the first `.` + # for non relative keys, the interpretation can be: + # 1. relative to cfg + # 2. relative to the config root + # This is controlled by the absolute_key flag. By default, such keys are relative to cfg. if not absolute_key and not key.startswith("."): key = f".{key}"