-
Notifications
You must be signed in to change notification settings - Fork 333
support None type in pyflyte run command #3348
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
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: yuteng <[email protected]>
Signed-off-by: yuteng <[email protected]>
|
Bito Automatic Review Skipped - Draft PR |
| ) from e | ||
|
|
||
|
|
||
| def is_optional(_type): |
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.
We can use is_optional_type from typing_inspect.
from typing_inspect import is_optional_type
flytekit/interaction/click_types.py
Outdated
| if not self._is_remote: | ||
| return value | ||
|
|
||
| if is_optional(self._python_type) and value == "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.
| if is_optional(self._python_type) and value == "None": | |
| if is_optional(self._python_type) and value.lower() == "none": |
| inputs_type = entity.python_interface.inputs | ||
| for input_name, v in entity.python_interface.inputs_with_defaults.items(): | ||
| processed_click_value = kwargs.get(input_name) | ||
| input_type = inputs_type[input_name] | ||
| if is_optional(input_type) and processed_click_value == "None": | ||
| processed_click_value = 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.
nit
| inputs_type = entity.python_interface.inputs | |
| for input_name, v in entity.python_interface.inputs_with_defaults.items(): | |
| processed_click_value = kwargs.get(input_name) | |
| input_type = inputs_type[input_name] | |
| if is_optional(input_type) and processed_click_value == "None": | |
| processed_click_value = None | |
| for input_name, v in entity.python_interface.inputs_with_defaults.items(): | |
| processed_click_value = kwargs.get(input_name) | |
| input_type = .python_interface.inputs[input_name] | |
| if is_optional(input_type) and processed_click_value == "None": | |
| processed_click_value = None |
Signed-off-by: yuteng <[email protected]>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3348 +/- ##
==========================================
- Coverage 47.70% 46.47% -1.23%
==========================================
Files 216 216
Lines 22692 22626 -66
Branches 2972 2972
==========================================
- Hits 10825 10516 -309
- Misses 11289 11588 +299
+ Partials 578 522 -56 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Tracking issue
flyteorg/flyte#6717
Why are the changes needed?
pyflyte command supports strong typing in pipeline and allow users to set the parameters by --var.
Supported None type make users set the None type or other values in their willing.
What changes were proposed in this pull request?
pyflyte run command parse None string to None type if type of parameter is Optional type.
pyflyte run --remote command parse None string to None type whether type of parameter is Optional type.
How was this patch tested?
Setup process
Screenshots
Check all the applicable boxes
Related PRs
Docs link
Summary by Bito