IntegerNode/FloatNode ValidationError message: include value type (#744)

news/743.api_change

@@ -0,0 +1 @@
+Improved error message when assigning an invalid value to int or float config nodes

omegaconf/_utils.py

@@ -774,7 +774,7 @@ def format_and_raise(
         KEY=key,
         FULL_KEY=full_key,
         VALUE=value,
-        VALUE_TYPE=f"{type(value).__name__}",
+        VALUE_TYPE=type_str(type(value), include_module_name=True),
         KEY_TYPE=f"{type(key).__name__}",
     )
 
@@ -821,7 +821,7 @@ def format_and_raise(
     _raise(ex, cause)
 
 
-def type_str(t: Any) -> str:
+def type_str(t: Any, include_module_name: bool = False) -> str:
     is_optional, t = _resolve_optional(t)
     if t is None:
         return type(t).__name__
@@ -848,16 +848,31 @@ def type_str(t: Any) -> str:
         else:
             if t._name is None:
                 if t.__origin__ is not None:
-                    name = type_str(t.__origin__)
+                    name = type_str(
+                        t.__origin__, include_module_name=include_module_name
+                    )
             else:
                 name = str(t._name)
 
     args = getattr(t, "__args__", None)
     if args is not None:
-        args = ", ".join([type_str(t) for t in t.__args__])
+        args = ", ".join(
+            [type_str(t, include_module_name=include_module_name) for t in t.__args__]
+        )
         ret = f"{name}[{args}]"
     else:
         ret = name
+    if include_module_name:
+        if (
+            hasattr(t, "__module__")
+            and t.__module__ != "builtins"
+            and t.__module__ != "typing"
+            and not t.__module__.startswith("omegaconf.")
+        ):
+            module_prefix = t.__module__ + "."
+        else:
+            module_prefix = ""
+        ret = module_prefix + ret
     if is_optional:
         return f"Optional[{ret}]"
     else:

omegaconf/nodes.py

@@ -203,7 +203,9 @@ def _validate_and_convert_impl(self, value: Any) -> int:
             else:
                 raise ValueError()
         except ValueError:
-            raise ValidationError("Value '$VALUE' could not be converted to Integer")
+            raise ValidationError(
+                "Value '$VALUE' of type '$VALUE_TYPE' could not be converted to Integer"
+            )
         return val
 
     def __deepcopy__(self, memo: Dict[int, Any]) -> "IntegerNode":
@@ -240,7 +242,9 @@ def _validate_and_convert_impl(self, value: Any) -> float:
             else:
                 raise ValueError()
         except ValueError:
-            raise ValidationError("Value '$VALUE' could not be converted to Float")
+            raise ValidationError(
+                "Value '$VALUE' of type '$VALUE_TYPE' could not be converted to Float"
+            )
 
     def __eq__(self, other: Any) -> bool:
         if isinstance(other, ValueNode):

tests/interpolation/test_interpolation.py

@@ -332,7 +332,7 @@ def drop_last(s: str) -> str:
                 match=re.escape(
                     dedent(
                         """\
-                        Value 'seven' could not be converted to Integer
+                        Value 'seven' of type 'str' could not be converted to Integer
                             full_key: age
                         """
                     )
@@ -345,7 +345,9 @@ def drop_last(s: str) -> str:
             "age",
             raises(
                 InterpolationValidationError,
-                match=re.escape("'Bond' could not be converted to Integer"),
+                match=re.escape(
+                    "'Bond' of type 'str' could not be converted to Integer"
+                ),
             ),
             id="type_mismatch_node_interpolation",
         ),

tests/structured_conf/test_structured_basic.py

@@ -51,7 +51,9 @@ def test_error_on_non_structured_nested_config_class(self, module: Any) -> None:
         def test_error_on_creation_with_bad_value_type(self, module: Any) -> None:
             with raises(
                 ValidationError,
-                match=re.escape("Value 'seven' could not be converted to Integer"),
+                match=re.escape(
+                    "Value 'seven' of type 'str' could not be converted to Integer"
+                ),
             ):
                 OmegaConf.structured(module.User(age="seven"))
 

tests/test_basic_ops_list.py

@@ -331,7 +331,9 @@ def test_list_append() -> None:
             "foo",
             raises(
                 ValidationError,
-                match=re.escape("Value 'foo' could not be converted to Integer"),
+                match=re.escape(
+                    "Value 'foo' of type 'str' could not be converted to Integer"
+                ),
             ),
             id="append_str_to_list[int]",
         ),

tests/test_errors.py

@@ -9,6 +9,7 @@
 import tests
 from omegaconf import (
     DictConfig,
+    FloatNode,
     IntegerNode,
     ListConfig,
     OmegaConf,
@@ -113,7 +114,7 @@ def finalize(self, cfg: Any) -> None:
             create=lambda: OmegaConf.structured(StructuredWithMissing),
             op=lambda cfg: OmegaConf.update(cfg, "num", "hello"),
             exception_type=ValidationError,
-            msg="Value 'hello' could not be converted to Integer",
+            msg="Value 'hello' of type 'str' could not be converted to Integer",
             parent_node=lambda cfg: cfg,
             child_node=lambda cfg: cfg._get_node("num"),
             object_type=StructuredWithMissing,
@@ -362,7 +363,7 @@ def finalize(self, cfg: Any) -> None:
             create=lambda: OmegaConf.structured(ConcretePlugin),
             op=lambda cfg: cfg.params.__setattr__("foo", "bar"),
             exception_type=ValidationError,
-            msg="Value 'bar' could not be converted to Integer",
+            msg="Value 'bar' of type 'str' could not be converted to Integer",
             key="foo",
             full_key="params.foo",
             object_type=ConcretePlugin.FoobarParams,
@@ -484,7 +485,7 @@ def finalize(self, cfg: Any) -> None:
             create=lambda: OmegaConf.structured(ConcretePlugin),
             op=lambda cfg: OmegaConf.merge(cfg, {"params": {"foo": "bar"}}),
             exception_type=ValidationError,
-            msg="Value 'bar' could not be converted to Integer",
+            msg="Value 'bar' of type 'str' could not be converted to Integer",
             key="foo",
             full_key="params.foo",
             object_type=ConcretePlugin.FoobarParams,
@@ -646,13 +647,37 @@ def finalize(self, cfg: Any) -> None:
                 ConcretePlugin(params=ConcretePlugin.FoobarParams(foo="x"))  # type: ignore
             ),
             exception_type=ValidationError,
-            msg="Value 'x' could not be converted to Integer",
+            msg="Value 'x' of type 'str' could not be converted to Integer",
             key="foo",
             full_key="foo",
             parent_node=lambda _: {},
             object_type=ConcretePlugin.FoobarParams,
         ),
