konradhalas / dacite

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

Logic problem in types.is_optional? #161

Closed joseph-long closed 1 year ago

joseph-long commented 2 years ago

I was wondering why my optional union wasn't matching, and instead raising an exception from a type hook trying to convert a string into a float... and after some investigation I think it's this line:

https://github.com/konradhalas/dacite/blob/master/dacite/types.py#L48

Changing the implementation to

def is_optional(type_: Type) -> bool:
    members = extract_generic(type_)
    return is_union(type_) and type(None) in members and len(members) == 2

makes things work, and from_dict takes the _build_value path instead. (At least, I think so, if I'm following it correctly!)

Thanks for building this and sharing it. I apologize for not including a minimal reproducing example now, but it's late here and I wanted to write things down while they are in my head 🙃

konradhalas commented 2 years ago

Hi @joseph-long - thank you for reporting this issue.

It looks like a bug, probably same issue as #163. I hope I will fix it soon.

majiang commented 2 years ago

I've come across similar issue with A | B style type annotation for typing.Union.

date: datetime.date | None

didn't get type_hooks={datetime.date: datetime.date.fromisoformat} working, but

date: typing.Union[datetime.date, None]

did.

antonagestam commented 2 years ago

@majiang See fix for PEP 604 unions in #184.

konradhalas commented 1 year ago

@joseph-long hope it's fixed via https://github.com/konradhalas/dacite/pull/164