Merge element type (#499)

* Merge: Don't overwrite element_type=strucutured

Closes #496.

* add another merge test case

* simplified and added additional test cases

* removed duplicate test and moved new tests to bottom of block

Co-authored-by: Jasha <[email protected]>

news/496.bugfix

@@ -0,0 +1 @@
+Fix merge when element type is a Structured Config

omegaconf/basecontainer.py

@@ -349,9 +349,11 @@ def expand(node: Container) -> None:
                     dest_node = dest._get_node(key)
 
             if (
-                is_structured_config(dest._metadata.element_type)
+                dest_node is None
+                and is_structured_config(dest._metadata.element_type)
                 and not missing_src_value
             ):
+                # merging into a new node. Use element_type as a base
                 dest[key] = DictConfig(content=dest._metadata.element_type, parent=dest)
                 dest_node = dest._get_node(key)
 

omegaconf/omegaconf.py

@@ -532,7 +532,7 @@ def to_container(
         )
 
     @staticmethod
-    def is_missing(cfg: Any, key: Union[int, str]) -> bool:
+    def is_missing(cfg: Any, key: DictKeyType) -> bool:
         assert isinstance(cfg, Container)
         try:
             node = cfg._get_node(key)

tests/test_merge.py

@@ -38,8 +38,8 @@
 @pytest.mark.parametrize(
     ("merge_function", "input_unchanged"),
     [
-        (OmegaConf.merge, True),
-        (OmegaConf.unsafe_merge, False),
+        pytest.param(OmegaConf.merge, True, id="merge"),
+        pytest.param(OmegaConf.unsafe_merge, False, id="unsafe_merge"),
     ],
 )
 @pytest.mark.parametrize(
@@ -234,6 +234,39 @@
             id="merge_none_into_existing_node",
         ),
         ([{"user": User()}, {"user": {"foo": "bar"}}], pytest.raises(ConfigKeyError)),
+        # DictConfig with element_type of Structured Config
+        pytest.param(
+            (
+                DictConfig({}, element_type=User),
+                {"user007": {"age": 99}},
+            ),
+            {"user007": {"name": "???", "age": 99}},
+            id="dict:merge_into_sc_element_type:expanding_new_element",
+        ),
+        pytest.param(
+            (
+                DictConfig({"user007": "???"}, element_type=User),
+                {"user007": {"age": 99}},
+            ),
+            {"user007": {"name": "???", "age": 99}},
+            id="dict:merge_into_sc_element_type:into_missing_element",
+        ),
+        pytest.param(
+            (
+                DictConfig({"user007": User("bond", 7)}, element_type=User),
+                {"user007": {"age": 99}},
+            ),
+            {"user007": {"name": "bond", "age": 99}},
+            id="dict:merge_into_sc_element_type:merging_with_existing_element",
+        ),
+        pytest.param(
+            (
+                DictConfig({"user007": None}, element_type=User),
+                {"user007": {"age": 99}},
+            ),
+            {"user007": {"name": "???", "age": 99}},
+            id="dict:merge_into_sc_element_type:merging_into_none",
+        ),
         # missing DictConfig
         pytest.param(
             [{"dict": DictConfig(content="???")}, {"dict": {"foo": "bar"}}],