Adding multiple outputs into sdk with sample #1667
Conversation
|
Hi @zanedurante. Thanks for your PR. I'm waiting for a kubeflow member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
| @@ -328,11 +340,15 @@ def _generate_entrypoint(self, component_func, python_version='python3'): | |||
| for input_arg in input_args: | |||
| func_signature += input_arg + ',' | |||
| func_signature += '_output_file):' | |||
| if output_is_named_tuple: | |||
| func_signature = func_signature.replace('file', 'files') | |||
There was a problem hiding this comment.
This might be fragile.
Let's do this:
if output_is_named_tuple:
func_signature += '_output_files'
else:
func_signature += '_output_file'
func_signature += '):'There was a problem hiding this comment.
Sounds good--updated!
|
Thank you for your contribution. It looks really good. |
|
Should be ready for testing @Ark-kun. |
|
/lgtm |
|
/cc @gaoning777 |
|
/ok-to-test |
|
I need to update this PR with the new test and component builder commits -- marking as WIP. |
|
Should now be up to date with the changes from the previous merge |
|
@gaoning777 PR should be ready to merge. |
|
/lgtm |
Sorry for the merge conflict. |
All good! |
|
Hi, Zane, thanks for all the efforts to resolve the merge conflicts. |
|
/lgtm |
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gaoning777 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest |
|
@gaoning777 do you know what this Travis CI error means? |
|
/lgtm |
|
/retest |
|
@gaoning777 looks ready to merge! |
Allows for build_python_component to use NamedTuples, fixing #1580.
Adds sample specifically for #1517, and for overall sample efforts #1034.
This change is