Skip to content

Commit

Permalink
Merging a missing list onto an existing one makes the target missing
Browse files Browse the repository at this point in the history
  • Loading branch information
omry committed Jul 16, 2020
1 parent b76be05 commit 8585861
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 4 deletions.
1 change: 1 addition & 0 deletions news/306.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Merging a missing list onto an existing one makes the target missing
7 changes: 5 additions & 2 deletions omegaconf/basecontainer.py
Original file line number Diff line number Diff line change
Expand Up @@ -336,8 +336,11 @@ def _merge_with(
else:
self.__dict__["_content"].clear()

for item in other:
self.append(item)
if other._is_missing():
self._set_value("???")
else:
for item in other:
self.append(item)

# explicit flags on the source config are replacing the flag values in the destination
for flag, value in other._metadata.flags.items():
Expand Down
21 changes: 19 additions & 2 deletions tests/test_merge.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from typing import Any, Tuple
from typing import Any, MutableMapping, MutableSequence, Tuple

import pytest

Expand Down Expand Up @@ -50,6 +50,11 @@
{"a": {"b": 10}},
id="dict_merge_into_missing",
),
pytest.param(
({"a": {"b": 10}}, {"a": "???"}),
{"a": "???"},
id="dict_merge_missing_onto",
),
# lists
(([1, 2, 3], [4, 5, 6]), [4, 5, 6]),
(([[1, 2, 3]], [[4, 5, 6]]), [[4, 5, 6]]),
Expand All @@ -62,6 +67,16 @@
{"a": [1, 2, 3]},
id="list_merge_into_missing",
),
pytest.param(
({"a": [1, 2, 3]}, {"a": "???"}),
{"a": "???"},
id="list_merge_missing_onto",
),
pytest.param(
([1, 2, 3], ListConfig(content=MISSING)),
ListConfig(content=MISSING),
id="list_merge_missing_onto2",
),
# Interpolations
# value interpolation
pytest.param(
Expand Down Expand Up @@ -205,7 +220,9 @@
def test_merge(inputs: Any, expected: Any) -> None:
configs = [OmegaConf.create(c) for c in inputs]

if isinstance(expected, (dict, list)) or is_structured_config(expected):
if isinstance(expected, (MutableMapping, MutableSequence)) or is_structured_config(
expected
):
merged = OmegaConf.merge(*configs)
assert merged == expected
# test input configs are not changed.
Expand Down

0 comments on commit 8585861

Please sign in to comment.