-
Notifications
You must be signed in to change notification settings - Fork 344
Add support union type #754
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,7 +28,7 @@ | |
| from flytekit.models import types as _type_models | ||
| from flytekit.models.core import types as _core_types | ||
| from flytekit.models.literals import Literal, LiteralCollection, LiteralMap, Primitive, Scalar | ||
| from flytekit.models.types import LiteralType, SimpleType | ||
| from flytekit.models.types import LiteralType, SimpleType, UnionType | ||
|
|
||
| T = typing.TypeVar("T") | ||
| DEFINITIONS = "definitions" | ||
|
|
@@ -444,7 +444,7 @@ def to_literal(cls, ctx: FlyteContext, python_val: typing.Any, python_type: Type | |
| """ | ||
| Converts a python value of a given type and expected ``LiteralType`` into a resolved ``Literal`` value. | ||
| """ | ||
| if python_val is None: | ||
| if python_val is None and expected.union_type is None: | ||
| raise AssertionError(f"Python value cannot be None, expected {python_type}/{expected}") | ||
| transformer = cls.get_transformer(python_type) | ||
| if transformer.type_assertions_enabled: | ||
|
|
@@ -596,6 +596,58 @@ def guess_python_type(self, literal_type: LiteralType) -> Type[list]: | |
| raise ValueError(f"List transformer cannot reverse {literal_type}") | ||
|
|
||
|
|
||
| class UnionTransformer(TypeTransformer[T]): | ||
| """ | ||
| Transformer that handles a univariate typing.Union[T] | ||
| """ | ||
|
|
||
| def __init__(self): | ||
| super().__init__("Typed Union", typing.Union) | ||
|
|
||
| @staticmethod | ||
| def get_sub_type(t: Type[T]) -> Type[T]: | ||
| """ | ||
| Return the generic Type T of the Union | ||
| """ | ||
| 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.") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. see above: only polyvariate Union types are meaningful |
||
|
|
||
| def get_literal_type(self, t: Type[T]) -> Optional[LiteralType]: | ||
| try: | ||
| sub_type = [TypeEngine.to_literal_type(v) if v else "" for v in self.get_sub_type(t)] | ||
| return _type_models.LiteralType(union_type=UnionType(sub_type)) | ||
| except Exception as e: | ||
| raise ValueError(f"Type of Generic Union type is not supported, {e}") | ||
|
|
||
| def to_literal(self, ctx: FlyteContext, python_val: T, python_type: Type[T], expected: LiteralType) -> Literal: | ||
| for t in python_type.__args__: | ||
| try: | ||
| return TypeEngine.to_literal(ctx, python_val, t, expected) | ||
| except Exception as e: | ||
| logger.debug(f"Failed to convert from {python_val} to {t}", e) | ||
| raise TypeError(f"Cannot convert from {python_val} to {python_type}") | ||
|
|
||
| def to_python_value(self, ctx: FlyteContext, lv: Literal, expected_python_type: Type[T]) -> Optional[typing.Any]: | ||
| if lv is None: | ||
| return None | ||
| st = self.get_sub_type(expected_python_type) | ||
| for v in st: | ||
| try: | ||
| val = TypeEngine.to_python_value(ctx, lv, v) | ||
| if val: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| return val | ||
| except Exception as e: | ||
| logger.debug(f"Failed to convert from {lv} to {v}", e) | ||
| raise TypeError(f"Cannot convert from {lv} to {expected_python_type}") | ||
|
|
||
| def guess_python_type(self, literal_type: LiteralType) -> type: | ||
| if literal_type.union_type: | ||
| return typing.Union[tuple(TypeEngine.guess_python_type(v) for v in literal_type.union_type.values)] | ||
| raise ValueError(f"Union transformer cannot reverse {literal_type}") | ||
|
|
||
|
|
||
| class DictTransformer(TypeTransformer[dict]): | ||
| """ | ||
| Transformer that transforms a univariate dictionary Dict[str, T] to a Literal Map or | ||
|
|
@@ -907,9 +959,11 @@ def _register_default_type_transformers(): | |
| _type_models.LiteralType(simple=_type_models.SimpleType.NONE), | ||
| lambda x: None, | ||
| lambda x: None, | ||
| ) | ||
| ), | ||
| [type(None)], | ||
| ) | ||
| TypeEngine.register(ListTransformer()) | ||
| TypeEngine.register(UnionTransformer()) | ||
| TypeEngine.register(DictTransformer()) | ||
| TypeEngine.register(TextIOTransformer()) | ||
| TypeEngine.register(BinaryIOTransformer()) | ||
|
|
||
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 method does not really make sense since a Union type only ever has multiple "sub types"
i.e.
Union[x]is by definition equivalent toxI think the way this method is used in
get_literal_typeimplies that the type signature should actually be(t: Type[T]) -> List[Type]insteadThere 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.
will update it, thanks