Conversation
Signed-off-by: Kevin Su <pingsutw@apache.org>
Signed-off-by: Kevin Su <pingsutw@apache.org>
Signed-off-by: Kevin Su <pingsutw@apache.org>
Codecov Report
@@ Coverage Diff @@
## master #754 +/- ##
==========================================
+ Coverage 85.61% 85.65% +0.03%
==========================================
Files 342 342
Lines 28914 29033 +119
Branches 2377 2388 +11
==========================================
+ Hits 24755 24868 +113
- Misses 3527 3533 +6
Partials 632 632
Continue to review full report at Codecov.
|
Signed-off-by: Kevin Su <pingsutw@apache.org>
Signed-off-by: Kevin Su <pingsutw@apache.org>
Signed-off-by: Kevin Su <pingsutw@apache.org>
| super().__init__("Typed Union", typing.Union) | ||
|
|
||
| @staticmethod | ||
| def get_sub_type(t: Type[T]) -> Type[T]: |
There was a problem hiding this comment.
this method does not really make sense since a Union type only ever has multiple "sub types"
i.e. Union[x] is by definition equivalent to x
I think the way this method is used in get_literal_type implies that the type signature should actually be (t: Type[T]) -> List[Type] instead
| if hasattr(t, "__origin__") and t.__origin__ is typing.Union: # type: ignore | ||
| if hasattr(t, "__args__"): | ||
| return t.__args__ # type: ignore | ||
| raise ValueError("Only generic univariate typing.Union[T] type is supported.") |
There was a problem hiding this comment.
see above: only polyvariate Union types are meaningful
| for v in st: | ||
| try: | ||
| val = TypeEngine.to_python_value(ctx, lv, v) | ||
| if val: |
There was a problem hiding this comment.
val could be None if this is an Optional. None should not indicate failure
There was a problem hiding this comment.
val will be None only when Literal is None. if Literal is None, we will return None in line 634.
So we should return val if and only if val has value here, otherwise, we should raise an error.
|
Closing in favor of #763 |
Signed-off-by: Kevin Su pingsutw@apache.org
TL;DR
Add union type transformer, and it can handle both union and optional type
Type
Are all requirements met?
Complete description
How did you fix the bug, make the feature etc. Link to any design docs etc
Tracking Issue
flyteorg/flyte#1349
Follow-up issue
No