From 8ad00dda1f4c6c3f2d5a9b5eb96f7171b70158e9 Mon Sep 17 00:00:00 2001 From: Ohad <> Date: Wed, 19 May 2021 16:10:43 +0300 Subject: [PATCH 01/17] added OmegaConf.missing_keys to get a set of missing keys --- omegaconf/omegaconf.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/omegaconf/omegaconf.py b/omegaconf/omegaconf.py index 27fc9fd5e..856ecd6a3 100644 --- a/omegaconf/omegaconf.py +++ b/omegaconf/omegaconf.py @@ -18,6 +18,7 @@ Generator, List, Optional, + Set, Tuple, Type, Union, @@ -785,6 +786,20 @@ def resolve(cfg: Container) -> None: ) omegaconf._impl._resolve(cfg) + @staticmethod + def missing_keys(cfg: Container) -> Set[str]: + missings = set() + + def gather(_cfg, prefix=""): + for key in _cfg: + if OmegaConf.is_missing(_cfg, key): + missings.add(f"{prefix}{key}") + elif OmegaConf.is_config(_cfg[key]): + gather(_cfg[key], prefix=f"{prefix}{key}.") + + gather(cfg) + return missings + # === private === # @staticmethod @@ -907,7 +922,6 @@ def flag_override( names: Union[List[str], str], values: Union[List[Optional[bool]], Optional[bool]], ) -> Generator[Node, None, None]: - if isinstance(names, str): names = [names] if values is None or isinstance(values, bool): From 7bbd9635a3232b7e3e36db93beaf75877327d7bb Mon Sep 17 00:00:00 2001 From: Ohad <> Date: Wed, 19 May 2021 16:19:38 +0300 Subject: [PATCH 02/17] =?UTF-8?q?=E2=9E=95=20Added=20annotations?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- omegaconf/omegaconf.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/omegaconf/omegaconf.py b/omegaconf/omegaconf.py index 856ecd6a3..1c674f4e3 100644 --- a/omegaconf/omegaconf.py +++ b/omegaconf/omegaconf.py @@ -790,7 +790,7 @@ def resolve(cfg: Container) -> None: def missing_keys(cfg: Container) -> Set[str]: missings = set() - def gather(_cfg, prefix=""): + def gather(_cfg: Container, prefix: str = ""): for key in _cfg: if OmegaConf.is_missing(_cfg, key): missings.add(f"{prefix}{key}") From 2ce5a4a957d0cba577ca11a9d731588ee128e83c Mon Sep 17 00:00:00 2001 From: Ohad <> Date: Wed, 19 May 2021 17:17:19 +0300 Subject: [PATCH 03/17] =?UTF-8?q?=E2=9E=95=20return=20type=20annotation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- omegaconf/omegaconf.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/omegaconf/omegaconf.py b/omegaconf/omegaconf.py index 1c674f4e3..f3b242f28 100644 --- a/omegaconf/omegaconf.py +++ b/omegaconf/omegaconf.py @@ -790,7 +790,7 @@ def resolve(cfg: Container) -> None: def missing_keys(cfg: Container) -> Set[str]: missings = set() - def gather(_cfg: Container, prefix: str = ""): + def gather(_cfg: Container, prefix: str = "") -> None: for key in _cfg: if OmegaConf.is_missing(_cfg, key): missings.add(f"{prefix}{key}") From ecc766f698a45a708847e99c7a54dfecddd6c9db Mon Sep 17 00:00:00 2001 From: Ohad <> Date: Wed, 19 May 2021 17:31:20 +0300 Subject: [PATCH 04/17] =?UTF-8?q?=E2=9E=95=20docstring?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- omegaconf/omegaconf.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/omegaconf/omegaconf.py b/omegaconf/omegaconf.py index f3b242f28..5787f5e51 100644 --- a/omegaconf/omegaconf.py +++ b/omegaconf/omegaconf.py @@ -788,6 +788,11 @@ def resolve(cfg: Container) -> None: @staticmethod def missing_keys(cfg: Container) -> Set[str]: + """ + Returns a set of missing keys flatten in a dotlist style. + :param cfg: An OmegaConf container . + :return: set of strings of the missing keys. + """ missings = set() def gather(_cfg: Container, prefix: str = "") -> None: From 7f8cd7484c22f06bd785bfc5ae7005c0bac103d3 Mon Sep 17 00:00:00 2001 From: Ohad <> Date: Fri, 4 Jun 2021 11:13:38 +0300 Subject: [PATCH 05/17] =?UTF-8?q?=E2=9E=95tests,=20docs,=20news?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/source/usage.rst | 18 +++ news/720.feature | 1 + omegaconf/omegaconf.py | 18 ++- tests/test_omegaconf.py | 271 +++++++++++++++++++++++++++++----------- 4 files changed, 232 insertions(+), 76 deletions(-) create mode 100644 news/720.feature diff --git a/docs/source/usage.rst b/docs/source/usage.rst index 9772d94c9..bb60c4e69 100644 --- a/docs/source/usage.rst +++ b/docs/source/usage.rst @@ -782,6 +782,24 @@ and ``OmegaConf.is_list(cfg)`` is equivalent to ``isinstance(cfg, ListConfig)``. >>> assert not OmegaConf.is_dict(l) +OmegaConf.missing_keys +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +Gives a set of missing keys represented in a dotlist style. +This utility function can be used at the end of creating configuration, after merging sources and so on, +to check for remaining mandatory fields and prompt a proper error message. + +.. doctest:: + + >>> OmegaConf.missing_keys({ + ... "foo": {"bar": "???"}, + ... "missing": "???", + ... "list": ["a", None, "???"] + ... }) + {'missing', 'list.2', 'foo.bar'} + +A plain dict, list or any valid input for `OmegaConf.create` is also acceptable - the function would raise a `ValueError` exception on wrong input. + + Debugger integration -------------------- diff --git a/news/720.feature b/news/720.feature new file mode 100644 index 000000000..f44ad8cc7 --- /dev/null +++ b/news/720.feature @@ -0,0 +1 @@ +New `OmegaConf.missing_keys` method that enables to get a config's set of missing keys represented in a dotlist style. diff --git a/omegaconf/omegaconf.py b/omegaconf/omegaconf.py index 5787f5e51..4f204e86d 100644 --- a/omegaconf/omegaconf.py +++ b/omegaconf/omegaconf.py @@ -787,16 +787,28 @@ def resolve(cfg: Container) -> None: omegaconf._impl._resolve(cfg) @staticmethod - def missing_keys(cfg: Container) -> Set[str]: + def missing_keys(cfg: Union[Container, Any]) -> Set[str]: """ Returns a set of missing keys flatten in a dotlist style. - :param cfg: An OmegaConf container . + :param cfg: An `OmegaConf.Container`, + or convertible object via `OmegaConf.create` (dict, list, ...). :return: set of strings of the missing keys. + :raises ValueError: On input not representing a config. """ + if not isinstance(cfg, Container): + try: + cfg = OmegaConf.create(cfg) # type: ignore + except ValidationError: + raise ValueError(f'Could not create a config out of {cfg}') missings = set() def gather(_cfg: Container, prefix: str = "") -> None: - for key in _cfg: + if isinstance(_cfg, ListConfig): + itr = range(len(_cfg)) + else: + itr = _cfg + + for key in itr: if OmegaConf.is_missing(_cfg, key): missings.add(f"{prefix}{key}") elif OmegaConf.is_config(_cfg[key]): diff --git a/tests/test_omegaconf.py b/tests/test_omegaconf.py index dcc0c8311..1b2159e93 100644 --- a/tests/test_omegaconf.py +++ b/tests/test_omegaconf.py @@ -68,29 +68,29 @@ id="missing_dict_interpolation", ), ( - {"foo": "${bar}", "bar": MISSING}, - "foo", - False, - raises(InterpolationToMissingValueError), + {"foo": "${bar}", "bar": MISSING}, + "foo", + False, + raises(InterpolationToMissingValueError), ), ( - {"foo": "foo_${bar}", "bar": MISSING}, - "foo", - False, - raises(InterpolationToMissingValueError), + {"foo": "foo_${bar}", "bar": MISSING}, + "foo", + False, + raises(InterpolationToMissingValueError), ), ( - {"foo": "${unknown_resolver:foo}"}, - "foo", - False, - raises(UnsupportedInterpolationType), + {"foo": "${unknown_resolver:foo}"}, + "foo", + False, + raises(UnsupportedInterpolationType), ), ({"foo": StringNode(value="???")}, "foo", True, raises(MissingMandatoryValue)), ( - {"foo": StringNode(value="???"), "inter": "${foo}"}, - "inter", - False, - raises(InterpolationToMissingValueError), + {"foo": StringNode(value="???"), "inter": "${foo}"}, + "inter", + False, + raises(InterpolationToMissingValueError), ), (StructuredWithMissing, "num", True, raises(MissingMandatoryValue)), (StructuredWithMissing, "opt_num", True, raises(MissingMandatoryValue)), @@ -101,27 +101,27 @@ (StructuredWithMissing, "user", True, raises(MissingMandatoryValue)), (StructuredWithMissing, "opt_user", True, raises(MissingMandatoryValue)), ( - StructuredWithMissing, - "inter_user", - False, - raises(InterpolationToMissingValueError), + StructuredWithMissing, + "inter_user", + False, + raises(InterpolationToMissingValueError), ), ( - StructuredWithMissing, - "inter_opt_user", - False, - raises(InterpolationToMissingValueError), + StructuredWithMissing, + "inter_opt_user", + False, + raises(InterpolationToMissingValueError), ), ( - StructuredWithMissing, - "inter_num", - False, - raises(InterpolationToMissingValueError), + StructuredWithMissing, + "inter_num", + False, + raises(InterpolationToMissingValueError), ), ], ) def test_is_missing( - cfg: Any, key: str, expected_is_missing: bool, expectation: Any + cfg: Any, key: str, expected_is_missing: bool, expectation: Any ) -> None: cfg = OmegaConf.create(cfg) with expectation: @@ -240,28 +240,28 @@ def test_coverage_for_deprecated_OmegaConf_is_optional() -> None: (lambda none: FloatNode(value=10 if not none else None, is_optional=True)), (lambda none: BooleanNode(value=True if not none else None, is_optional=True)), ( - lambda none: EnumNode( - enum_type=Color, - value=Color.RED if not none else None, - is_optional=True, - ) + lambda none: EnumNode( + enum_type=Color, + value=Color.RED if not none else None, + is_optional=True, + ) ), ( - lambda none: ListConfig( - content=[1, 2, 3] if not none else None, is_optional=True - ) + lambda none: ListConfig( + content=[1, 2, 3] if not none else None, is_optional=True + ) ), ( - lambda none: DictConfig( - content={"foo": "bar"} if not none else None, is_optional=True - ) + lambda none: DictConfig( + content={"foo": "bar"} if not none else None, is_optional=True + ) ), ( - lambda none: DictConfig( - ref_type=ConcretePlugin, - content=ConcretePlugin() if not none else None, - is_optional=True, - ) + lambda none: DictConfig( + ref_type=ConcretePlugin, + content=ConcretePlugin() if not none else None, + is_optional=True, + ) ), ], ) @@ -302,48 +302,48 @@ def test_is_none_invalid_node() -> None: "fac", [ ( - lambda inter: StringNode( - value="foo" if inter is None else inter, is_optional=True - ) + lambda inter: StringNode( + value="foo" if inter is None else inter, is_optional=True + ) ), ( - lambda inter: IntegerNode( - value=10 if inter is None else inter, is_optional=True - ) + lambda inter: IntegerNode( + value=10 if inter is None else inter, is_optional=True + ) ), ( - lambda inter: FloatNode( - value=10 if inter is None else inter, is_optional=True - ) + lambda inter: FloatNode( + value=10 if inter is None else inter, is_optional=True + ) ), ( - lambda inter: BooleanNode( - value=True if inter is None else inter, is_optional=True - ) + lambda inter: BooleanNode( + value=True if inter is None else inter, is_optional=True + ) ), ( - lambda inter: EnumNode( - enum_type=Color, - value=Color.RED if inter is None else inter, - is_optional=True, - ) + lambda inter: EnumNode( + enum_type=Color, + value=Color.RED if inter is None else inter, + is_optional=True, + ) ), ( - lambda inter: ListConfig( - content=[1, 2, 3] if inter is None else inter, is_optional=True - ) + lambda inter: ListConfig( + content=[1, 2, 3] if inter is None else inter, is_optional=True + ) ), ( - lambda inter: DictConfig( - content={"foo": "bar"} if inter is None else inter, is_optional=True - ) + lambda inter: DictConfig( + content={"foo": "bar"} if inter is None else inter, is_optional=True + ) ), ( - lambda inter: DictConfig( - ref_type=ConcretePlugin, - content=ConcretePlugin() if inter is None else inter, - is_optional=True, - ) + lambda inter: DictConfig( + ref_type=ConcretePlugin, + content=ConcretePlugin() if inter is None else inter, + is_optional=True, + ) ), ], ids=[ @@ -518,3 +518,128 @@ def test_resolve(cfg: Any, expected: Any) -> None: def test_resolve_invalid_input() -> None: with raises(ValueError): OmegaConf.resolve("aaa") # type: ignore + + +@mark.parametrize( + "cfg, expected", + [ + # dict: + ( + DictConfig({ + "a": 10, + "b": { + "c": "???", + "d": "..." + } + }), + {"b.c"} + ), + ( + DictConfig({ + "a": "???", + "b": { + "foo": "bar", + "bar": "???", + "more": { + "missing": "???", + "available": "yes" + } + }, + }), + {"a", "b.bar", "b.more.missing"} + ), + ( + DictConfig({ + "a": "a", + "b": { + "foo": "bar", + "bar": "foo", + }, + }), + set() + ), + ( + { + "foo": "bar", + "bar": "???", + "more": { + "foo": "???", + "bar": "foo" + } + }, + {"bar", "more.foo"} + ), + + # list: + ( + ListConfig([ + "???", + "foo", + "bar", + "???", + 77 + ]), + {"0", "3"} + ), + ( + ListConfig([ + "", + "foo", + "bar" + ]), + set() + ), + ( + ["foo", "bar", "???"], + {"2"} + ), + + # mixing: + ( + ListConfig([ + "???", + "foo", + DictConfig({ + "a": True, + "b": "???", + "c": ["???", None], + "d": { + "e": "???", + "f": "fff", + "g": [True, "???"] + } + }), + "???", + 77 + ]), + {"0", "2.b", "2.c.0", "2.d.e", "2.d.g.1", "3"} + ), + ( + { + "list": [ + 0, + DictConfig({ + "foo": "???", + "bar": None + }), + "???", + ["???", 3, False] + ], + "x": "y", + "y": "???" + }, + {"list.1.foo", "list.2", "list.3.0", "y"} + ) + ] +) +def test_missing_keys(cfg: Any, expected: Any) -> None: + assert OmegaConf.missing_keys(cfg) == expected + + +@mark.parametrize( + "cfg", + [float, int] +) +def test_missing_keys_invalid_input(cfg): + with raises(ValueError) as exc: + OmegaConf.missing_keys(cfg) # type: ignore From 49b9d4642535edc1f5f7c1fd3744aeb0570e1e49 Mon Sep 17 00:00:00 2001 From: Ohad <> Date: Fri, 4 Jun 2021 11:32:09 +0300 Subject: [PATCH 06/17] missing annotations, syntax --- omegaconf/omegaconf.py | 9 +++++---- tests/test_omegaconf.py | 6 +++--- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/omegaconf/omegaconf.py b/omegaconf/omegaconf.py index 4f204e86d..0edbbe986 100644 --- a/omegaconf/omegaconf.py +++ b/omegaconf/omegaconf.py @@ -797,16 +797,17 @@ def missing_keys(cfg: Union[Container, Any]) -> Set[str]: """ if not isinstance(cfg, Container): try: - cfg = OmegaConf.create(cfg) # type: ignore + cfg = OmegaConf.create(cfg) except ValidationError: - raise ValueError(f'Could not create a config out of {cfg}') + raise ValueError(f"Could not create a config out of {cfg}") + cfg: Container missings = set() def gather(_cfg: Container, prefix: str = "") -> None: if isinstance(_cfg, ListConfig): - itr = range(len(_cfg)) + itr = range(len(_cfg)) # type: ignore else: - itr = _cfg + itr = _cfg # type: ignore for key in itr: if OmegaConf.is_missing(_cfg, key): diff --git a/tests/test_omegaconf.py b/tests/test_omegaconf.py index 1b2159e93..11effb244 100644 --- a/tests/test_omegaconf.py +++ b/tests/test_omegaconf.py @@ -640,6 +640,6 @@ def test_missing_keys(cfg: Any, expected: Any) -> None: "cfg", [float, int] ) -def test_missing_keys_invalid_input(cfg): - with raises(ValueError) as exc: - OmegaConf.missing_keys(cfg) # type: ignore +def test_missing_keys_invalid_input(cfg: Any) -> None: + with raises(ValueError): + OmegaConf.missing_keys(cfg) From b1bac7b22ef0f3d0e3697792eb9c71360c27af49 Mon Sep 17 00:00:00 2001 From: Ohad <> Date: Fri, 4 Jun 2021 11:51:22 +0300 Subject: [PATCH 07/17] mypy error, docs output reorder --- docs/source/usage.rst | 2 +- omegaconf/omegaconf.py | 12 +++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/docs/source/usage.rst b/docs/source/usage.rst index bb60c4e69..de0653a2d 100644 --- a/docs/source/usage.rst +++ b/docs/source/usage.rst @@ -795,7 +795,7 @@ to check for remaining mandatory fields and prompt a proper error message. ... "missing": "???", ... "list": ["a", None, "???"] ... }) - {'missing', 'list.2', 'foo.bar'} + {'list.2', 'foo.bar', 'missing'} A plain dict, list or any valid input for `OmegaConf.create` is also acceptable - the function would raise a `ValueError` exception on wrong input. diff --git a/omegaconf/omegaconf.py b/omegaconf/omegaconf.py index 0edbbe986..9fdf4e51b 100644 --- a/omegaconf/omegaconf.py +++ b/omegaconf/omegaconf.py @@ -797,17 +797,19 @@ def missing_keys(cfg: Union[Container, Any]) -> Set[str]: """ if not isinstance(cfg, Container): try: - cfg = OmegaConf.create(cfg) + cfg_ = OmegaConf.create(cfg) except ValidationError: raise ValueError(f"Could not create a config out of {cfg}") - cfg: Container + else: + cfg_ = cfg missings = set() def gather(_cfg: Container, prefix: str = "") -> None: + itr: Any if isinstance(_cfg, ListConfig): - itr = range(len(_cfg)) # type: ignore + itr = range(len(_cfg)) else: - itr = _cfg # type: ignore + itr = _cfg for key in itr: if OmegaConf.is_missing(_cfg, key): @@ -815,7 +817,7 @@ def gather(_cfg: Container, prefix: str = "") -> None: elif OmegaConf.is_config(_cfg[key]): gather(_cfg[key], prefix=f"{prefix}{key}.") - gather(cfg) + gather(cfg_) return missings # === private === # From 13dcd1b29033d8d199aea4becffd98a6555c4e0d Mon Sep 17 00:00:00 2001 From: Ohad <> Date: Fri, 4 Jun 2021 16:46:42 +0300 Subject: [PATCH 08/17] annotations, black --- omegaconf/omegaconf.py | 15 +-- tests/test_omegaconf.py | 288 +++++++++++++++++----------------------- 2 files changed, 130 insertions(+), 173 deletions(-) diff --git a/omegaconf/omegaconf.py b/omegaconf/omegaconf.py index 9fdf4e51b..4ecc3fd10 100644 --- a/omegaconf/omegaconf.py +++ b/omegaconf/omegaconf.py @@ -16,6 +16,7 @@ Callable, Dict, Generator, + Iterable, List, Optional, Set, @@ -787,25 +788,23 @@ def resolve(cfg: Container) -> None: omegaconf._impl._resolve(cfg) @staticmethod - def missing_keys(cfg: Union[Container, Any]) -> Set[str]: + def missing_keys(cfg: Any) -> Set[str]: """ Returns a set of missing keys flatten in a dotlist style. :param cfg: An `OmegaConf.Container`, - or convertible object via `OmegaConf.create` (dict, list, ...). + or a convertible object via `OmegaConf.create` (dict, list, ...). :return: set of strings of the missing keys. :raises ValueError: On input not representing a config. """ if not isinstance(cfg, Container): try: - cfg_ = OmegaConf.create(cfg) + cfg = OmegaConf.create(cfg) except ValidationError: raise ValueError(f"Could not create a config out of {cfg}") - else: - cfg_ = cfg - missings = set() + missings: Set[str] = set() def gather(_cfg: Container, prefix: str = "") -> None: - itr: Any + itr: Iterable[Any] if isinstance(_cfg, ListConfig): itr = range(len(_cfg)) else: @@ -817,7 +816,7 @@ def gather(_cfg: Container, prefix: str = "") -> None: elif OmegaConf.is_config(_cfg[key]): gather(_cfg[key], prefix=f"{prefix}{key}.") - gather(cfg_) + gather(cfg) return missings # === private === # diff --git a/tests/test_omegaconf.py b/tests/test_omegaconf.py index 11effb244..a50a9f2a8 100644 --- a/tests/test_omegaconf.py +++ b/tests/test_omegaconf.py @@ -68,29 +68,29 @@ id="missing_dict_interpolation", ), ( - {"foo": "${bar}", "bar": MISSING}, - "foo", - False, - raises(InterpolationToMissingValueError), + {"foo": "${bar}", "bar": MISSING}, + "foo", + False, + raises(InterpolationToMissingValueError), ), ( - {"foo": "foo_${bar}", "bar": MISSING}, - "foo", - False, - raises(InterpolationToMissingValueError), + {"foo": "foo_${bar}", "bar": MISSING}, + "foo", + False, + raises(InterpolationToMissingValueError), ), ( - {"foo": "${unknown_resolver:foo}"}, - "foo", - False, - raises(UnsupportedInterpolationType), + {"foo": "${unknown_resolver:foo}"}, + "foo", + False, + raises(UnsupportedInterpolationType), ), ({"foo": StringNode(value="???")}, "foo", True, raises(MissingMandatoryValue)), ( - {"foo": StringNode(value="???"), "inter": "${foo}"}, - "inter", - False, - raises(InterpolationToMissingValueError), + {"foo": StringNode(value="???"), "inter": "${foo}"}, + "inter", + False, + raises(InterpolationToMissingValueError), ), (StructuredWithMissing, "num", True, raises(MissingMandatoryValue)), (StructuredWithMissing, "opt_num", True, raises(MissingMandatoryValue)), @@ -101,27 +101,27 @@ (StructuredWithMissing, "user", True, raises(MissingMandatoryValue)), (StructuredWithMissing, "opt_user", True, raises(MissingMandatoryValue)), ( - StructuredWithMissing, - "inter_user", - False, - raises(InterpolationToMissingValueError), + StructuredWithMissing, + "inter_user", + False, + raises(InterpolationToMissingValueError), ), ( - StructuredWithMissing, - "inter_opt_user", - False, - raises(InterpolationToMissingValueError), + StructuredWithMissing, + "inter_opt_user", + False, + raises(InterpolationToMissingValueError), ), ( - StructuredWithMissing, - "inter_num", - False, - raises(InterpolationToMissingValueError), + StructuredWithMissing, + "inter_num", + False, + raises(InterpolationToMissingValueError), ), ], ) def test_is_missing( - cfg: Any, key: str, expected_is_missing: bool, expectation: Any + cfg: Any, key: str, expected_is_missing: bool, expectation: Any ) -> None: cfg = OmegaConf.create(cfg) with expectation: @@ -240,28 +240,28 @@ def test_coverage_for_deprecated_OmegaConf_is_optional() -> None: (lambda none: FloatNode(value=10 if not none else None, is_optional=True)), (lambda none: BooleanNode(value=True if not none else None, is_optional=True)), ( - lambda none: EnumNode( - enum_type=Color, - value=Color.RED if not none else None, - is_optional=True, - ) + lambda none: EnumNode( + enum_type=Color, + value=Color.RED if not none else None, + is_optional=True, + ) ), ( - lambda none: ListConfig( - content=[1, 2, 3] if not none else None, is_optional=True - ) + lambda none: ListConfig( + content=[1, 2, 3] if not none else None, is_optional=True + ) ), ( - lambda none: DictConfig( - content={"foo": "bar"} if not none else None, is_optional=True - ) + lambda none: DictConfig( + content={"foo": "bar"} if not none else None, is_optional=True + ) ), ( - lambda none: DictConfig( - ref_type=ConcretePlugin, - content=ConcretePlugin() if not none else None, - is_optional=True, - ) + lambda none: DictConfig( + ref_type=ConcretePlugin, + content=ConcretePlugin() if not none else None, + is_optional=True, + ) ), ], ) @@ -302,48 +302,48 @@ def test_is_none_invalid_node() -> None: "fac", [ ( - lambda inter: StringNode( - value="foo" if inter is None else inter, is_optional=True - ) + lambda inter: StringNode( + value="foo" if inter is None else inter, is_optional=True + ) ), ( - lambda inter: IntegerNode( - value=10 if inter is None else inter, is_optional=True - ) + lambda inter: IntegerNode( + value=10 if inter is None else inter, is_optional=True + ) ), ( - lambda inter: FloatNode( - value=10 if inter is None else inter, is_optional=True - ) + lambda inter: FloatNode( + value=10 if inter is None else inter, is_optional=True + ) ), ( - lambda inter: BooleanNode( - value=True if inter is None else inter, is_optional=True - ) + lambda inter: BooleanNode( + value=True if inter is None else inter, is_optional=True + ) ), ( - lambda inter: EnumNode( - enum_type=Color, - value=Color.RED if inter is None else inter, - is_optional=True, - ) + lambda inter: EnumNode( + enum_type=Color, + value=Color.RED if inter is None else inter, + is_optional=True, + ) ), ( - lambda inter: ListConfig( - content=[1, 2, 3] if inter is None else inter, is_optional=True - ) + lambda inter: ListConfig( + content=[1, 2, 3] if inter is None else inter, is_optional=True + ) ), ( - lambda inter: DictConfig( - content={"foo": "bar"} if inter is None else inter, is_optional=True - ) + lambda inter: DictConfig( + content={"foo": "bar"} if inter is None else inter, is_optional=True + ) ), ( - lambda inter: DictConfig( - ref_type=ConcretePlugin, - content=ConcretePlugin() if inter is None else inter, - is_optional=True, - ) + lambda inter: DictConfig( + ref_type=ConcretePlugin, + content=ConcretePlugin() if inter is None else inter, + is_optional=True, + ) ), ], ids=[ @@ -524,122 +524,80 @@ def test_resolve_invalid_input() -> None: "cfg, expected", [ # dict: + (DictConfig({"a": 10, "b": {"c": "???", "d": "..."}}), {"b.c"}), ( - DictConfig({ - "a": 10, - "b": { - "c": "???", - "d": "..." + DictConfig( + { + "a": "???", + "b": { + "foo": "bar", + "bar": "???", + "more": {"missing": "???", "available": "yes"}, + }, } - }), - {"b.c"} - ), - ( - DictConfig({ - "a": "???", - "b": { - "foo": "bar", - "bar": "???", - "more": { - "missing": "???", - "available": "yes" - } - }, - }), - {"a", "b.bar", "b.more.missing"} + ), + {"a", "b.bar", "b.more.missing"}, ), ( - DictConfig({ - "a": "a", - "b": { - "foo": "bar", - "bar": "foo", - }, - }), - set() - ), - ( - { - "foo": "bar", - "bar": "???", - "more": { - "foo": "???", - "bar": "foo" + DictConfig( + { + "a": "a", + "b": { + "foo": "bar", + "bar": "foo", + }, } - }, - {"bar", "more.foo"} - ), - - # list: - ( - ListConfig([ - "???", - "foo", - "bar", - "???", - 77 - ]), - {"0", "3"} - ), - ( - ListConfig([ - "", - "foo", - "bar" - ]), - set() + ), + set(), ), ( - ["foo", "bar", "???"], - {"2"} + {"foo": "bar", "bar": "???", "more": {"foo": "???", "bar": "foo"}}, + {"bar", "more.foo"}, ), - + # list: + (ListConfig(["???", "foo", "bar", "???", 77]), {"0", "3"}), + (ListConfig(["", "foo", "bar"]), set()), + (["foo", "bar", "???"], {"2"}), # mixing: ( - ListConfig([ - "???", - "foo", - DictConfig({ - "a": True, - "b": "???", - "c": ["???", None], - "d": { - "e": "???", - "f": "fff", - "g": [True, "???"] - } - }), - "???", - 77 - ]), - {"0", "2.b", "2.c.0", "2.d.e", "2.d.g.1", "3"} + ListConfig( + [ + "???", + "foo", + DictConfig( + { + "a": True, + "b": "???", + "c": ["???", None], + "d": {"e": "???", "f": "fff", "g": [True, "???"]}, + } + ), + "???", + 77, + ] + ), + {"0", "2.b", "2.c.0", "2.d.e", "2.d.g.1", "3"}, ), ( { "list": [ 0, - DictConfig({ - "foo": "???", - "bar": None - }), + DictConfig({"foo": "???", "bar": None}), "???", - ["???", 3, False] + ["???", 3, False], ], "x": "y", - "y": "???" + "y": "???", }, - {"list.1.foo", "list.2", "list.3.0", "y"} - ) - ] + {"list.1.foo", "list.2", "list.3.0", "y"}, + ), + ], ) def test_missing_keys(cfg: Any, expected: Any) -> None: assert OmegaConf.missing_keys(cfg) == expected -@mark.parametrize( - "cfg", - [float, int] -) +@mark.parametrize("cfg", [float, int]) def test_missing_keys_invalid_input(cfg: Any) -> None: with raises(ValueError): OmegaConf.missing_keys(cfg) From 5df391017df4fa11920664c4be79a86398618904 Mon Sep 17 00:00:00 2001 From: Ohad <> Date: Sun, 6 Jun 2021 09:20:56 +0300 Subject: [PATCH 09/17] Using _get_full_key, updated docs --- docs/source/usage.rst | 4 ++-- omegaconf/omegaconf.py | 6 +++--- tests/test_omegaconf.py | 23 +++++++++-------------- 3 files changed, 14 insertions(+), 19 deletions(-) diff --git a/docs/source/usage.rst b/docs/source/usage.rst index de0653a2d..aa9f8b545 100644 --- a/docs/source/usage.rst +++ b/docs/source/usage.rst @@ -790,12 +790,12 @@ to check for remaining mandatory fields and prompt a proper error message. .. doctest:: - >>> OmegaConf.missing_keys({ + >>> missings = OmegaConf.missing_keys({ ... "foo": {"bar": "???"}, ... "missing": "???", ... "list": ["a", None, "???"] ... }) - {'list.2', 'foo.bar', 'missing'} + >>> assert missings == {'list[2]', 'foo.bar', 'missing'} A plain dict, list or any valid input for `OmegaConf.create` is also acceptable - the function would raise a `ValueError` exception on wrong input. diff --git a/omegaconf/omegaconf.py b/omegaconf/omegaconf.py index 4ecc3fd10..2bdd22f1d 100644 --- a/omegaconf/omegaconf.py +++ b/omegaconf/omegaconf.py @@ -803,7 +803,7 @@ def missing_keys(cfg: Any) -> Set[str]: raise ValueError(f"Could not create a config out of {cfg}") missings: Set[str] = set() - def gather(_cfg: Container, prefix: str = "") -> None: + def gather(_cfg: Container) -> None: itr: Iterable[Any] if isinstance(_cfg, ListConfig): itr = range(len(_cfg)) @@ -812,9 +812,9 @@ def gather(_cfg: Container, prefix: str = "") -> None: for key in itr: if OmegaConf.is_missing(_cfg, key): - missings.add(f"{prefix}{key}") + missings.add(_cfg._get_full_key(key)) elif OmegaConf.is_config(_cfg[key]): - gather(_cfg[key], prefix=f"{prefix}{key}.") + gather(_cfg[key]) gather(cfg) return missings diff --git a/tests/test_omegaconf.py b/tests/test_omegaconf.py index a50a9f2a8..9f5b27700 100644 --- a/tests/test_omegaconf.py +++ b/tests/test_omegaconf.py @@ -534,20 +534,13 @@ def test_resolve_invalid_input() -> None: "bar": "???", "more": {"missing": "???", "available": "yes"}, }, + Color.GREEN: {"tint": "???", "default": Color.BLUE} } ), - {"a", "b.bar", "b.more.missing"}, + {"a", "b.bar", "b.more.missing", "GREEN.tint"}, ), ( - DictConfig( - { - "a": "a", - "b": { - "foo": "bar", - "bar": "foo", - }, - } - ), + DictConfig({"a": "a", "b": {"foo": "bar", "bar": "foo"}}), set(), ), ( @@ -555,9 +548,10 @@ def test_resolve_invalid_input() -> None: {"bar", "more.foo"}, ), # list: - (ListConfig(["???", "foo", "bar", "???", 77]), {"0", "3"}), + (ListConfig(["???", "foo", "bar", "???", 77]), {"[0]", "[3]"}), (ListConfig(["", "foo", "bar"]), set()), - (["foo", "bar", "???"], {"2"}), + (["foo", "bar", "???"], {"[2]"}), + (["foo", "???", ["???", "bar"]], {"[1]", "[2][0]"}), # mixing: ( ListConfig( @@ -576,7 +570,7 @@ def test_resolve_invalid_input() -> None: 77, ] ), - {"0", "2.b", "2.c.0", "2.d.e", "2.d.g.1", "3"}, + {"[0]", "[2].b", "[2].c[0]", "[2].d.e", "[2].d.g[1]", "[3]"}, ), ( { @@ -589,8 +583,9 @@ def test_resolve_invalid_input() -> None: "x": "y", "y": "???", }, - {"list.1.foo", "list.2", "list.3.0", "y"}, + {"list[1].foo", "list[2]", "list[3][0]", "y"}, ), + ({Color.RED: ["???", {"missing": "???"}]}, {"RED[0]", "RED[1].missing"}) ], ) def test_missing_keys(cfg: Any, expected: Any) -> None: From c68242f533188416c3fdf3fa754f98d1edaa690b Mon Sep 17 00:00:00 2001 From: Ohad <> Date: Sun, 6 Jun 2021 09:32:48 +0300 Subject: [PATCH 10/17] black --- tests/test_omegaconf.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_omegaconf.py b/tests/test_omegaconf.py index 9f5b27700..335aef572 100644 --- a/tests/test_omegaconf.py +++ b/tests/test_omegaconf.py @@ -534,7 +534,7 @@ def test_resolve_invalid_input() -> None: "bar": "???", "more": {"missing": "???", "available": "yes"}, }, - Color.GREEN: {"tint": "???", "default": Color.BLUE} + Color.GREEN: {"tint": "???", "default": Color.BLUE}, } ), {"a", "b.bar", "b.more.missing", "GREEN.tint"}, @@ -585,7 +585,7 @@ def test_resolve_invalid_input() -> None: }, {"list[1].foo", "list[2]", "list[3][0]", "y"}, ), - ({Color.RED: ["???", {"missing": "???"}]}, {"RED[0]", "RED[1].missing"}) + ({Color.RED: ["???", {"missing": "???"}]}, {"RED[0]", "RED[1].missing"}), ], ) def test_missing_keys(cfg: Any, expected: Any) -> None: From b1ef193ed04300ea430ba0fe9c0de3eb80b51e39 Mon Sep 17 00:00:00 2001 From: Ohad <> Date: Wed, 9 Jun 2021 10:45:00 +0300 Subject: [PATCH 11/17] Using _ensure_container --- omegaconf/omegaconf.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/omegaconf/omegaconf.py b/omegaconf/omegaconf.py index 2bdd22f1d..5ca4192d2 100644 --- a/omegaconf/omegaconf.py +++ b/omegaconf/omegaconf.py @@ -796,11 +796,7 @@ def missing_keys(cfg: Any) -> Set[str]: :return: set of strings of the missing keys. :raises ValueError: On input not representing a config. """ - if not isinstance(cfg, Container): - try: - cfg = OmegaConf.create(cfg) - except ValidationError: - raise ValueError(f"Could not create a config out of {cfg}") + cfg = _ensure_container(cfg) missings: Set[str] = set() def gather(_cfg: Container) -> None: From e12def8f869c0dda57d06e76bb51b66216448bd0 Mon Sep 17 00:00:00 2001 From: Ohad31415 <47403773+Ohad31415@users.noreply.github.com> Date: Fri, 11 Jun 2021 09:49:55 +0300 Subject: [PATCH 12/17] Update news/720.feature Co-authored-by: Omry Yadan --- news/720.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/news/720.feature b/news/720.feature index f44ad8cc7..39dd70a4d 100644 --- a/news/720.feature +++ b/news/720.feature @@ -1 +1 @@ -New `OmegaConf.missing_keys` method that enables to get a config's set of missing keys represented in a dotlist style. +New `OmegaConf.missing_keys` method that returns the missing keys in a config object From 32e0789519133d6a351509d83c3a17607420c6f1 Mon Sep 17 00:00:00 2001 From: Ohad <> Date: Fri, 11 Jun 2021 10:16:31 +0300 Subject: [PATCH 13/17] simplified most of the tests to use plain dict/list --- tests/test_omegaconf.py | 73 ++++++++++++++++++++++++----------------- 1 file changed, 42 insertions(+), 31 deletions(-) diff --git a/tests/test_omegaconf.py b/tests/test_omegaconf.py index 335aef572..0bf80e9aa 100644 --- a/tests/test_omegaconf.py +++ b/tests/test_omegaconf.py @@ -524,23 +524,21 @@ def test_resolve_invalid_input() -> None: "cfg, expected", [ # dict: - (DictConfig({"a": 10, "b": {"c": "???", "d": "..."}}), {"b.c"}), + ({"a": 10, "b": {"c": "???", "d": "..."}}, {"b.c"}), ( - DictConfig( - { - "a": "???", - "b": { - "foo": "bar", - "bar": "???", - "more": {"missing": "???", "available": "yes"}, - }, - Color.GREEN: {"tint": "???", "default": Color.BLUE}, - } - ), + { + "a": "???", + "b": { + "foo": "bar", + "bar": "???", + "more": {"missing": "???", "available": "yes"}, + }, + Color.GREEN: {"tint": "???", "default": Color.BLUE}, + }, {"a", "b.bar", "b.more.missing", "GREEN.tint"}, ), ( - DictConfig({"a": "a", "b": {"foo": "bar", "bar": "foo"}}), + {"a": "a", "b": {"foo": "bar", "bar": "foo"}}, set(), ), ( @@ -548,28 +546,24 @@ def test_resolve_invalid_input() -> None: {"bar", "more.foo"}, ), # list: - (ListConfig(["???", "foo", "bar", "???", 77]), {"[0]", "[3]"}), - (ListConfig(["", "foo", "bar"]), set()), + (["???", "foo", "bar", "???", 77], {"[0]", "[3]"}), + (["", "foo", "bar"], set()), (["foo", "bar", "???"], {"[2]"}), (["foo", "???", ["???", "bar"]], {"[1]", "[2][0]"}), # mixing: ( - ListConfig( - [ - "???", - "foo", - DictConfig( - { - "a": True, - "b": "???", - "c": ["???", None], - "d": {"e": "???", "f": "fff", "g": [True, "???"]}, - } - ), - "???", - 77, - ] - ), + [ + "???", + "foo", + { + "a": True, + "b": "???", + "c": ["???", None], + "d": {"e": "???", "f": "fff", "g": [True, "???"]}, + }, + "???", + 77, + ], {"[0]", "[2].b", "[2].c[0]", "[2].d.e", "[2].d.g[1]", "[3]"}, ), ( @@ -586,6 +580,23 @@ def test_resolve_invalid_input() -> None: {"list[1].foo", "list[2]", "list[3][0]", "y"}, ), ({Color.RED: ["???", {"missing": "???"}]}, {"RED[0]", "RED[1].missing"}), + # with DictConfig and ListConfig: + ( + DictConfig( + { + "foo": "???", + "list": ["???", 1], + "bar": {"missing": "???", "more": "yes"}, + } + ), + {"foo", "list[0]", "bar.missing"}, + ), + ( + ListConfig( + ["???", "yes", "???", [0, 1, "???"], {"missing": "???", "more": ""}], + ), + {"[0]", "[2]", "[3][2]", "[4].missing"}, + ), ], ) def test_missing_keys(cfg: Any, expected: Any) -> None: From 81ae5e86714b46b4dd3fc5015efc8225d9dde424 Mon Sep 17 00:00:00 2001 From: Ohad31415 <47403773+Ohad31415@users.noreply.github.com> Date: Tue, 15 Jun 2021 09:20:52 +0300 Subject: [PATCH 14/17] Update docs/source/usage.rst Co-authored-by: Jasha10 <8935917+Jasha10@users.noreply.github.com> --- docs/source/usage.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/source/usage.rst b/docs/source/usage.rst index aa9f8b545..0e1451dc1 100644 --- a/docs/source/usage.rst +++ b/docs/source/usage.rst @@ -784,7 +784,8 @@ and ``OmegaConf.is_list(cfg)`` is equivalent to ``isinstance(cfg, ListConfig)``. OmegaConf.missing_keys ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -Gives a set of missing keys represented in a dotlist style. +``OmegaConf.missing_keys(cfg)`` returns a set of missing keys present in the input ``cfg``. +Each missing key is represented as a ``str``, using a dotlist style. This utility function can be used at the end of creating configuration, after merging sources and so on, to check for remaining mandatory fields and prompt a proper error message. From 496e98fb6d2d2ce0959b70eb7995e35af5071de4 Mon Sep 17 00:00:00 2001 From: Ohad31415 <47403773+Ohad31415@users.noreply.github.com> Date: Tue, 15 Jun 2021 09:21:16 +0300 Subject: [PATCH 15/17] Update docs/source/usage.rst Co-authored-by: Jasha10 <8935917+Jasha10@users.noreply.github.com> --- docs/source/usage.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/source/usage.rst b/docs/source/usage.rst index 0e1451dc1..3fda8d5b3 100644 --- a/docs/source/usage.rst +++ b/docs/source/usage.rst @@ -786,8 +786,8 @@ OmegaConf.missing_keys ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ``OmegaConf.missing_keys(cfg)`` returns a set of missing keys present in the input ``cfg``. Each missing key is represented as a ``str``, using a dotlist style. -This utility function can be used at the end of creating configuration, after merging sources and so on, -to check for remaining mandatory fields and prompt a proper error message. +This utility function can be used after creating a config object, after merging sources and so on, +to check for missing mandatory fields and aid in creating a proper error message. .. doctest:: From 83660dbebfcddf288aa9694dfe8bf18708cce891 Mon Sep 17 00:00:00 2001 From: Ohad <> Date: Fri, 18 Jun 2021 13:32:15 +0300 Subject: [PATCH 16/17] docs --- docs/source/usage.rst | 2 +- omegaconf/omegaconf.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/source/usage.rst b/docs/source/usage.rst index aa9f8b545..c1e84dada 100644 --- a/docs/source/usage.rst +++ b/docs/source/usage.rst @@ -797,7 +797,7 @@ to check for remaining mandatory fields and prompt a proper error message. ... }) >>> assert missings == {'list[2]', 'foo.bar', 'missing'} -A plain dict, list or any valid input for `OmegaConf.create` is also acceptable - the function would raise a `ValueError` exception on wrong input. +The function raises a `ValueError` on input not representing a config. Debugger integration diff --git a/omegaconf/omegaconf.py b/omegaconf/omegaconf.py index 5ca4192d2..86acb0b06 100644 --- a/omegaconf/omegaconf.py +++ b/omegaconf/omegaconf.py @@ -790,7 +790,7 @@ def resolve(cfg: Container) -> None: @staticmethod def missing_keys(cfg: Any) -> Set[str]: """ - Returns a set of missing keys flatten in a dotlist style. + Returns a set of missing keys in a dotlist style. :param cfg: An `OmegaConf.Container`, or a convertible object via `OmegaConf.create` (dict, list, ...). :return: set of strings of the missing keys. From 709ff5e95a4b7ee6542759654bf38fd5e4c6d626 Mon Sep 17 00:00:00 2001 From: Jasha <8935917+Jasha10@users.noreply.github.com> Date: Wed, 27 Oct 2021 22:36:34 -0500 Subject: [PATCH 17/17] lint --- tests/test_omegaconf.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_omegaconf.py b/tests/test_omegaconf.py index 5a83fa01a..b1f9da09a 100644 --- a/tests/test_omegaconf.py +++ b/tests/test_omegaconf.py @@ -645,4 +645,4 @@ def test_clear_resolver( assert expected["pre_clear"] == OmegaConf.has_resolver(name) assert OmegaConf.clear_resolver(name) == expected["result"] - assert not OmegaConf.has_resolver(name) \ No newline at end of file + assert not OmegaConf.has_resolver(name)