konradhalas / dacite

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

`dacite.types.is_instance` bug for generic collections in unions. #114

Closed acturner closed 3 years ago

acturner commented 3 years ago

Here is the bug:

>>> import dacite
>>> from typing import Sequence, Union
>>> from number import Integral
>>> dacite.types.is_instance('0', Integral) or dacite.types.is_instance('0', Sequence[Integral])
False
>>> dacite.types.is_instance('0', Union[Integral, Sequence[Integral]])
True

This comes from the following lines in is_instance

    elif is_union(type_):
        types = []
        for inner_type in extract_generic(type_):
            if is_generic(inner_type) and not is_literal(inner_type):
                inner_type = extract_origin_collection(inner_type)
            if is_new_type(inner_type):
                inner_type = extract_new_type(inner_type)
            types.append(inner_type)
        return any(is_instance(value, t) for t in types)

since for the second argument inner_type=Sequence[Integral] becomes inner_type=Sequence (and for the example above, a string is a sequence) when passed through extract_origin_collection. I don't see why this block of code shouldn't read

    elif is_union(type_):
        return any(is_instance(value, t) for t in extract_generic(type_))

as the unions are fundamentally just the any operator (unless I'm missing something about what's going on here).

konradhalas commented 3 years ago

@acturner thank you for reporting this issue. You have right, it's a bug. TBH I don't know the answer (or I should say: I don't remember) why it was implemented that way. Fixed as you suggested, all tests green.