Bug fix for case when param type is Optional[Union...] (#4510)

* Bug fix for case when param type is Optional[Union...]

* Update CHANGELOG

* Adding test case

Author: AkshitaB

CHANGELOG.md

@@ -11,7 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Removed unnecessary warning about deadlocks in `DataLoader`.
 - Use slower tqdm intervals when output is being piped or redirected.
-- Fixed testing models that only return a loss when they are in training mode
+- Fixed testing models that only return a loss when they are in training mode.
+- Fixed a bug in `FromParams` that causes silent failure in case of the parameter type being Optional[Union[...]].
 
 ### Added
 

allennlp/common/from_params.py

@@ -105,8 +105,9 @@ def remove_optional(annotation: type):
     """
     origin = getattr(annotation, "__origin__", None)
     args = getattr(annotation, "__args__", ())
-    if origin == Union and len(args) == 2 and args[1] == type(None):  # noqa
-        return args[0]
+
+    if origin == Union:
+        return Union[tuple([arg for arg in args if arg != type(None)])]  # noqa: E721
     else:
         return annotation
 

tests/common/from_params_test.py

@@ -845,3 +845,19 @@ def __init__(self):
 
         with pytest.raises(ConfigurationError, match="no registered concrete types"):
             B.from_params(Params({}))
+
+    def test_from_params_raises_error_on_wrong_parameter_name_in_optional_union(self):
+        class NestedClass(FromParams):
+            def __init__(self, varname: Optional[str] = None):
+                self.varname = varname
+
+        class WrapperClass(FromParams):
+            def __init__(self, nested_class: Optional[Union[str, NestedClass]] = None):
+                if isinstance(nested_class, str):
+                    nested_class = NestedClass(varname=nested_class)
+                self.nested_class = nested_class
+
+        with pytest.raises(ConfigurationError):
+            WrapperClass.from_params(
+                params=Params({"nested_class": {"wrong_varname": "varstring"}})
+            )