-
Notifications
You must be signed in to change notification settings - Fork 333
[Core feature] Add support Literal Transformer #3304
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
Conversation
Signed-off-by: Barry Wu <[email protected]>
Thank you for opening this pull request! 🙌 These tips will help get your PR across the finish line:
|
Signed-off-by: Barry Wu <[email protected]>
What is the goal of a literal type? Is it to provide an enum like behavior or just a weird way of passing primitives |
Currently, the Literal will be serialized to a pickle file instead of proto, and it would be nice to have the typing.Literal parameter also render as string in the UI. Before, the input in UI is shown as like feature: "single (PythonPickle) blob" if the input type is Literal. I think the goal is to support the Literal can be serialized to primitive |
Signed-off-by: Barry Wu <[email protected]>
Based on my understanding, Literal type in Python is to set the acceptable value for a variable. Probably a bit like For example, if |
Hi @BarryWu0812 , Could you please fix the CI error? Thanks! |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3304 +/- ##
==========================================
+ Coverage 44.53% 47.62% +3.09%
==========================================
Files 295 215 -80
Lines 26792 22634 -4158
Branches 2955 2965 +10
==========================================
- Hits 11931 10779 -1152
+ Misses 14764 11278 -3486
- Partials 97 577 +480 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Barry Wu <[email protected]>
Hi @machichima , I fixed it, Thanks! |
Signed-off-by: Barry Wu <[email protected]>
Could you please run an example using Thanks |
Hi @machichima , I have uploaded the result. Thanks |
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 for working on it. Could you also add some unit tests
…t tests Signed-off-by: Barry Wu <[email protected]>
flytekit/core/type_engine.py
Outdated
return base_transformer.to_python_value(ctx, lv, base_type) | ||
|
||
def guess_python_type(self, literal_type: LiteralType) -> Type: | ||
ann = getattr(literal_type, "annotation", 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.
literal_type will always have an annotation attribute, right? I think we can just do if literal_type.annotation and literal_type.annotation.annotations
Signed-off-by: Barry Wu <[email protected]>
Signed-off-by: Barry Wu <[email protected]>
Signed-off-by: Barry Wu <[email protected]>
Signed-off-by: Barry Wu <[email protected]> Co-authored-by: Barry Wu <[email protected]> Signed-off-by: Atharva <[email protected]>
Tracking issue
Closes flyteorg/flyte#6503
Why are the changes needed?
Support to serialize
Literal
to a literal primitiveWhat changes were proposed in this pull request?
Add a
Literal
transformerHow was this patch tested?
Unit test
and
Run the below workflow
Screenshots?
Check all the applicable boxes
Summary by Bito
This pull request introduces a new Literal transformer in the Flyte framework, enhancing the type engine's ability to serialize Literal types into primitive types. It improves data type handling and usability in type management, and includes methods for converting between Literal and Python values. Unit tests have been added to verify the new functionality.