Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ConfigKeyError for interpolation keys when merging configs #442

Closed
4 tasks done
mustafagok opened this issue Nov 22, 2020 · 6 comments · Fixed by #510
Closed
4 tasks done

ConfigKeyError for interpolation keys when merging configs #442

mustafagok opened this issue Nov 22, 2020 · 6 comments · Fixed by #510
Labels
bug Something isn't working
Milestone

Comments

@mustafagok
Copy link

Description of the bug
When merging configurations, if the secondary config contains an interpolation key that refers to the primary (base/default) config, omegaconf fails to merge and raises omegaconf.errors.ConfigKeyError: str interpolation key 'foo' not found error.

Assume that there is a default config for all clients. In this config, there is a list with large objects and they are defined separately for reusability. Some clients need to modify this list in the following ways:

  • add a new item
  • remove an existing item
  • modify an existing item

To Reproduce

from omegaconf import OmegaConf

# region BASE CONFIGURATION

base_conf_str = """
# large objects defined here for reusability
# assume that these object are large (20+ lines for each)
key_a: large_object_a
key_b: large_object_b
key_c: large_object_c

# default list
list:
- ${key_a}
- ${key_b}
- ${key_c}
"""
# create base configuration
base_conf = OmegaConf.create(base_conf_str)
print("base configuration:")
print(OmegaConf.to_yaml(base_conf))
print("base configuration interpolations resolved:")
print(OmegaConf.to_yaml(base_conf, resolve=True))

# endregion BASE CONFIGURATION

# region CLIENT_X

# client_x wants to override "large_object_a" in the list easily
client_x_override_str = """
key_a: new_large_object_a_for_x
"""
# if the large objects were not defined separately
# client_x would had to define whole list again:
# list:
# - new_large_object_a_for_x
# - large_object_b
# - large_object_c
# so, reusability works for client_x

client_x_override = OmegaConf.create(client_x_override_str)
print("client x override:")
print(OmegaConf.to_yaml(client_x_override))

# merge override with base configuration to get final configuration for x
client_x_conf = OmegaConf.merge(base_conf, client_x_override)
print("client x configuration:")
print(OmegaConf.to_yaml(client_x_conf))
print("client x configuration interpolations resolved:")
print(OmegaConf.to_yaml(client_x_conf, resolve=True))
print("client x list:")
print(OmegaConf.to_yaml(client_x_conf.list, resolve=True))

# endregion CLIENT_X

# region CLIENT_Y

# client_y wants only "large_object_a" and "large_object_c" in list
client_y_override_str = """
list:
- ${key_a}
- ${key_c}
"""
client_y_override = OmegaConf.create(client_y_override_str)
print("client y override:")
print(OmegaConf.to_yaml(client_y_override))
# it is ok to get error here when resolving client_y_override
# because we have interpolation keys which is unknown for now
# OmegaConf.to_yaml(client_y_override, resolve=True)

# I believe that we should not get error here
# since interpolations are evaluated lazily on access
client_y_conf = OmegaConf.merge(base_conf, client_y_override)
# expected result
"""
key_a: large_object_a
key_b: large_object_b
key_c: large_object_c

list:
- ${key_a}
- ${key_c}
"""

# endregion CLIENT_Y

# region CLIENT_Z

# client_z wants to add a custom item to the list
client_z_override_str = """
list:
- ${key_a}
- ${key_b}
- ${key_c}
- custom_large_object_for_z
"""
client_z_override = OmegaConf.create(client_z_override_str)
print("client x override:")
print(OmegaConf.to_yaml(client_z_override))
# it is ok to get error here when resolving client_z_override
# because we have interpolation keys which is unknown for now
# OmegaConf.to_yaml(client_z_override, resolve=True)

# I believe that we should not get error here
# since interpolations are evaluated lazily on access
client_z_conf = OmegaConf.merge(base_conf, client_z_override)
# expected result
"""
key_a: large_object_a
key_b: large_object_b
key_c: large_object_c

list:
- ${key_a}
- ${key_b}
- ${key_c}
- custom_large_object_for_z
"""

# endregion CLIENT_Z

Client Y and Z have errors.

Expected behavior
This operations can be done if omegaconf does not try to resolve interpolations when merging configurations. Actually the documentation says "interpolations are evaluated lazily on access". See the expected result parts in the script.

Additional context

  • OmegaConf version: 2.0.5
  • Python version: 3.6.9
  • Operating system: ubuntu 18.04
  • Please provide a minimal repro: not minimal, a bit long script, sorry for that

Note: I would like to open a PR to fix this issue, but before that I want to hear your opinions and suggestions about the problem.

@mustafagok mustafagok added the bug Something isn't working label Nov 22, 2020
@omry
Copy link
Owner

omry commented Nov 22, 2020

