Skip to content

Commit

Permalink
Fixed handling parameters with default values in task factory constru…
Browse files Browse the repository at this point in the history
…ction (#1047)

* Fixed handling default inputs in task factory construction

* Added tests.
  • Loading branch information
Ark-kun authored and k8s-ci-robot committed Mar 27, 2019
1 parent e90a334 commit e452385
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 2 deletions.
4 changes: 2 additions & 2 deletions sdk/python/kfp/components/_components.py
Original file line number Diff line number Diff line change
Expand Up @@ -235,8 +235,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)]
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

0 comments on commit e452385

Please sign in to comment.