omegaconf icon indicating copy to clipboard operation
omegaconf copied to clipboard

Validation error merging structured configs containing enum in a nested list

Open edlanglois opened this issue 2 years ago • 6 comments

Describe the bug When merging into a structured config with a field of type List[List[SomeEnum]], any enums in the second (non-structured) config fail validation with

omegaconf.errors.ValidationError: Value 'SOME_VALUE' (str) is incompatible with type hint 'SomeEnum'

This does not occur for direct use of enums (value: SomeEnum) or non-nested lists (value: List[SomeEnum]).

To Reproduce

import enum
from dataclasses import dataclass, field
from typing import List

from omegaconf import OmegaConf


class MyEnum(enum.Enum):
    X = 1
    Y = 2


@dataclass
class Config:
    bare: MyEnum = MyEnum.X
    list: List[MyEnum] = field(default_factory=lambda: [MyEnum.X])
    list_list: List[List[MyEnum]] = field(default_factory=lambda: [[MyEnum.X]])


print("Base")
base = Config()
print(OmegaConf.to_yaml(base))

print("No nested list")
no_nested_list = OmegaConf.create({"bare": "Y", "list": ["Y"]})  # OK
print(OmegaConf.to_yaml(no_nested_list))

print("Merged without nested list")
print(OmegaConf.to_yaml(OmegaConf.merge(base, no_nested_list)))  # OK

print("Nested list")
nested_list = OmegaConf.create({"list_list": [["Y"]]})  # OK
print(OmegaConf.to_yaml(nested_list))

print("Merged with nested list")
print(OmegaConf.to_yaml(OmegaConf.merge(base, nested_list)))  # ValueError

Expected behavior I expect the merge of base and nested_list to complete successfully, resulting in the following structure:

bare: X
list:
- X
list_list:
- - 'Y'

instead I get

omegaconf.errors.ValidationError: Value 'Y' (str) is incompatible with type hint 'MyEnum'
    full_key: list_list[0]
    reference_type=List[List[MyEnum]]
    object_type=list

Additional context

  • [X] OmegaConf version: 2.3.0
  • [X] Python version: 3.8.10
  • [X] Operating system: Ubuntu 20.04.6 LTS
  • [X] Please provide a minimal repro

edlanglois avatar Jun 23 '23 20:06 edlanglois

Thanks for the report, @edlanglois.

It is indeed inconsistent that merge(base, no_nested_list) works and merge(base, nested_list) gets rejected. What you've put your finger on is that merge semantics for nested containers do not perform type coercion aggressively.

>>> @dataclass
... class Nested:
...  l: list[list[str]]
...
>>> OmegaConf.merge(Nested, {"l": [[2]]})  # rejects `2` as not a string
Traceback ...

>>> @dataclass
... class Shallow:
...  l: list[str]
...
>>> OmegaConf.merge(Shallow, {"l": [2]})  # coerces `2` to a string
{'l': ['2']}

Motivation for this design decision was that aggressive coercion might be unwanted (i.e. a ValidationError might be more desirable if the data are mistyped).

Your writeup also highlights the case of coercion from str to Enum. It might make sense to add a special case handling for str -> Enum coercion in nested containers since .yaml files don't support enum values natively. @omry, what do you think?

Jasha10 avatar Jun 25 '23 17:06 Jasha10

Thanks for explaining the underlying cause. Yes, my issue is that I'm unable to merge a .yaml config into the nested structured config.

edlanglois avatar Jun 25 '23 21:06 edlanglois

Your writeup also highlights the case of coercion from str to Enum. It might make sense to add a special case handling for str -> Enum coercion in nested containers since .yaml files don't support enum values natively. @omry, what do you think?

I am open to a fix in that direction. Ideally it would not be handled as a special condition though.

omry avatar Jul 02 '23 09:07 omry

Ideally it would not be handled as a special condition though.

What do you have in mind? Enabling type coercion fully within nested containers?

Jasha10 avatar Jul 02 '23 23:07 Jasha10

I think so.

omry avatar Jul 07 '23 20:07 omry