Hi.
Thanks for reporting.
Having spent some time looking at your issue it's very hard to understand what the problem you are reporting is.

Can you clean up your repro a bit?

  1. Use dictionaries instead of strings to represent input configs.
  2. Remove anything not needed to demonstrate the problem.
  3. Show the error you receive, or the unexpected output if there is no error.

Finally, can you also try against master?

@mustafagok
Copy link
Author

Hi, thanks for reply.

Here is a minimal script to get the error (tried against master & v2.0.5):

from omegaconf import OmegaConf

base_conf_source = {
    "key_a": "large_object_a",
    "key_b": "large_object_b",
    "key_c": "large_object_c",
    "list": [
        "${key_a}",
        "${key_b}",
        "${key_c}",
    ]
}
base_conf = OmegaConf.create(base_conf_source)

# client_y wants only "large_object_a" and "large_object_c" in list
client_y_override_source = {
    "list": [
        "${key_a}",
        "${key_c}",
    ]
}
client_y_override = OmegaConf.create(client_y_override_source)

# I believe that we should not get error here
# since interpolations are evaluated lazily on access
client_y_conf = OmegaConf.merge(base_conf, client_y_override)
# expected result
"""
key_a: large_object_a
key_b: large_object_b
key_c: large_object_c

list:
- ${key_a}
- ${key_c}
"""

error:

client_y_conf = OmegaConf.merge(base_conf, client_y_override)
Traceback (most recent call last):
  File "<input>", line 1, in <module>
  File "/home/x/py-env/lib/python3.6/site-packages/omegaconf/omegaconf.py", line 321, in merge
    target.merge_with(*others[1:])
  File "/home/x/py-env/lib/python3.6/site-packages/omegaconf/basecontainer.py", line 331, in merge_with
    self._format_and_raise(key=None, value=None, cause=e)
  File "/home/x/py-env/lib/python3.6/site-packages/omegaconf/base.py", line 101, in _format_and_raise
    type_override=type_override,
  File "/home/x/py-env/lib/python3.6/site-packages/omegaconf/_utils.py", line 629, in format_and_raise
    _raise(ex, cause)
  File "/home/x/py-env/lib/python3.6/site-packages/omegaconf/_utils.py", line 610, in _raise
    raise ex  # set end OC_CAUSE=1 for full backtrace
  File "/home/x/py-env/lib/python3.6/site-packages/omegaconf/basecontainer.py", line 329, in merge_with
    self._merge_with(*others)
  File "/home/x/py-env/lib/python3.6/site-packages/omegaconf/basecontainer.py", line 347, in _merge_with
    BaseContainer._map_merge(self, other)
  File "/home/x/py-env/lib/python3.6/site-packages/omegaconf/basecontainer.py", line 294, in _map_merge
    dest_node._merge_with(src_value)
  File "/home/x/py-env/lib/python3.6/site-packages/omegaconf/basecontainer.py", line 367, in _merge_with
    for item in other:
  File "/home/x/py-env/lib/python3.6/site-packages/omegaconf/listconfig.py", line 457, in __next__
    v = self.lst[self.index]
  File "/home/x/py-env/lib/python3.6/site-packages/omegaconf/listconfig.py", line 176, in __getitem__
    self._format_and_raise(key=index, value=None, cause=e)
  File "/home/x/py-env/lib/python3.6/site-packages/omegaconf/base.py", line 101, in _format_and_raise
    type_override=type_override,
  File "/home/x/py-env/lib/python3.6/site-packages/omegaconf/_utils.py", line 694, in format_and_raise
    _raise(ex, cause)
  File "/home/x/py-env/lib/python3.6/site-packages/omegaconf/_utils.py", line 610, in _raise
    raise ex  # set end OC_CAUSE=1 for full backtrace
omegaconf.errors.ConfigKeyError: str interpolation key 'key_a' not found
	full_key: list[0]
	reference_type=Optional[List[Any]]
	object_type=list

@omry
Copy link
Owner

omry commented Nov 23, 2020

Yup, now I understand. Thanks for reporting.

@omry omry added this to the OmegaConf 2.1 milestone Nov 23, 2020
@omry
Copy link
Owner

omry commented Nov 23, 2020

As a workaround, I suggest that you leave the list empty and allow every case to populate it.

base_conf_source = {
    "key_a": "large_object_a",
    "key_b": "large_object_b",
    "key_c": "large_object_c",
    "list": "???" # missing value, will raise an exception if accessed before overriden.
}

@mustafagok
Copy link
Author

Thanks, it worked. Unfortunately, I can't leave the list empty, as most clients use the default list. It leads to more config duplication for my case.

@omry
Copy link
Owner

omry commented Nov 28, 2020

Feel free to try to solve it master and sending a PR.
Note that this will also have to be fixed in the 2.0_branch if you want it fixed in 2.0 too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants