SDK/Components - Removed the old argument syntax (#168)

sdk/python/kfp/components/_components.py

@@ -231,19 +231,15 @@ def create_container_op_with_expanded_arguments(pythonic_input_argument_values):
         file_outputs = file_outputs_from_def.copy()
 
         def expand_command_part(arg): #input values with original names
-            #(Union[str,List],Mapping[str, Any]) -> str
-            if isinstance(arg, dict) or isinstance(arg, list):
-                if isinstance(arg, list):
-                    (func_name, func_argument) = (arg[0], arg[1:])
-                    if len(func_argument) == 1:
-                        func_argument = func_argument[0]
-                elif isinstance(arg, dict):
-                    if len(arg) != 1:
-                        raise ValueError('Failed to parse argument dict: "{}"'.format(arg))
-                    (func_name, func_argument) = list(arg.items())[0]
-                else:
-                    raise TypeError()
+            #(Union[str,Mapping[str, Any]]) -> Union[str,List[str]]
+            if isinstance(arg, str):
+                return arg
+            elif isinstance(arg, dict):
+                if len(arg) != 1:
+                    raise ValueError('Failed to parse argument dict: "{}"'.format(arg))
+                (func_name, func_argument) = list(arg.items())[0]
                 func_name=func_name.lower()
+
                 if func_name == 'value':
                     assert isinstance(func_argument, str)
                     port_name = func_argument
@@ -285,19 +281,14 @@ def expand_command_part(arg): #input values with original names
                     return ''.join(expanded_argument_strings)
 
                 elif func_name == 'if':
-                    if isinstance(func_argument, dict):
-                        condition_node = func_argument['cond']
-                        then_node = func_argument['then']
-                        else_node = func_argument.get('else', None)
-                    elif isinstance(func_argument, list):
-                        assert len(func_argument) in [2, 3]
-                        condition_node = func_argument[0]
-                        then_node = func_argument[1]
-                        else_node = func_argument[2] if len(func_argument) == 3 else None
-                    else:
-                        raise TypeError()
+                    assert isinstance(func_argument, dict)
+                    condition_node = func_argument['cond']
+                    then_node = func_argument['then']
+                    else_node = func_argument.get('else', None)
                     condition_result = bool(expand_command_part(condition_node))
                     result_node = then_node if condition_result else else_node
+                    if result_node is None:
+                        return []
                     if isinstance(result_node, list):
                         expanded_result = [expand_command_part(arg1) for arg1 in result_node]
                     else:
@@ -311,7 +302,7 @@ def expand_command_part(arg): #input values with original names
                     argument_is_present = pythonic_input_name in pythonic_input_argument_values
                     return str(argument_is_present)
             else:
-                return arg
+                raise TypeError('Unrecognized argument type: {}'.format(arg))
 
         expanded_command = []
         if container_spec.command != None:

sdk/python/tests/components/test_components.py

@@ -226,23 +226,7 @@ def test_load_component_from_url_fail_on_none_arg(self):
     def test_load_component_from_text_fail_on_none_arg(self):
         comp.load_component_from_text(None)
 
-    def test_input_value_resolving_syntax1(self):
-        component_text = '''\
-inputs:
-- {name: Data}
-implementation:
-  dockerContainer:
-    image: busybox
-    arguments:
-      - --data
-      - [value, Data]
-'''
-        task_factory1 = comp.load_component(text=component_text)
-        task1 = task_factory1('some-data')
-
-        self.assertEqual(task1.arguments, ['--data', 'some-data'])
-
-    def test_input_value_resolving_syntax3(self):
+    def test_input_value_resolving(self):
         component_text = '''\
 inputs:
 - {name: Data}
@@ -258,22 +242,6 @@ def test_input_value_resolving_syntax3(self):
 
         self.assertEqual(task1.arguments, ['--data', 'some-data'])
 
-    def test_output_resolving_syntax1(self):
-        component_text = '''\
-outputs:
-- {name: Data}
-implementation:
-  dockerContainer:
-    image: busybox
-    arguments:
-      - --output-data
-      - [output, Data]
-'''
-        task_factory1 = comp.load_component(text=component_text)
-        task1 = task_factory1(data='/outputs/some-data')
-
-        self.assertEqual(task1.arguments, ['--output-data', '/outputs/some-data'])
-
     def test_output_resolving(self):
         component_text = '''\
 outputs:
@@ -290,22 +258,6 @@ def test_output_resolving(self):
 
         self.assertEqual(task1.arguments, ['--output-data', '/outputs/some-data'])
 
-    def test_automatic_output_resolving_syntax1(self):
-        component_text = '''\
-outputs:
-- {name: Data}
-implementation:
-  dockerContainer:
-    image: busybox
-    arguments:
-      - --output-data
-      - [output, Data]
-'''
-        task_factory1 = comp.load_component(text=component_text)
-        task1 = task_factory1()
-
-        self.assertEqual(len(task1.arguments), 2)
-
     def test_automatic_output_resolving(self):
         component_text = '''\
 outputs:
@@ -338,44 +290,6 @@ def test_command_concat(self):
 
         self.assertEqual(task1.arguments, ['somedata'])
 
-    def test_command_if_then_else_syntax1(self):
-        component_text = '''\
-inputs:
-- {name: In, required: false}
-implementation:
-  dockerContainer:
-    image: busybox
-    arguments:
-      - [if, [isPresent, In], [--in, [value, In]], --no-in]
-'''
-        task_factory1 = comp.load_component(text=component_text)
-
-        task_then = task_factory1('data')
-        self.assertEqual(task_then.arguments, ['--in', 'data']) 
-
-        #TODO: Fix optional arguments
-        #task_else = task_factory1() #Error: TypeError: Component() missing 1 required positional argument: 'in'
-        #self.assertEqual(task_else.arguments, ['--no-in'])
-
-    def test_command_if_then_syntax1(self):
-        component_text = '''\
-inputs:
-- {name: In, required: false}
-implementation:
-  dockerContainer:
-    image: busybox
-    arguments:
-      - [if, [isPresent, In], [--in, [value, In]]]
-'''
-        task_factory1 = comp.load_component(text=component_text)
-
-        task_then = task_factory1('data')
-        self.assertEqual(task_then.arguments, ['--in', 'data']) 
-
-        #TODO: Fix optional arguments
-        #task_else = task_factory1() #Error: TypeError: Component() missing 1 required positional argument: 'in'
-        #self.assertEqual(task_else.arguments, [])
-
     def test_command_if_then(self):
         component_text = '''\
 inputs: