konradhalas / dacite

Simple creation of data classes from dictionaries.
MIT License
1.72k stars 107 forks source link

Optional and Union not working together #26

Closed adezegher closed 5 years ago

adezegher commented 5 years ago

It seems that when using Optional and Union together, the Optional attribute is ignored.

This test will fail right now because a MissingValueError is incorrectly raised:

@dataclass
class X:
    i: int

@dataclass
class Y:
    s: str

@dataclass
class Z:
    x_or_y: Optional[Union[X,Y]]

result = from_dict(Z, {'a': {'s': 'test'}})

assert result == Z(x_or_y=None)

Error:

dacite.MissingValueError: missing value for field x_or_y

Looking into the code it seems related to this function:

def _is_optional(t: Type) -> bool:
    return _is_union(t) and type(None) in t.__args__ and len(t.__args__) == 2

The function is returning false, when it should be returning true. This is because len(t.__args__) == 2 is returning false. What is the reason for the len == 2 check? It seems that removing this will solve this corner case.

konradhalas commented 5 years ago

@adezegher thank you for your issue.

It looks that your test is broken, instead of x_or_y key you have a in your data dict. This is why exception was raised.

I made a few tests and all of them are passing:

@dataclass
class X:
    i: int

@dataclass
class Y:
    s: str

@dataclass
class Z:
    x_or_y: Optional[Union[X, Y]]

result = from_dict(Z, {'x_or_y': None})
assert result == Z(x_or_y=None)

result = from_dict(Z, {'x_or_y': {'s': 'test'}})
assert result == Z(x_or_y=Y(s='test'))

result = from_dict(Z, {'x_or_y': {'i': 1}})
assert result == Z(x_or_y=X(i=1))
adezegher commented 5 years ago

@konradhalas - I indeed had an issue in my test - sorry about that.

There still seems to be a bug. When the key is defined with value None, dacite works fine on optional unions. When the key is undefined, it seems dacite considers the key required for optional unions.

This is a corrected test that shows the issue - the last one will fail because key 'x_or_y' is missing:

@dataclass
class X:
    i: int

@dataclass
class Y:
    s: str

@dataclass
class Z:
    x_or_y: Optional[Union[X, Y]]

@dataclass
class Z:
    x_or_y: Optional[Union[X, Y]]
    x: Optional[X]

result = from_dict(Z, {'not_in_class': {'i': 1}, 'x_or_y': None})
assert result == Z(x=None, x_or_y=None)

# => This fails because of missing x_or_y
result = from_dict(Z, {'not_in_class': {'i': 1}})
assert result == Z(x=None, x_or_y=None)
konradhalas commented 5 years ago

@adezegher you are right, we have to remove len(t.__args__) == 2. I will fix it soon.