improve error message when creating a DictConfig from a bad dataclass (…

…#701) * improve error message when creating a DictConfig from a bad Structured Config * in DictConfig.__init__ and ListConfig.__init__: pass key to format_and_raise * news fragment Co-authored-by: Jasha <[email protected]>
omry · Apr 30, 2021 · 3b04bf9 · 3b04bf9
1 parent 0ba497e
commit 3b04bf9
Show file tree

Hide file tree

Showing 6 changed files with 61 additions and 13 deletions.
diff --git a/news/697.bugfix b/news/697.bugfix
@@ -0,0 +1 @@
+Improve error message when creating a config from a Structured Config that fails type validation
diff --git a/omegaconf/_utils.py b/omegaconf/_utils.py
@@ -24,6 +24,7 @@
     ConfigIndexError,
     ConfigTypeError,
     ConfigValueError,
+    GrammarParseError,
     OmegaConfBaseException,
     ValidationError,
 )
@@ -265,12 +266,15 @@ def get_attr_data(obj: Any, allow_objects: Optional[bool] = None) -> Dict[str, A
     from omegaconf.omegaconf import OmegaConf, _maybe_wrap
 
     flags = {"allow_objects": allow_objects} if allow_objects is not None else {}
-    dummy_parent = OmegaConf.create(flags=flags)
+
     from omegaconf import MISSING
 
     d = {}
     is_type = isinstance(obj, type)
     obj_type = obj if is_type else type(obj)
+    dummy_parent = OmegaConf.create({}, flags=flags)
+    dummy_parent._metadata.object_type = obj_type
+
     for name, attrib in attr.fields_dict(obj_type).items():
         is_optional, type_ = _resolve_optional(attrib.type)
         type_ = _resolve_forward(type_, obj.__module__)
@@ -294,8 +298,10 @@ def get_attr_data(obj: Any, allow_objects: Optional[bool] = None) -> Dict[str, A
                 value=value,
                 parent=dummy_parent,
             )
-        except ValidationError as ex:
-            format_and_raise(node=None, key=name, value=value, cause=ex, msg=str(ex))
+        except (ValidationError, GrammarParseError) as ex:
+            format_and_raise(
+                node=dummy_parent, key=name, value=value, cause=ex, msg=str(ex)
+            )
         d[name]._set_parent(None)
     dict_subclass_data = extract_dict_subclass_data(obj=obj, parent=dummy_parent)
     if dict_subclass_data is not None:
@@ -313,9 +319,10 @@ def get_dataclass_data(
     from omegaconf.omegaconf import MISSING, OmegaConf, _maybe_wrap
 
     flags = {"allow_objects": allow_objects} if allow_objects is not None else {}
-    dummy_parent = OmegaConf.create({}, flags=flags)
     d = {}
     obj_type = get_type_of(obj)
+    dummy_parent = OmegaConf.create({}, flags=flags)
+    dummy_parent._metadata.object_type = obj_type
     resolved_hints = get_type_hints(obj_type)
     for field in dataclasses.fields(obj):
         name = field.name
@@ -345,8 +352,10 @@ def get_dataclass_data(
                 value=value,
                 parent=dummy_parent,
             )
-        except ValidationError as ex:
-            format_and_raise(node=None, key=name, value=value, cause=ex, msg=str(ex))
+        except (ValidationError, GrammarParseError) as ex:
+            format_and_raise(
+                node=dummy_parent, key=name, value=value, cause=ex, msg=str(ex)
+            )
         d[name]._set_parent(None)
     dict_subclass_data = extract_dict_subclass_data(obj=obj, parent=dummy_parent)
     if dict_subclass_data is not None:
@@ -721,7 +730,7 @@ def format_and_raise(
 
     child_node: Optional[Node] = None
     if node is None:
-        full_key = ""
+        full_key = key if key is not None else ""
         object_type = None
         ref_type = None
         ref_type_str = None

diff --git a/omegaconf/dictconfig.py b/omegaconf/dictconfig.py
@@ -110,7 +110,7 @@ def __init__(
                     self.__dict__["_metadata"] = metadata
                 self._set_value(content, flags=flags)
         except Exception as ex:
-            format_and_raise(node=None, key=None, value=None, cause=ex, msg=str(ex))
+            format_and_raise(node=None, key=key, value=None, cause=ex, msg=str(ex))
 
     def __deepcopy__(self, memo: Dict[int, Any]) -> "DictConfig":
         res = DictConfig(None)

diff --git a/omegaconf/listconfig.py b/omegaconf/listconfig.py
@@ -77,7 +77,7 @@ def __init__(
             self.__dict__["_content"] = None
             self._set_value(value=content, flags=flags)
         except Exception as ex:
-            format_and_raise(node=None, key=None, value=None, cause=ex, msg=str(ex))
+            format_and_raise(node=None, key=key, value=None, cause=ex, msg=str(ex))
 
     def _validate_get(self, key: Any, value: Any = None) -> None:
         if not isinstance(key, (int, slice)):

diff --git a/tests/__init__.py b/tests/__init__.py
@@ -117,6 +117,16 @@ class UnionError:
     x: Union[int, str] = 10
 
 
+@dataclass
+class StructuredWithBadDict:
+    foo: Dict[str, str] = 123  # type: ignore
+
+
+@dataclass
+class StructuredWithBadList:
+    foo: List[str] = 123  # type: ignore
+
+
 @dataclass
 class MissingList:
     list: List[str] = MISSING

diff --git a/tests/test_errors.py b/tests/test_errors.py
@@ -39,6 +39,8 @@
     Package,
     Plugin,
     Str2Int,
+    StructuredWithBadDict,
+    StructuredWithBadList,
     StructuredWithMissing,
     SubscriptedDict,
     UnionError,
@@ -605,7 +607,9 @@ def finalize(self, cfg: Any) -> None:
             exception_type=ValidationError,
             msg="Non optional field cannot be assigned None",
             key="foo",
-            full_key="",
+            full_key="foo",
+            parent_node=lambda _: {},
+            object_type=NotOptionalInt,
         ),
         id="dict:create_none_optional_with_none",
     ),
@@ -616,7 +620,9 @@ def finalize(self, cfg: Any) -> None:
             exception_type=ValidationError,
             msg="Non optional field cannot be assigned None",
             key="foo",
-            full_key="",
+            full_key="foo",
+            parent_node=lambda _: {},
+            object_type=NotOptionalInt,
         ),
         id="dict:create:not_optional_int_field_with_none",
     ),
@@ -654,7 +660,9 @@ def finalize(self, cfg: Any) -> None:
             exception_type=ValidationError,
             msg="Value 'x' could not be converted to Integer",
             key="foo",
-            full_key="",
+            full_key="foo",
+            parent_node=lambda _: {},
+            object_type=ConcretePlugin.FoobarParams,
         ),
         id="structured:create_with_invalid_value",
     ),
@@ -795,11 +803,31 @@ def finalize(self, cfg: Any) -> None:
             msg="Value 'qux' could not be converted to Integer",
             object_type=None,
             key="bar",
-            full_key="",
+            full_key="bar",
             parent_node=lambda cfg: None,
         ),
         id="structured,Dict_subclass:bad_value_type",
     ),
+    param(
+        Expected(
+            create=lambda: None,
+            op=lambda _: OmegaConf.structured(StructuredWithBadDict),
+            exception_type=ValidationError,
+            msg="Cannot assign int to Dict[str, str]",
+            key="foo",
+        ),
+        id="structured,bad_default_value_for_dict",
+    ),
+    param(
+        Expected(
+            create=lambda: None,
+            op=lambda _: OmegaConf.structured(StructuredWithBadList),
+            exception_type=ValidationError,
+            msg="Invalid value assigned: int is not a ListConfig, list or tuple.",
+            key="foo",
+        ),
+        id="structured,bad_default_value_for_list",
+    ),
     ##############
     # ListConfig #
     ##############
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		Improve error message when creating a config from a Structured Config that fails type validation