refactor DictConfig._validate_non_optional, standardize err messages

omry · Oct 6, 2021 · d4fca02 · d4fca02
1 parent 7fb6fe5
commit d4fca02
Show file tree

Hide file tree

Showing 5 changed files with 19 additions and 25 deletions.
diff --git a/omegaconf/dictconfig.py b/omegaconf/dictconfig.py
@@ -236,33 +236,27 @@ def _validate_merge(self, value: Any) -> None:
             )
             raise ValidationError(msg)
 
-    def _validate_non_optional(self, key: Any, value: Any) -> None:
+    def _validate_non_optional(self, key: Optional[DictKeyType], value: Any) -> None:
         if _is_none(value, resolve=True, throw_on_resolution_failure=False):
+
             if key is not None:
                 child = self._get_node(key)
                 if child is not None:
                     assert isinstance(child, Node)
-                    if not child._is_optional():
-                        self._format_and_raise(
-                            key=key,
-                            value=value,
-                            cause=ValidationError("child '$FULL_KEY' is not Optional"),
-                        )
+                    field_is_optional = child._is_optional()
                 else:
-                    is_optional, _ = _resolve_optional(self._metadata.element_type)
-                    if not is_optional:
-                        self._format_and_raise(
-                            key=key,
-                            value=value,
-                            cause=ValidationError("field '$FULL_KEY' is not Optional"),
-                        )
-            else:
-                if not self._is_optional():
-                    self._format_and_raise(
-                        key=None,
-                        value=value,
-                        cause=ValidationError("field '$FULL_KEY' is not Optional"),
+                    field_is_optional, _ = _resolve_optional(
+                        self._metadata.element_type
                     )
+            else:
+                field_is_optional = self._is_optional()
+
+            if not field_is_optional:
+                self._format_and_raise(
+                    key=key,
+                    value=value,
+                    cause=ValidationError("field '$FULL_KEY' is not Optional"),
+                )
 
     def _raise_invalid_value(
         self, value: Any, value_type: Any, target_type: Any

diff --git a/tests/structured_conf/test_structured_basic.py b/tests/structured_conf/test_structured_basic.py
@@ -106,7 +106,7 @@ def test_merge_error_override_bad_type(self, module: Any) -> None:
 
         def test_error_message(self, module: Any) -> None:
             cfg = OmegaConf.structured(module.StructuredOptional)
-            msg = re.escape("child 'not_optional' is not Optional")
+            msg = re.escape("field 'not_optional' is not Optional")
             with raises(ValidationError, match=msg):
                 cfg.not_optional = None
 

diff --git a/tests/test_errors.py b/tests/test_errors.py
@@ -131,7 +131,7 @@ def finalize(self, cfg: Any) -> None:
             create=lambda: OmegaConf.structured(StructuredWithMissing),
             op=lambda cfg: OmegaConf.update(cfg, "num", None),
             exception_type=ValidationError,
-            msg="child 'num' is not Optional",
+            msg="field 'num' is not Optional",
             parent_node=lambda cfg: cfg,
             child_node=lambda cfg: cfg._get_node("num"),
             object_type=StructuredWithMissing,
@@ -370,7 +370,7 @@ def finalize(self, cfg: Any) -> None:
             ),
             op=lambda cfg: setattr(cfg, "foo", None),
             exception_type=ValidationError,
-            msg="child 'foo' is not Optional",
+            msg="field 'foo' is not Optional",
             key="foo",
             full_key="foo",
             child_node=lambda cfg: cfg.foo,

diff --git a/tests/test_matrix.py b/tests/test_matrix.py
@@ -111,7 +111,7 @@ def test_none_assignment_and_merging_in_dict(
             data = {"node": node}
             cfg = OmegaConf.create(obj=data)
             verify(cfg, "node", none=False, opt=False, missing=False, inter=False)
-            msg = "child 'node' is not Optional"
+            msg = "field 'node' is not Optional"
             with raises(ValidationError, match=re.escape(msg)):
                 cfg.node = None
 

diff --git a/tests/test_merge.py b/tests/test_merge.py
@@ -630,7 +630,7 @@ def test_merge(
                 match=re.escape(
                     dedent(
                         """\
-                        child 'foo' is not Optional
+                        field 'foo' is not Optional
                             full_key: foo
                             object_type=dict"""
                     )