-        id="structured:create_with_invalid_value",
+        id="structured:create_with_invalid_value,int",
+    ),
+    param(
+        Expected(
+            create=lambda: DictConfig({"bar": FloatNode(123.456)}),
+            op=lambda cfg: cfg.__setattr__("bar", "x"),
+            exception_type=ValidationError,
+            msg="Value 'x' of type 'str' could not be converted to Float",
+            key="bar",
+            full_key="bar",
+            child_node=lambda cfg: cfg._get_node("bar"),
+        ),
+        id="typed_DictConfig:assign_with_invalid_value,float",
+    ),
+    param(
+        Expected(
+            create=lambda: DictConfig({"bar": FloatNode(123.456)}),
+            op=lambda cfg: cfg.__setattr__("bar", Color.BLUE),
+            exception_type=ValidationError,
+            msg="Value 'Color.BLUE' of type 'tests.Color' could not be converted to Float",
+            key="bar",
+            full_key="bar",
+            child_node=lambda cfg: cfg._get_node("bar"),
+        ),
+        id="typed_DictConfig:assign_with_invalid_value,full_module_in_error",
     ),
     param(
         Expected(
@@ -694,7 +719,7 @@ def finalize(self, cfg: Any) -> None:
             ),
             op=lambda cfg: cfg.__setitem__("baz", "fail"),
             exception_type=ValidationError,
-            msg="Value 'fail' could not be converted to Integer",
+            msg="Value 'fail' of type 'str' could not be converted to Integer",
             key="baz",
         ),
         id="DictConfig[str,int]:assigned_str_value",
@@ -1102,7 +1127,7 @@ def finalize(self, cfg: Any) -> None:
             create=lambda: ListConfig(element_type=int, content=[1, 2, 3]),
             op=lambda cfg: cfg.__setitem__(0, "foo"),
             exception_type=ValidationError,
-            msg="Value 'foo' could not be converted to Integer",
+            msg="Value 'foo' of type 'str' could not be converted to Integer",
             key=0,
             full_key="[0]",
             child_node=lambda cfg: cfg[0],
@@ -1117,7 +1142,7 @@ def finalize(self, cfg: Any) -> None:
             ),
             op=lambda cfg: cfg.__setitem__(0, "foo"),
             exception_type=ValidationError,
