omry / omegaconf

Flexible Python configuration system. The last one you will ever need.
BSD 3-Clause "New" or "Revised" License
1.98k stars 113 forks source link

Merge Option of update function does not merge list #1154

Open mschwab12 opened 9 months ago

mschwab12 commented 9 months ago

Describe the bug I want to update a list config an use the merge option (#367) as I want to add another list item. When I do this the existing list item always gets replaced instead of merged which is because in the code of _list_merge of basecontainer.py the temp dest gets initialised with an empty list, gets the new value added and then the original list is replaced with the temp list. So this of course does not make a match. I am not sure if this behaviour is intended.

To Reproduce

from omegaconf import OmegaConf

cfg = OmegaConf.create({"a": {"b": [10]}})
print(OmegaConf.to_yaml(cast(OmegaConfig, cfg)))
OmegaConf.update(cfg, "a.b", [20])
print(OmegaConf.to_yaml(cast(OmegaConfig, cfg)))

# Outputs
# a:
#   b:
#   - 10
#
# a:
#   b:
#   - 20

Expected behavior Output should be:

# Outputs
# a:
#   b:
#   - 10
#
# a:
#   b:
#   - 10
#   - 20

Additional context

odelalleau commented 9 months ago

This is because merging lists in OmegaConf leads to the merged list overriding the previous one. So this is actually working as intended.

You can check this comment for pointers towards potential workarounds: https://github.com/facebookresearch/hydra/issues/1547#issuecomment-1235918733