-
Notifications
You must be signed in to change notification settings - Fork 196
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
Change codegen to treat typing.Union[Foo, NoneType] and typing.Optional[Foo] as the same #508
Conversation
…al[Foo] as the same
if typestr.startswith("<class '") and typestr.endswith("'>"): | ||
typestr = typestr[8:-2] | ||
typestr = re.sub(CLASS_RE, r"\1", typestr) | ||
typestr = re.sub(OPTIONAL_RE, r"typing.Optional[\1]", typestr) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This won't work for Union[A, B, NoneType]
or Union[A, None]
. While this might not cause an issue today, I don't want to be on the debugging end if it pops up in the future :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why don't it work for Union[A, B, NoneType]
, my regex rejects that.
I'm not sure I understand the difference between Union[A, NoneType]
and Union[A, None]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, fair enough. apparently Option[Union[A, B]]
always gets flattened to Union[A, B, NoneType]
by dataclasses, so it doesn't even matter.
if typestr.startswith("<class '") and typestr.endswith("'>"): | ||
typestr = typestr[8:-2] | ||
typestr = re.sub(CLASS_RE, r"\1", typestr) | ||
typestr = re.sub(OPTIONAL_RE, r"typing.Optional[\1]", typestr) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, fair enough. apparently Option[Union[A, B]]
always gets flattened to Union[A, B, NoneType]
by dataclasses, so it doesn't even matter.
Summary
This is a fix for #467. As per this comment, the type of optional fields in dataclasses has changed in python 3.9.
This change makes the codegen rewrite the old type to the new type, so it works the same in all versions.
Test Plan
Codegen tests