-            msg="Value 'foo' could not be converted to Integer",
+            msg="Value 'foo' of type 'str' could not be converted to Integer",
             key=0,
             full_key="[0]",
             child_node=lambda cfg: cfg[0],
@@ -1464,7 +1489,7 @@ def test_dict_subclass_error() -> None:
     src["bar"] = "qux"  # type: ignore
     with raises(
         ValidationError,
-        match=re.escape("Value 'qux' could not be converted to Integer"),
+        match=re.escape("Value 'qux' of type 'str' could not be converted to Integer"),
     ) as einfo:
         with warns_dict_subclass_deprecated(Str2Int):
             OmegaConf.structured(src)

tests/test_utils.py

@@ -366,32 +366,56 @@ def test_is_primitive_type(type_: Any, is_primitive: bool) -> None:
 
 @mark.parametrize("optional", [False, True])
 @mark.parametrize(
-    "type_, expected",
+    "type_, include_module_name, expected",
     [
-        (int, "int"),
-        (bool, "bool"),
-        (float, "float"),
-        (str, "str"),
-        (Color, "Color"),
-        (DictConfig, "DictConfig"),
-        (ListConfig, "ListConfig"),
-        (Dict[str, str], "Dict[str, str]"),
-        (Dict[Color, int], "Dict[Color, int]"),
-        (Dict[str, Plugin], "Dict[str, Plugin]"),
-        (Dict[str, List[Plugin]], "Dict[str, List[Plugin]]"),
-        (List[str], "List[str]"),
-        (List[Color], "List[Color]"),
-        (List[Dict[str, Color]], "List[Dict[str, Color]]"),
-        (Tuple[str], "Tuple[str]"),
-        (Tuple[str, int], "Tuple[str, int]"),
-        (Tuple[float, ...], "Tuple[float, ...]"),
+        (int, False, "int"),
+        (int, True, "int"),
+        (bool, False, "bool"),
+        (bool, True, "bool"),
+        (float, False, "float"),
+        (float, True, "float"),
+        (str, False, "str"),
+        (str, True, "str"),
+        (Color, False, "Color"),
+        (Color, True, "tests.Color"),
+        (DictConfig, False, "DictConfig"),
+        (DictConfig, True, "DictConfig"),
+        (ListConfig, False, "ListConfig"),
+        (ListConfig, True, "ListConfig"),
+        (Dict[str, str], False, "Dict[str, str]"),
+        (Dict[str, str], True, "Dict[str, str]"),
+        (Dict[Color, int], False, "Dict[Color, int]"),
+        (Dict[Color, int], True, "Dict[tests.Color, int]"),
+        (Dict[str, Plugin], False, "Dict[str, Plugin]"),
+        (Dict[str, Plugin], True, "Dict[str, tests.Plugin]"),
+        (Dict[str, List[Plugin]], False, "Dict[str, List[Plugin]]"),
+        (Dict[str, List[Plugin]], True, "Dict[str, List[tests.Plugin]]"),
+        (List[str], False, "List[str]"),
+        (List[str], True, "List[str]"),
+        (List[Color], False, "List[Color]"),
+        (List[Color], True, "List[tests.Color]"),
+        (List[Dict[str, Color]], False, "List[Dict[str, Color]]"),
+        (List[Dict[str, Color]], True, "List[Dict[str, tests.Color]]"),
+        (Tuple[str], False, "Tuple[str]"),
+        (Tuple[str], True, "Tuple[str]"),
+        (Tuple[str, int], False, "Tuple[str, int]"),
+        (Tuple[str, int], True, "Tuple[str, int]"),
+        (Tuple[float, ...], False, "Tuple[float, ...]"),
+        (Tuple[float, ...], True, "Tuple[float, ...]"),
     ],
 )
-def test_type_str(type_: Any, expected: str, optional: bool) -> None:
+def test_type_str(
+    type_: Any, include_module_name: bool, expected: str, optional: bool
+) -> None:
     if optional:
-        assert _utils.type_str(Optional[type_]) == f"Optional[{expected}]"
+        assert (
+            _utils.type_str(Optional[type_], include_module_name=include_module_name)
+            == f"Optional[{expected}]"
+        )
     else:
-        assert _utils.type_str(type_) == expected
+        assert (
+            _utils.type_str(type_, include_module_name=include_module_name) == expected
+        )
 
 
 def test_type_str_ellipsis() -> None: