konradhalas / dacite

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

WrongTypeError: should be "float" instead of "int" #62

Closed Garrett-R closed 4 years ago

Garrett-R commented 5 years ago

If you run this code:

import dataclasses
from dataclasses import dataclass

import dacite

@dataclass
class Person:
    height: float = 160

person = Person()
person_dict = dataclasses.asdict(person)

new_person_1 = dacite.from_dict(data_class=Person, data=person_dict)

it gives this error:

WrongTypeError: wrong type for field "height" - should be "float" instead of "int"

I think it should be able to safely cast what it interprets as ints to be floats. Or alternatively, perhaps when doing asdict, it should save it as a float 160.0. Not sure which is better, or if the current behavior is desired since the user kind of erred with their datatype (although seems a bit user-unfriendly).

BTW, two "workarounds": 1) change a line above to height: float = 160.0 or cast to float 1) change the final line to:

new_person_2 = dacite.from_dict(data_class=Person, data=person_dict,
                                config=dacite.Config({float: float}))

Happy to submit a PR if you like.

tgpfeiffer commented 4 years ago

I have the same issue; I'm reading from a config file into a dict and then converting to a dataclass, which currently requires Union[int, float] everywhere because a value like 6 isn't recognized a valid float value.

According to mypy, x: float = 6 is valid, but isinstance(6, float) is false, so at least it is not obvious what is the right behavior here.

One concern I have is that while it is currently easy to work around this issue (using either Union or some explicit converter), if we change the behavior to implicitly accept ints as floats, then it is not possible any more to say "I actually do require a float here, not an integer". Do you have any opinion, @konradhalas?

konradhalas commented 4 years ago

Hi! Thank you @Garrett-R for reporting issue and also than you @tgpfeiffer for comprehensive response.

I would say that we should leave it as it is. If isinstance(6, float) returns False it means that dacite should also raise exception.

Solutions:

Garrett-R commented 4 years ago

Yeah, good points. And great lib BTW!

konradhalas commented 4 years ago

@Garrett-R thank you :)

konradhalas commented 4 years ago

I changed my mind, please check #74 for more info.

Garrett-R commented 4 years ago

Awesome, thanks! And thanks again for your work on this package!