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

ValidationError for Unions in nested Dicts using merge with structured config #1166

Open denialofsandwich opened 8 months ago

denialofsandwich commented 8 months ago

Describe the bug The validation of unions, which are nested in a dict seems to be broken and throws a ValidationError during a merge, even if the types should be correct.

To Reproduce The last line is throwing the ValidationError:

from dataclasses import dataclass, field
from typing import Union

from omegaconf import OmegaConf

@dataclass
# Default is z.a.b = 1
class HasUnion:
    z: dict[str, dict[str, Union[str, int]]] = field(
        default_factory=lambda: {"a": {"b": 1}}
    )

# z.a.b = 1 (This works)
# z.c.b = 1 (This doesn't work)
cfg = OmegaConf.merge(  # <-- Oh, no!
    OmegaConf.structured(HasUnion),
    OmegaConf.create("""
z:
    c:
        b: 1
""")
)

This is the exception thwown at the last line:

ValidationError: Value 1 (int) is incompatible with type hint 'typing.Union[int, str]'
    full_key: z.c
    reference_type=Dict[str, Dict[str, Union[int, str]]]
    object_type=dict

Expected behavior I expect z.c.b = 1 not to fail, since 1 is a valid type of Union[str, int] and if the Union is not nested inside a dict.

Additional context

denialofsandwich commented 8 months ago

I found another few examples. Here with a dict[str, list[union]] type:

from dataclasses import dataclass, field
from typing import Union

from omegaconf import OmegaConf

@dataclass
# Default is z.a.b = 1
class HasUnion:
    z: dict[str, list[Union[str, int]]] = field(
        default_factory=lambda: {"a": [1,2]}
    )

# z.a = [1] (This works)
# z.c = [1] (This doesn't work)
cfg = OmegaConf.merge(
    OmegaConf.structured(HasUnion),
    OmegaConf.create("""
z:
    c:
        - 1
""")
)

And here with a dict[str, list[dataclass]] type. The validation doesn't work, if the parent key wasn't part of the default value before:

from dataclasses import dataclass, field

from omegaconf import OmegaConf

@dataclass
class Foo:
    exist1: int
    exist2: str

@dataclass
class BaseStructure:
    z: dict[str, list[Foo]] = field(
        default_factory=lambda: {"a": [Foo(1, "lol")]}
    )

cfg = OmegaConf.merge(
    OmegaConf.structured(BaseStructure),
    OmegaConf.create("""
z:
    c:
        - exist1: 1
          dontexist2: lol
""")
)

EDIT: I accidentally clicked on close, sry.

Jasha10 commented 8 months ago

Thanks for the bug report, @denialofsandwich.