Skip to content
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

Fixed handling parameters with default values in task factory construction #1047

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions sdk/python/kfp/components/_components.py
Original file line number Diff line number Diff line change
Expand Up @@ -230,8 +230,8 @@ def create_task_from_component_and_arguments(pythonic_arguments):
import inspect
from . import _dynamic

#Reordering the inputs since in Python optional parameters must come after reuired parameters
reordered_input_list = [input for input in inputs_list if not input.optional] + [input for input in inputs_list if input.optional]
#Reordering the inputs since in Python optional parameters must come after required parameters
reordered_input_list = [input for input in inputs_list if input.default is None and not input.optional] + [input for input in inputs_list if not (input.default is None and not input.optional)]
Ark-kun marked this conversation as resolved.
Show resolved Hide resolved
Ark-kun marked this conversation as resolved.
Show resolved Hide resolved
Ark-kun marked this conversation as resolved.
Show resolved Hide resolved
input_parameters = [_dynamic.KwParameter(input_name_to_pythonic[port.name], annotation=(_try_get_object_by_name(str(port.type)) if port.type else inspect.Parameter.empty), default=port.default if port.default is not None else (None if port.optional else inspect.Parameter.empty)) for port in reordered_input_list]
factory_function_parameters = input_parameters #Outputs are no longer part of the task factory function signature. The paths are always generated by the system.

Expand Down
43 changes: 43 additions & 0 deletions sdk/python/tests/components/test_components.py
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,49 @@ def test_optional_inputs_reordering(self):
actual_signature = list(signature.parameters.keys())
self.assertSequenceEqual(actual_signature, ['in1', 'in3', 'in2'], str)

def test_inputs_reordering_when_inputs_have_defaults(self):
'''Tests reordering of inputs with default values.
In python signature, optional arguments must come after the required arguments.
'''
component_text = '''\
inputs:
- {name: in1}
- {name: in2, default: val}
- {name: in3}
implementation:
container:
image: busybox
'''
task_factory1 = comp.load_component_from_text(component_text)
import inspect
signature = inspect.signature(task_factory1)
actual_signature = list(signature.parameters.keys())
self.assertSequenceEqual(actual_signature, ['in1', 'in3', 'in2'], str)

def test_inputs_reordering_stability(self):
'''Tests input reordering stability. Required inputs and optional/default inputs should keep the ordering.
In python signature, optional arguments must come after the required arguments.
'''
component_text = '''\
inputs:
- {name: a1}
- {name: b1, default: val}
- {name: a2}
- {name: b2, optional: True}
- {name: a3}
- {name: b3, default: val}
- {name: a4}
- {name: b4, optional: True}
implementation:
container:
image: busybox
'''
task_factory1 = comp.load_component_from_text(component_text)
import inspect
signature = inspect.signature(task_factory1)
actual_signature = list(signature.parameters.keys())
self.assertSequenceEqual(actual_signature, ['a1', 'a2', 'a3', 'a4', 'b1', 'b2', 'b3', 'b4'], str)

def test_missing_optional_input_value_argument(self):
'''Missing optional inputs should resolve to nothing'''
component_text = '''\
Expand Down