Skip to content

Commit

Permalink
feat(sdk): use custom basemodel and remove pydantic (kubeflow#7639)
Browse files Browse the repository at this point in the history
* fix discovered bug

* update tests

* implement custom BaseModel

* use basemodel for structures

* remove pydantic dependency

* assorted code cleanup
  • Loading branch information
connor-mccarthy authored and Jagadeesh J committed May 11, 2022
1 parent 01af152 commit 5919a9d
Show file tree
Hide file tree
Showing 10 changed files with 1,337 additions and 433 deletions.
16 changes: 8 additions & 8 deletions sdk/python/kfp/compiler/compiler_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -601,7 +601,7 @@ def pipeline_hello_world(text: str = 'hi there'):
v2.compiler.Compiler().compile(
pipeline_func=pipeline_hello_world, package_path=temp_filepath)

with open(temp_filepath, "r") as f:
with open(temp_filepath, 'r') as f:
yaml.load(f)

def test_import_modules(self): # pylint: disable=no-self-use
Expand All @@ -625,7 +625,7 @@ def pipeline_hello_world(text: str = 'hi there'):
compiler.Compiler().compile(
pipeline_func=pipeline_hello_world, package_path=temp_filepath)

with open(temp_filepath, "r") as f:
with open(temp_filepath, 'r') as f:
yaml.load(f)

def test_import_object(self): # pylint: disable=no-self-use
Expand All @@ -650,7 +650,7 @@ def pipeline_hello_world(text: str = 'hi there'):
Compiler().compile(
pipeline_func=pipeline_hello_world, package_path=temp_filepath)

with open(temp_filepath, "r") as f:
with open(temp_filepath, 'r') as f:
yaml.load(f)


Expand All @@ -670,8 +670,8 @@ def my_pipeline():
return my_pipeline

@parameterized.parameters(
{"extension": ".yaml"},
{"extension": ".yml"},
{'extension': '.yaml'},
{'extension': '.yml'},
)
def test_can_write_to_yaml(self, extension):

Expand Down Expand Up @@ -701,7 +701,7 @@ def test_can_write_to_json(self):

target_file = os.path.join(tmpdir, 'result.json')
with self.assertWarnsRegex(DeprecationWarning,
r"Compiling to JSON is deprecated"):
r'Compiling to JSON is deprecated'):
compiler.Compiler().compile(
pipeline_func=pipeline_spec, package_path=target_file)
with open(target_file) as f:
Expand Down Expand Up @@ -735,7 +735,7 @@ def test_compile_pipeline_with_default_value(self):
inputs:
- {name: location, type: String, default: 'us-central1'}
- {name: name, type: Integer, default: 1}
- {name: noDefault, type: String}
- {name: nodefault, type: String}
implementation:
container:
image: gcr.io/my-project/my-image:tag
Expand All @@ -745,7 +745,7 @@ def test_compile_pipeline_with_default_value(self):

@dsl.pipeline(name='test-pipeline')
def simple_pipeline():
producer = producer_op(location="1")
producer = producer_op(location='1', nodefault='string')

target_json_file = os.path.join(tmpdir, 'result.json')
compiler.Compiler().compile(
Expand Down
6 changes: 3 additions & 3 deletions sdk/python/kfp/components/base_component.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,11 @@ def __init__(self, component_spec: structures.ComponentSpec):
# Arguments typed as PipelineTaskFinalStatus are special arguments that
# do not count as user inputs. Instead, they are reserved to for the
# (backend) system to pass a value.
self._component_inputs = set([
self._component_inputs = {
input_name for input_name, input_spec in (
self.component_spec.inputs or {}).items()
if not type_utils.is_task_final_status_type(input_spec.type)
])
}

def __call__(self, *args, **kwargs) -> pipeline_task.PipelineTask:
"""Creates a PipelineTask object.
Expand Down Expand Up @@ -74,7 +74,7 @@ def __call__(self, *args, **kwargs) -> pipeline_task.PipelineTask:
missing_arguments = [
input_name for input_name, input_spec in (
self.component_spec.inputs or {}).items()
if input_name not in task_inputs and not input_spec.optional and
if input_name not in task_inputs and not input_spec._optional and
not type_utils.is_task_final_status_type(input_spec.type)
]
if missing_arguments:
Expand Down
42 changes: 22 additions & 20 deletions sdk/python/kfp/components/base_component_test.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright 2021 The Kubeflow Authors
# Copyright 2021-2022 The Kubeflow Authors
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
Expand All @@ -17,8 +17,8 @@
from unittest.mock import patch

from kfp.components import base_component
from kfp.components import structures
from kfp.components import pipeline_task
from kfp.components import structures


class TestComponent(base_component.BaseComponent):
Expand All @@ -30,18 +30,19 @@ def execute(self, *args, **kwargs):
component_op = TestComponent(
component_spec=structures.ComponentSpec(
name='component_1',
implementation=structures.ContainerSpec(
image='alpine',
command=[
'sh',
'-c',
'set -ex\necho "$0" "$1" "$2" > "$3"',
structures.InputValuePlaceholder(input_name='input1'),
structures.InputValuePlaceholder(input_name='input2'),
structures.InputValuePlaceholder(input_name='input3'),
structures.OutputPathPlaceholder(output_name='output1'),
],
),
implementation=structures.Implementation(
container=structures.ContainerSpec(
image='alpine',
command=[
'sh',
'-c',
'set -ex\necho "$0" "$1" "$2" > "$3"',
structures.InputValuePlaceholder(input_name='input1'),
structures.InputValuePlaceholder(input_name='input2'),
structures.InputValuePlaceholder(input_name='input3'),
structures.OutputPathPlaceholder(output_name='output1'),
],
)),
inputs={
'input1':
structures.InputSpec(type='String'),
Expand All @@ -53,7 +54,7 @@ def execute(self, *args, **kwargs):
structures.InputSpec(type='Optional[Float]', default=None),
},
outputs={
'output1': structures.OutputSpec(name='output1', type='String'),
'output1': structures.OutputSpec(type='String'),
},
))

Expand Down Expand Up @@ -92,25 +93,26 @@ def test_instantiate_component_with_positional_arugment(self):
with self.assertRaisesRegex(
TypeError,
'Components must be instantiated using keyword arguments.'
' Positional parameters are not allowed \(found 3 such'
' parameters for component "component_1"\).'):
r' Positional parameters are not allowed \(found 3 such'
r' parameters for component "component_1"\).'):
component_op('abc', 1, 2.3)

def test_instantiate_component_with_unexpected_keyword_arugment(self):
with self.assertRaisesRegex(
TypeError,
'component_1\(\) got an unexpected keyword argument "input0".'):
r'component_1\(\) got an unexpected keyword argument "input0".'
):
component_op(input1='abc', input2=1, input3=2.3, input0='extra')

def test_instantiate_component_with_missing_arugments(self):
with self.assertRaisesRegex(
TypeError,
'component_1\(\) missing 1 required argument: input1.'):
r'component_1\(\) missing 1 required argument: input1.'):
component_op(input2=1)

with self.assertRaisesRegex(
TypeError,
'component_1\(\) missing 2 required arguments: input1, input2.'
r'component_1\(\) missing 2 required arguments: input1, input2.'
):
component_op()

Expand Down
Loading

0 comments on commit 5919a9d

Please sign in to comment.