-
Notifications
You must be signed in to change notification settings - Fork 13
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
convert union type with none to optional #31
Conversation
any update on this? |
# because TypedDict can't handle union types with None | ||
# rewrite them as Optional[type] | ||
for arg_name, arg_type in new_type.items(): | ||
type_args = get_args(arg_type) | ||
if len(type_args) == 2 and type_args[1] is type(None): | ||
new_type[arg_name] = Optional[type_args[0]] |
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.
Wouldn't this be better in _handle_generics
? 🤔
If you have nested TypedDict
s this would fail in any case because we're checking only the first level right?
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.
I could try. Just the code was a little hard to understand and I've decided to do it the end.
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.
Ok, if you can't manage I can give it a try. Don't want to spoil all the fun. :)
@Rusteam Could you make the above-requested changes? |
Yes I will during this week. |
@silvanocerza added changes, is this what you mean? |
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.
Thanks! 🙏
@Rusteam sorry for the late review, was caught by higher priority stuff. |
TypedDict
is unable to handleUnion[type, NoneType]
, it requires the type to beOptional[type]
. I've added a small fix for that. This fixes #25.Example traceback with deploying a pipeline that contains
DocumentJoiner
: