konradhalas / dacite

Simple creation of data classes from dictionaries.
MIT License
1.76k stars 106 forks source link

Dacite throws an error while using Optional on NewType #46

Closed AlwxSin closed 5 years ago

AlwxSin commented 5 years ago

Example

import dataclasses
from typing import  NewType, Optional

CUSTOM_TYPE = NewType('CUSTOM_TYPE', str)

@dataclasses.dataclass
class Test:
    usual_field: str
    custom_type_field: CUSTOM_TYPE
    optional_custom_type_field: Optional[CUSTOM_TYPE]

test = Test('usual', CUSTOM_TYPE('custom'), CUSTOM_TYPE('optional_custom'))
test_dict = dataclasses.asdict(test)
dacite.from_dict(Test, test_dict)

Output

---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-22-c59ddd955c43> in <module>
      8 test = Test('usual', CUSTOM_TYPE('custom'))
      9 test_dict = dataclasses.asdict(test)
---> 10 dacite.from_dict(Test, test_dict)

.venv/lib/python3.7/site-packages/dacite/core.py in from_dict(data_class, data, config)
     43                 error.update_path(field.name)
     44                 raise
---> 45             if config.check_types and not is_instance(value, field.type):
     46                 raise WrongTypeError(
     47                     field_path=field.name,

.venv/lib/python3.7/site-packages/dacite/types.py in is_instance(value, t)
     53     elif is_union(t):
     54         types = tuple(extract_origin_collection(t) if is_generic(t) else t for t in extract_generic(t))
---> 55         return isinstance(value, types)
     56     elif is_generic_collection(t):
     57         return isinstance(value, extract_origin_collection(t))

TypeError: isinstance() arg 2 must be a type or tuple of types

This is happens only if I use Optional[NewType]. Without Optional dacite works like charm.

>>> dataclasses.fields(Test)[0].type, dataclasses.fields(Test)[1].type, dataclasses.fields(Test)[2].type
(str,
 <function typing.NewType.<locals>.new_type(x)>,
 typing.Union[CUSTOM_TYPE, NoneType])
GreyZmeem commented 5 years ago

I think this error may be fixed by adding addition check for new type:

types = tuple(t.__supertype__ if is_new_type(t) else t for t in types)

after this line: https://github.com/konradhalas/dacite/blob/ece070cc3c25e86634086db8ee4f2e45bdfe6fe5/dacite/types.py#L53 @AlwxSin Can you check this? If everything is good, I will add tests and open a PR.

AlwxSin commented 5 years ago

@GreyZmeem sorry, but for several days I'll have only smartphone. I think using my example in issue description should be enough for testing.

konradhalas commented 5 years ago

@AlwxSin thank you for reporting this issue! It's definitely a bug. I've fixed it in the latest commit - 574015d

@GreyZmeem thank you for your suggestion - this was a problematic function.

GreyZmeem commented 5 years ago

@konradhalas I did not open a PR, because the fix I proposed only works in simple situations when underlying type is a generic type. But will fail if you have nested complex types like:

MyType = NewType("MyType", str)
MyType2 = NewType("MyType2", MyType)
MyType3 = NewType("MyType3", Optional[MyType])
...

It seems a recursive is_instance call needed for all NewTypes, if its __supertype__ is not a generic type.

konradhalas commented 5 years ago

@GreyZmeem hmm, interesting, I will check.