Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Logic problem in types.is_optional? #161

Closed
joseph-long opened this issue Oct 26, 2021 · 4 comments
Closed

Logic problem in types.is_optional? #161

joseph-long opened this issue Oct 26, 2021 · 4 comments
Labels
bug Something isn't working

Comments

@joseph-long
Copy link

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 🙃

joseph-long added a commit to xwcl/xconf that referenced this issue Oct 26, 2021
@konradhalas konradhalas added the bug Something isn't working label Apr 12, 2022
@konradhalas
Copy link
Owner

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
Copy link

majiang commented Apr 29, 2022

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
Copy link
Contributor

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

@konradhalas
Copy link
Owner

@joseph-long hope it's fixed via #164

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants