omry / omegaconf

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

Support deep merging of configs #1080

Closed MatteoVoges closed 1 year ago

MatteoVoges commented 1 year ago

Is your feature request related to a problem? Please describe. I don't know if it's a bug or the expected behavior in the project, but I have the following example:

I want to merge the following configurations:

dict: 
  list:
    - entry_a1: val_a1
      entry_a2: val_a2
dict: 
  list:
    - entry_b1: val_b1
      entry_b2: val_b2

If I merge them, I get: ( see full script and steps to reproduce below

dict: 
  list:
    - entry_b1: val_b1
      entry_b2: val_b2

Describe the solution you'd like However I would expect, that the configs get deep merged, so that I get

dict: 
  list:
    - entry_a1: val_a1
      entry_a2: val_a2
    - entry_b1: val_b1
      entry_b2: val_b2

as the merged result. The order of the elements doesn't matter.

Is that already possible and I'm missing something or is this functionality not supported?

Describe alternatives you've considered The main thing with this feature is, that we have to check, if two types are mergable, so that not something like

dict: 
  string: val

and

dict: 
  list:
      - entry1

get merged, because the types string and list would be incompatible.

Additional context

Here is the full python script for you to reproduce the examples: ```python from omegaconf import OmegaConf source_a = """ dict: list: - entry_a1: val_a1 entry_a2: val_a2 """ source_b = """ dict: list: - entry_b1: val_b1 entry_b2: val_b2 """ cfg_a = OmegaConf.create(source_a) cfg_b = OmegaConf.create(source_b) cfg_merged = OmegaConf.merge(cfg_a, cfg_b) print(OmegaConf.to_yaml(cfg_merged)) ```
omry commented 1 year ago

OmegaConf supports deep merging. The problem here is that this does not work for lists. The are currently no plans to change this behavior (in fact, changing it would break backward compatibility, in addition this is not well defined (e.g what is the lists do not have the same number of items?)

MatteoVoges commented 1 year ago

Thank you for your answer. I would expect, that merging two lists [a] and [b] results in a list [a,b]. This would be really helpful for our usecase. What do you think about an argument / parameter in the merge function, maybe something like extend_lists, that activates the requested feature?

omry commented 1 year ago

Sounds reasonable. I am open to a pull request adding this functionality (along with the appropriate tests and documentation updates. Note that OmegaConf is currently in maintenance mode and I cannot commit that I will be able to release a version with it in the near future.

ademariag commented 1 year ago

Thank you @omry and OmegaConf team,

for context this is the first step to enable for our project kapitan to adopt OmegaConf as an alternative to reclass

As OmegaConf is on maintenance mode, would you have anything against us forking the project under the kapicorp org? We'd be happy to continue sending upstreams changes when appropriate.

omry commented 1 year ago

This is an open-source project so you are free to do this. However, I think we will at least try to accept your change soon so you can check out from a specific commit on the official repo (you can also use a git submodule, or maybe just depend directly on github commit from your build.

MatteoVoges commented 1 year ago

The most important thing for us would be to have a PyPi package that has the feature. Could you make a pre-release for it, when the feature is merged?

ademariag commented 1 year ago

Thank you @omry, no rush. I was just offering to reduce the work on your side. Happy to wait and meanwhile we can follow your suggestions.

ademariag commented 1 year ago

@MatteoVoges perhaps we can use pip to install straight from the branch as @omry suggests for now:

pip install -U git+https://github.com/neXenio/omegaconf.git@1080-add-list-deep-merging