konradhalas / dacite

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

Union match errors are difficult to debug for unions of complex types #58

Closed chaoflow closed 5 years ago

chaoflow commented 5 years ago

Given:

import dacite
from dataclasses import dataclass
from typing import List, Union

@dataclass
class Foo:
    x: int

@dataclass
class Bar:
    y: int

@dataclass
class Action:
    target: str

@dataclass
class FooAction(Action):
    foo: Foo

@dataclass
class BarAction(Action):
    bar: Bar

@dataclass
class Config:
    actions: List[Union[FooAction, BarAction]]

The following misses a value for target

cfg = dacite.from_dict(Config, {'actions': [
    {'foo': {'x': 1}},
    {'bar': {'y': 2}},
]})

and raises:

dacite.exceptions.UnionMatchError: can not match type "dict" to any type of "actions" union: typing.Union[__main__.FooAction, __main__.BarAction]

The very same exception is raised if the type of x is wrong:

cfg = dacite.from_dict(Config, {'actions': [
    {'foo': {'x': '1'}, 'target': 'target'},
    {'bar': {'y': 2}, 'target': 'target'},
]})

while this parses fine:

cfg = dacite.from_dict(Config, {'actions': [
    {'foo': {'x': 1}, 'target': 'target'},
    {'bar': {'y': 2}, 'target': 'target'},
]})

It would be great if dacite could produce more specific error messages when handling unions of complex types.

konradhalas commented 5 years ago

@chaoflow thank you for reporting this issue.

I have to say that Union is a pain in the neck, but well... dacite supports it and it should definitely provide some more informative exception.

Do you have any suggestions? I'm open for a PR.

The problem is that when we build value for Union field (core. _build_value_for_union) we try to match each type to the input data, and if something is wrong, we take the next one. This is why the easiest way is to say "we can not match any type from union" instead of "we can not match any type, this and this was very close, but it doesn't work because of..."

konradhalas commented 5 years ago

@chaoflow as I said, it's difficult to provide more relevant info in this case. I'm closing this issue for now. Feel free to reopen or submit PR :)

matdmul commented 4 months ago

Hi @konradhalas Thanks for a very useful library. I've just discovered this library and I noticed for my own use-case, when it throws a UnionMatchError, the value recorded is a dict (I'm guessing that's pretty standard considering what this lib is for?) but looking at: https://github.com/konradhalas/dacite/blob/master/dacite/exceptions.py#L48-L50 Vs https://github.com/konradhalas/dacite/blob/master/dacite/exceptions.py#L33-L35 for me, just recording self.value in the UnionMatchError just like its parent might make things a little simpler ? Or what have I missed?