From 592ca36f22a58aeffbad5902b999678cc43e1c96 Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Thu, 28 Feb 2019 17:27:32 -0800 Subject: [PATCH 01/42] add core types and type checking function --- sdk/python/kfp/dsl/_types.py | 206 +++++++++++++++++++++++++++++ sdk/python/tests/dsl/main.py | 3 +- sdk/python/tests/dsl/type_tests.py | 92 +++++++++++++ 3 files changed, 300 insertions(+), 1 deletion(-) create mode 100644 sdk/python/kfp/dsl/_types.py create mode 100644 sdk/python/tests/dsl/type_tests.py diff --git a/sdk/python/kfp/dsl/_types.py b/sdk/python/kfp/dsl/_types.py new file mode 100644 index 00000000000..e5c467e212a --- /dev/null +++ b/sdk/python/kfp/dsl/_types.py @@ -0,0 +1,206 @@ +# Copyright 2018 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +class MetaType: + '''MetaType is a base type for all scalar and artifact types. + ''' + pass + +# Primitive Types +class MetaInt(MetaType): + openapi_schema_validator = '''{ + "type": "integer" + }''' + +def Integer(attr={}): + return type('Integer', (MetaInt, ), attr) + +class MetaString(MetaType): + openapi_schema_validator = '''{ + "type": "string" + }''' + +def String(attr={}): + return type('String', (MetaString, ), attr) + +class MetaFloat(MetaType): + openapi_schema_validator = '''{ + "type": "number" + }''' + +def Float(attr={}): + return type('Float', (MetaFloat, ), attr) + +class MetaBool(MetaType): + openapi_schema_validator = '''{ + "type": "boolean" + }''' + +def Bool(attr={}): + return type('Bool', (MetaBool, ), attr) + +class MetaList(MetaType): + openapi_schema_validator = '''{ + "type": "array" + }''' + +def List(attr={}): + return type('List', (MetaList, ), attr) + +class MetaDict(MetaType): + openapi_schema_validator = '''{ + "type": "object", + }''' + +def Dict(attr={}): + return type('Dict', (MetaDict, ), attr) + +# GCP Types +class MetaGCSPath(MetaType): + openapi_schema_validator = '''{ + "type": "string", + "pattern": "^gs://$" + } + }''' + # path_type describes the paths, for example, bucket, directory, file, etc. + path_type = '' + # file_type describes the files, for example, JSON, CSV, etc. + file_type = '' + +def GCSPath(attr={}): + return type('GCSPath', (MetaGCSPath, ), attr) + +class MetaGCRPath(MetaType): + openapi_schema_validator = '''{ + "type": "string", + "pattern": "^(us.|eu.|asia.)?gcr\\.io/.*$" + } + }''' + +def GCRPath(attr={}): + return type('GCRPath', (MetaGCRPath, ), attr) + +class MetaGCPRegion(MetaType): + openapi_schema_validator = '''{ + "type": "string", + "enum": ["asia-east1","asia-east2","asia-northeast1", + "asia-south1","asia-southeast1","australia-southeast1", + "europe-north1","europe-west1","europe-west2", + "europe-west3","europe-west4","northamerica-northeast1", + "southamerica-east1","us-central1","us-east1", + "us-east4","us-west1", "us-west4" ] + }''' + +def GCPRegion(attr={}): + return type('GCPRegion', (MetaGCPRegion, ), attr) + +class MetaGCPProjectID(MetaType): + '''MetaGCPProjectID: GCP project id''' + openapi_schema_validator = '''{ + "type": "string" + }''' + +def GCPProjectID(attr={}): + return type('GCPProjectID', (MetaGCPProjectID, ), attr) + +# General Types +class MetaLocalPath(MetaType): + #TODO: add restriction to path + openapi_schema_validator = '''{ + "type": "string" + }''' + +def LocalPath(attr={}): + return type('LocalPath', (MetaLocalPath, ), attr) + +class InconsistentTypeException(Exception): + '''InconsistencyTypeException is raised when two types are not consistent''' + pass + +def _check_valid_dict(payload): + '''_check_valid_dict_type checks whether a dict is a correct serialization of a type + Args: + payload(dict) + ''' + if not isinstance(payload, dict) or len(payload) != 1: + return False + for type_name in payload: + if not isinstance(payload[type_name], dict): + return False + property_types = (int, str, float, bool) + for property_name in payload[type_name]: + if not isinstance(property_name, property_types) or not isinstance(payload[type_name][property_name], property_types): + return False + return True + +def _class_to_dict(payload): + '''serialize_type serializes the type instance into a json string + Args: + payload(type): A class that describes a type + + Return: + dict + ''' + attrs = set([i for i in dir(payload) if not i.startswith('__')]) + json_dict = {payload.__name__: {}} + for attr in attrs: + json_dict[payload.__name__][attr] = getattr(payload, attr) + return json_dict + +def _str_to_dict(payload): + import json + json_dict = json.loads(payload) + if not _check_valid_dict(json_dict): + raise ValueError(payload + ' is not a valid type string') + return json_dict + +def _check_dict_types(typeA, typeB): + '''_check_type_types checks the type consistency. + Args: + typeA (dict): A dict that describes a type from the upstream component output + typeB (dict): A dict that describes a type from the downstream component input + ''' + typeA_name,_ = list(typeA.items())[0] + typeB_name,_ = list(typeB.items())[0] + if typeA_name != typeB_name: + return False + type_name = typeA_name + for type_property in typeA[type_name]: + if type_property not in typeB[type_name]: + print(type_name + ' has a property ' + str(type_property) + ' that the latter does not.') + return False + if typeA[type_name][type_property] != typeB[type_name][type_property]: + print(type_name + ' has a property ' + str(type_property) + ' with value: ' + + str(typeA[type_name][type_property]) + ' and ' + + str(typeB[type_name][type_property])) + return False + return True + +def check_types(typeA, typeB): + '''check_types checks the type consistency. + For each of the attribute in typeA, there is the same attribute in typeB with the same value. + However, typeB could contain more attributes that typeA does not contain. + Args: + typeA (type/str/dict): it describes a type from the upstream component output + typeB (type/str/dict): it describes a type from the downstream component input + ''' + if isinstance(typeA, type): + typeA = _class_to_dict(typeA) + elif isinstance(typeA, str): + typeA = _str_to_dict(typeA) + if isinstance(typeB, type): + typeB = _class_to_dict(typeB) + elif isinstance(typeB, str): + typeB = _str_to_dict(typeB) + return _check_dict_types(typeA, typeB) \ No newline at end of file diff --git a/sdk/python/tests/dsl/main.py b/sdk/python/tests/dsl/main.py index 7e194cf386f..6192a944d88 100644 --- a/sdk/python/tests/dsl/main.py +++ b/sdk/python/tests/dsl/main.py @@ -20,7 +20,7 @@ import pipeline_param_tests import container_op_tests import ops_group_tests - +import type_tests if __name__ == '__main__': suite = unittest.TestSuite() @@ -28,6 +28,7 @@ suite.addTests(unittest.defaultTestLoader.loadTestsFromModule(pipeline_tests)) suite.addTests(unittest.defaultTestLoader.loadTestsFromModule(container_op_tests)) suite.addTests(unittest.defaultTestLoader.loadTestsFromModule(ops_group_tests)) + suite.addTests(unittest.defaultTestLoader.loadTestsFromModule(type_tests)) runner = unittest.TextTestRunner() if not runner.run(suite).wasSuccessful(): sys.exit(1) diff --git a/sdk/python/tests/dsl/type_tests.py b/sdk/python/tests/dsl/type_tests.py new file mode 100644 index 00000000000..1fed6d20a8d --- /dev/null +++ b/sdk/python/tests/dsl/type_tests.py @@ -0,0 +1,92 @@ +# Copyright 2018 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + + +from kfp.dsl._types import _class_to_dict, _str_to_dict, check_types, GCSPath +import unittest + +class TestTypes(unittest.TestCase): + + def test_class_to_dict(self): + """Test _class_to_dict function.""" + gcspath_dict = _class_to_dict(GCSPath({'path_type': 'file', 'file_type': 'csv'})) + golden_dict = { + 'GCSPath': { + 'path_type': 'file', + 'file_type': 'csv', + 'openapi_schema_validator': '''{ + "type": "object", + "properties": { + "path": { + "type": "string", + "pattern": "^gs://$" + } + } + + }''' + } + } + self.assertEqual(golden_dict, gcspath_dict) + + def test_str_to_dict(self): + gcspath_str = '{"GCSPath": {"file_type": "csv", "path_type": "file"}}' + gcspath_dict = _str_to_dict(gcspath_str) + golden_dict = { + 'GCSPath': { + 'path_type': 'file', + 'file_type': 'csv' + } + } + self.assertEqual(golden_dict, gcspath_dict) + gcspath_str = '{"file_type": "csv", "path_type": "file"}' + with self.assertRaises(ValueError): + gcspath_dict = _str_to_dict(gcspath_str) + + def test_check_types(self): + #Core types + typeA = GCSPath({'path_type': 'file', 'file_type': 'csv'}) + typeB = GCSPath({'path_type': 'file', 'file_type': 'csv'}) + self.assertTrue(check_types(typeA, typeB)) + typeA = GCSPath({'path_type': 'file', 'file_type': 'tsv'}) + typeB = GCSPath({'path_type': 'file', 'file_type': 'csv'}) + self.assertFalse(check_types(typeA, typeB)) + + # Custom types + typeA = { + 'A':{ + 'X': 'value1', + 'Y': 'value2' + } + } + typeB = { + 'B':{ + 'X': 'value1', + 'Y': 'value2' + } + } + typeC = { + 'A':{ + 'X': 'value1' + } + } + typeD = { + 'A':{ + 'X': 'value1', + 'Y': 'value3' + } + } + self.assertFalse(check_types(typeA, typeB)) + self.assertFalse(check_types(typeA, typeC)) + self.assertTrue(check_types(typeC, typeA)) + self.assertFalse(check_types(typeA, typeD)) \ No newline at end of file From 3a3fc179692d99e27ac517d47612e5f1c0b76cdc Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Thu, 28 Feb 2019 17:52:28 -0800 Subject: [PATCH 02/42] fix unit test bug --- sdk/python/kfp/dsl/_types.py | 1 - sdk/python/tests/dsl/type_tests.py | 10 ++-------- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/sdk/python/kfp/dsl/_types.py b/sdk/python/kfp/dsl/_types.py index e5c467e212a..7d2baaff7b9 100644 --- a/sdk/python/kfp/dsl/_types.py +++ b/sdk/python/kfp/dsl/_types.py @@ -71,7 +71,6 @@ class MetaGCSPath(MetaType): openapi_schema_validator = '''{ "type": "string", "pattern": "^gs://$" - } }''' # path_type describes the paths, for example, bucket, directory, file, etc. path_type = '' diff --git a/sdk/python/tests/dsl/type_tests.py b/sdk/python/tests/dsl/type_tests.py index 1fed6d20a8d..d7791264b6b 100644 --- a/sdk/python/tests/dsl/type_tests.py +++ b/sdk/python/tests/dsl/type_tests.py @@ -26,14 +26,8 @@ def test_class_to_dict(self): 'path_type': 'file', 'file_type': 'csv', 'openapi_schema_validator': '''{ - "type": "object", - "properties": { - "path": { - "type": "string", - "pattern": "^gs://$" - } - } - + "type": "string", + "pattern": "^gs://$" }''' } } From db45632c5f165e89aeb30b1361cb062e6286dcb3 Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Fri, 1 Mar 2019 10:23:44 -0800 Subject: [PATCH 03/42] avoid defining dynamic classes --- sdk/python/kfp/dsl/_types.py | 86 ++++++++++-------------------- sdk/python/tests/dsl/type_tests.py | 19 +++---- 2 files changed, 34 insertions(+), 71 deletions(-) diff --git a/sdk/python/kfp/dsl/_types.py b/sdk/python/kfp/dsl/_types.py index 7d2baaff7b9..71993d06512 100644 --- a/sdk/python/kfp/dsl/_types.py +++ b/sdk/python/kfp/dsl/_types.py @@ -12,85 +12,66 @@ # See the License for the specific language governing permissions and # limitations under the License. -class MetaType: +class BaseType: '''MetaType is a base type for all scalar and artifact types. ''' pass # Primitive Types -class MetaInt(MetaType): +class Integer(BaseType): openapi_schema_validator = '''{ "type": "integer" }''' -def Integer(attr={}): - return type('Integer', (MetaInt, ), attr) - -class MetaString(MetaType): +class String(BaseType): openapi_schema_validator = '''{ "type": "string" }''' -def String(attr={}): - return type('String', (MetaString, ), attr) - -class MetaFloat(MetaType): +class Float(BaseType): openapi_schema_validator = '''{ "type": "number" }''' -def Float(attr={}): - return type('Float', (MetaFloat, ), attr) - -class MetaBool(MetaType): +class Bool(BaseType): openapi_schema_validator = '''{ "type": "boolean" }''' -def Bool(attr={}): - return type('Bool', (MetaBool, ), attr) - -class MetaList(MetaType): +class List(BaseType): openapi_schema_validator = '''{ "type": "array" }''' -def List(attr={}): - return type('List', (MetaList, ), attr) - -class MetaDict(MetaType): +class Dict(BaseType): openapi_schema_validator = '''{ "type": "object", }''' -def Dict(attr={}): - return type('Dict', (MetaDict, ), attr) - # GCP Types -class MetaGCSPath(MetaType): +class GCSPath(BaseType): openapi_schema_validator = '''{ "type": "string", "pattern": "^gs://$" }''' - # path_type describes the paths, for example, bucket, directory, file, etc. - path_type = '' - # file_type describes the files, for example, JSON, CSV, etc. - file_type = '' -def GCSPath(attr={}): - return type('GCSPath', (MetaGCSPath, ), attr) + def __init__(self, path_type='', file_type=''): + ''' + Args + :param path_type: describes the paths, for example, bucket, directory, file, etc + :param file_type: describes the files, for example, JSON, CSV, etc. + ''' + self.path_type = path_type + self.file_type = file_type -class MetaGCRPath(MetaType): +class GCRPath(BaseType): openapi_schema_validator = '''{ "type": "string", "pattern": "^(us.|eu.|asia.)?gcr\\.io/.*$" } }''' -def GCRPath(attr={}): - return type('GCRPath', (MetaGCRPath, ), attr) - -class MetaGCPRegion(MetaType): +class GCPRegion(BaseType): openapi_schema_validator = '''{ "type": "string", "enum": ["asia-east1","asia-east2","asia-northeast1", @@ -101,28 +82,19 @@ class MetaGCPRegion(MetaType): "us-east4","us-west1", "us-west4" ] }''' -def GCPRegion(attr={}): - return type('GCPRegion', (MetaGCPRegion, ), attr) - -class MetaGCPProjectID(MetaType): +class GCPProjectID(BaseType): '''MetaGCPProjectID: GCP project id''' openapi_schema_validator = '''{ "type": "string" }''' -def GCPProjectID(attr={}): - return type('GCPProjectID', (MetaGCPProjectID, ), attr) - # General Types -class MetaLocalPath(MetaType): +class LocalPath(BaseType): #TODO: add restriction to path openapi_schema_validator = '''{ "type": "string" }''' -def LocalPath(attr={}): - return type('LocalPath', (MetaLocalPath, ), attr) - class InconsistentTypeException(Exception): '''InconsistencyTypeException is raised when two types are not consistent''' pass @@ -143,19 +115,15 @@ def _check_valid_dict(payload): return False return True -def _class_to_dict(payload): +def _instance_to_dict(instance): '''serialize_type serializes the type instance into a json string Args: - payload(type): A class that describes a type + instance(BaseType): An instance that describes a type Return: dict ''' - attrs = set([i for i in dir(payload) if not i.startswith('__')]) - json_dict = {payload.__name__: {}} - for attr in attrs: - json_dict[payload.__name__][attr] = getattr(payload, attr) - return json_dict + return {type(instance).__name__: instance.__dict__} def _str_to_dict(payload): import json @@ -194,12 +162,12 @@ def check_types(typeA, typeB): typeA (type/str/dict): it describes a type from the upstream component output typeB (type/str/dict): it describes a type from the downstream component input ''' - if isinstance(typeA, type): - typeA = _class_to_dict(typeA) + if isinstance(typeA, BaseType): + typeA = _instance_to_dict(typeA) elif isinstance(typeA, str): typeA = _str_to_dict(typeA) - if isinstance(typeB, type): - typeB = _class_to_dict(typeB) + if isinstance(typeB, BaseType): + typeB = _instance_to_dict(typeB) elif isinstance(typeB, str): typeB = _str_to_dict(typeB) return _check_dict_types(typeA, typeB) \ No newline at end of file diff --git a/sdk/python/tests/dsl/type_tests.py b/sdk/python/tests/dsl/type_tests.py index d7791264b6b..525b5b5f49e 100644 --- a/sdk/python/tests/dsl/type_tests.py +++ b/sdk/python/tests/dsl/type_tests.py @@ -13,22 +13,18 @@ # limitations under the License. -from kfp.dsl._types import _class_to_dict, _str_to_dict, check_types, GCSPath +from kfp.dsl._types import _instance_to_dict, _str_to_dict, check_types, GCSPath import unittest class TestTypes(unittest.TestCase): def test_class_to_dict(self): """Test _class_to_dict function.""" - gcspath_dict = _class_to_dict(GCSPath({'path_type': 'file', 'file_type': 'csv'})) + gcspath_dict = _instance_to_dict(GCSPath(path_type='file', file_type='csv')) golden_dict = { 'GCSPath': { 'path_type': 'file', 'file_type': 'csv', - 'openapi_schema_validator': '''{ - "type": "string", - "pattern": "^gs://$" - }''' } } self.assertEqual(golden_dict, gcspath_dict) @@ -45,16 +41,15 @@ def test_str_to_dict(self): self.assertEqual(golden_dict, gcspath_dict) gcspath_str = '{"file_type": "csv", "path_type": "file"}' with self.assertRaises(ValueError): - gcspath_dict = _str_to_dict(gcspath_str) + _str_to_dict(gcspath_str) def test_check_types(self): #Core types - typeA = GCSPath({'path_type': 'file', 'file_type': 'csv'}) - typeB = GCSPath({'path_type': 'file', 'file_type': 'csv'}) + typeA = GCSPath(path_type='file', file_type='csv') + typeB = GCSPath(path_type='file', file_type='csv') self.assertTrue(check_types(typeA, typeB)) - typeA = GCSPath({'path_type': 'file', 'file_type': 'tsv'}) - typeB = GCSPath({'path_type': 'file', 'file_type': 'csv'}) - self.assertFalse(check_types(typeA, typeB)) + typeC = GCSPath(path_type='file', file_type='tsv') + self.assertFalse(check_types(typeA, typeC)) # Custom types typeA = { From dcc3baf3e1eaad78835dd4459ceb2a2124eb4e5d Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Fri, 1 Mar 2019 11:12:09 -0800 Subject: [PATCH 04/42] typo fix --- sdk/python/kfp/dsl/_types.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/python/kfp/dsl/_types.py b/sdk/python/kfp/dsl/_types.py index 71993d06512..4516cd70edb 100644 --- a/sdk/python/kfp/dsl/_types.py +++ b/sdk/python/kfp/dsl/_types.py @@ -159,8 +159,8 @@ def check_types(typeA, typeB): For each of the attribute in typeA, there is the same attribute in typeB with the same value. However, typeB could contain more attributes that typeA does not contain. Args: - typeA (type/str/dict): it describes a type from the upstream component output - typeB (type/str/dict): it describes a type from the downstream component input + typeA (BaseType/str/dict): it describes a type from the upstream component output + typeB (BaseType/str/dict): it describes a type from the downstream component input ''' if isinstance(typeA, BaseType): typeA = _instance_to_dict(typeA) From 89c09079110dee76b83a07224f1150ac3a9d930e Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Fri, 1 Mar 2019 11:19:54 -0800 Subject: [PATCH 05/42] add component metadata format --- sdk/python/kfp/dsl/__init__.py | 2 +- sdk/python/kfp/dsl/_container_op.py | 59 +++++++++++++++++- sdk/python/tests/dsl/container_op_tests.py | 70 ++++++++++++++++++++++ 3 files changed, 128 insertions(+), 3 deletions(-) diff --git a/sdk/python/kfp/dsl/__init__.py b/sdk/python/kfp/dsl/__init__.py index a656e53eac0..d2a2912d65f 100644 --- a/sdk/python/kfp/dsl/__init__.py +++ b/sdk/python/kfp/dsl/__init__.py @@ -15,6 +15,6 @@ from ._pipeline_param import PipelineParam from ._pipeline import Pipeline, pipeline, get_pipeline_conf -from ._container_op import ContainerOp +from ._container_op import ContainerOp, ComponentMeta, ParameterMeta, TypeMeta from ._ops_group import OpsGroup, ExitHandler, Condition from ._python_component import python_component diff --git a/sdk/python/kfp/dsl/_container_op.py b/sdk/python/kfp/dsl/_container_op.py index 75a88405784..30a673fb24a 100644 --- a/sdk/python/kfp/dsl/_container_op.py +++ b/sdk/python/kfp/dsl/_container_op.py @@ -17,8 +17,63 @@ from . import _pipeline_param from ._pipeline_param import _extract_pipelineparams import re -from typing import Dict - +from typing import Dict, List +from abc import ABCMeta, abstractmethod + +class BaseMeta(object): + __metaclass__ = ABCMeta + def __init__(self): + pass + + @abstractmethod + def to_dict(self): + pass + + def serialize(self): + import yaml + return yaml.dump(self.to_dict()) + +class TypeMeta(BaseMeta): + def __init__(self, name, properties): + self.name = name + self.properties = properties + + def to_dict(self): + return {self.name: self.properties} + +class ParameterMeta(BaseMeta): + def __init__(self, + name: str = None, + description: str = None, + param_type: TypeMeta = None): + self.name = name + self.description = description + self.param_type = param_type + + def to_dict(self): + return {'name': self.name, + 'description': self.description, + 'type': self.param_type.to_dict()} + +class ComponentMeta(BaseMeta): + def __init__( + self, + name: str = None, + description: str = None, + inputs: List[ParameterMeta] = None, + outputs: List[ParameterMeta] = None + ): + self.name = name + self.description = description + self.inputs = inputs + self.outputs = outputs + + def to_dict(self): + return {'name': self.name, + 'description': self.description, + 'inputs': [ input.to_dict() for input in self.inputs ], + 'outputs': [ output.to_dict() for output in self.outputs ] + } class ContainerOp(object): """Represents an op implemented by a docker container image.""" diff --git a/sdk/python/tests/dsl/container_op_tests.py b/sdk/python/tests/dsl/container_op_tests.py index 3dc407c669f..57712485e14 100644 --- a/sdk/python/tests/dsl/container_op_tests.py +++ b/sdk/python/tests/dsl/container_op_tests.py @@ -14,8 +14,78 @@ from kfp.dsl import Pipeline, PipelineParam, ContainerOp +from kfp.dsl import ComponentMeta, ParameterMeta, TypeMeta import unittest +class TestComponentMeta(unittest.TestCase): + + def test_to_dict(self): + component_meta = ComponentMeta(name='foobar', + description='foobar example', + inputs=[ParameterMeta(name='input1', + description='input1 desc', + param_type=TypeMeta(name='GCSPath', + properties={'bucket_type': 'directory', + 'file_type': 'csv' + } + ) + ), + ParameterMeta(name='input2', + description='input2 desc', + param_type=TypeMeta(name='TFModel', + properties={'input_data': 'tensor', + 'version': '1.8.0' + } + ) + ), + ], + outputs=[ParameterMeta(name='output1', + description='output1 desc', + param_type=TypeMeta(name='Schema', + properties={'file_type': 'tsv' + } + ) + ) + ] + ) + golden_meta = { + 'name': 'foobar', + 'description': 'foobar example', + 'inputs': [ + { + 'name': 'input1', + 'description': 'input1 desc', + 'type': { + 'GCSPath': { + 'bucket_type': 'directory', + 'file_type': 'csv' + } + } + }, + { + 'name': 'input2', + 'description': 'input2 desc', + 'type': { + 'TFModel': { + 'input_data': 'tensor', + 'version': '1.8.0' + } + } + } + ], + 'outputs': [ + { + 'name': 'output1', + 'description': 'output1 desc', + 'type': { + 'Schema': { + 'file_type': 'tsv' + } + } + } + ] + } + self.assertEqual(component_meta.to_dict(), golden_meta) class TestContainerOp(unittest.TestCase): From e55515edcc4e1ba706c561f7e87722cbceaff197 Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Fri, 1 Mar 2019 11:40:25 -0800 Subject: [PATCH 06/42] add a construct for the component decorator --- sdk/python/kfp/dsl/_python_component.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/sdk/python/kfp/dsl/_python_component.py b/sdk/python/kfp/dsl/_python_component.py index bb14f37feae..339fd17dc58 100644 --- a/sdk/python/kfp/dsl/_python_component.py +++ b/sdk/python/kfp/dsl/_python_component.py @@ -12,6 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +from kfp.dsl import ComponentMeta, ParameterMeta, TypeMeta + def python_component(name, description=None, base_image=None, target_component_file: str = None): """Decorator for Python component functions. This decorator adds the metadata to the function object itself. @@ -47,3 +49,25 @@ def _python_component(func): return func return _python_component + +def component(): + """Decorator for component functions that use ContainerOp. + This is useful to enable type checking in the DSL compiler + + Usage: + ```python + @dsl.component + def foobar(model: TFModel(), step: MLStep()): + return dsl.ContainerOp() + """ + def _component(func): + import inspect + fullargspec = inspect.getfullargspec(func) + args = fullargspec.args + annotations = fullargspec.annotations + + # Construct the ComponentMeta + #TODO: convert and record + return func + + return _component \ No newline at end of file From eed41faf61eb9710a3e612cef2a170f4891aa28b Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Fri, 1 Mar 2019 11:42:47 -0800 Subject: [PATCH 07/42] add default values for the meta classes --- sdk/python/kfp/dsl/_container_op.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/sdk/python/kfp/dsl/_container_op.py b/sdk/python/kfp/dsl/_container_op.py index 30a673fb24a..9886017c3af 100644 --- a/sdk/python/kfp/dsl/_container_op.py +++ b/sdk/python/kfp/dsl/_container_op.py @@ -34,7 +34,9 @@ def serialize(self): return yaml.dump(self.to_dict()) class TypeMeta(BaseMeta): - def __init__(self, name, properties): + def __init__(self, + name: str = '', + properties: Dict = {}): self.name = name self.properties = properties @@ -43,9 +45,9 @@ def to_dict(self): class ParameterMeta(BaseMeta): def __init__(self, - name: str = None, - description: str = None, - param_type: TypeMeta = None): + name: str = '', + description: str = '', + param_type: TypeMeta = TypeMeta()): self.name = name self.description = description self.param_type = param_type @@ -58,10 +60,10 @@ def to_dict(self): class ComponentMeta(BaseMeta): def __init__( self, - name: str = None, - description: str = None, - inputs: List[ParameterMeta] = None, - outputs: List[ParameterMeta] = None + name: str = '', + description: str = '', + inputs: List[ParameterMeta] = [], + outputs: List[ParameterMeta] = [] ): self.name = name self.description = description From 18d54592d62c01a416819d985ea8dc115cb1694d Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Fri, 1 Mar 2019 12:25:50 -0800 Subject: [PATCH 08/42] add input/output types to the metadata --- sdk/python/kfp/dsl/_python_component.py | 34 ++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/sdk/python/kfp/dsl/_python_component.py b/sdk/python/kfp/dsl/_python_component.py index 339fd17dc58..a5ccb3f5e5b 100644 --- a/sdk/python/kfp/dsl/_python_component.py +++ b/sdk/python/kfp/dsl/_python_component.py @@ -13,6 +13,8 @@ # limitations under the License. from kfp.dsl import ComponentMeta, ParameterMeta, TypeMeta +from kfp.dsl import BaseType +from ._types import _instance_to_dict,_str_to_dict, _check_valid_dict def python_component(name, description=None, base_image=None, target_component_file: str = None): """Decorator for Python component functions. @@ -50,6 +52,24 @@ def _python_component(func): return _python_component +def _annotation_to_typemeta(annotation): + '''_annotation_to_type_meta converts an annotation to an instance of TypeMeta + Args: + annotation(BaseType/str/dict): input/output annotations + Returns: + TypeMeta + ''' + if isinstance(annotation, BaseType): + arg_type = TypeMeta.from_dict(_instance_to_dict(annotation)) + elif isinstance(annotation, str): + arg_type = TypeMeta.from_dict(_str_to_dict(annotation)) + elif isinstance(annotation, dict): + if not _check_valid_dict(annotation): + raise ValueError('Annotation ' + str(annotation) + ' is not a valid type dictionary.') + arg_type = TypeMeta.from_dict(annotation) + else: + raise ValueError('Annotation ' + str(annotation) + ' is not valid. Use core types, str, or dict.') + def component(): """Decorator for component functions that use ContainerOp. This is useful to enable type checking in the DSL compiler @@ -67,7 +87,19 @@ def _component(func): annotations = fullargspec.annotations # Construct the ComponentMeta - #TODO: convert and record + component_meta = ComponentMeta(name=func.__name__, description='') + # Inputs + for arg in args: + arg_type = TypeMeta() + if arg in annotations: + arg_type = _annotation_to_typemeta(annotations[arg]) + component_meta.inputs.append(ParameterMeta(name=arg, description='', param_type=arg_type)) + # Outputs + for output in annotations['return']: + arg_type = _annotation_to_typemeta(annotations['return'][output]) + component_meta.inputs.append(ParameterMeta(name=output, description='', param_type=arg_type)) + + #TODO: add descriptions to the metadata return func return _component \ No newline at end of file From 9218ffe8eb5d1b6b7823a4d94e395ce4e3174123 Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Fri, 1 Mar 2019 12:36:35 -0800 Subject: [PATCH 09/42] add from_dict in TypeMeta --- sdk/python/kfp/dsl/_container_op.py | 12 ++++++++++++ sdk/python/tests/dsl/container_op_tests.py | 11 +++++++++++ 2 files changed, 23 insertions(+) diff --git a/sdk/python/kfp/dsl/_container_op.py b/sdk/python/kfp/dsl/_container_op.py index 9886017c3af..c8bff86d1ac 100644 --- a/sdk/python/kfp/dsl/_container_op.py +++ b/sdk/python/kfp/dsl/_container_op.py @@ -19,6 +19,7 @@ import re from typing import Dict, List from abc import ABCMeta, abstractmethod +from ._types import _check_valid_dict class BaseMeta(object): __metaclass__ = ABCMeta @@ -43,6 +44,17 @@ def __init__(self, def to_dict(self): return {self.name: self.properties} + @staticmethod + def from_dict(json_dict): + if not _check_valid_dict(json_dict): + raise ValueError(json_dict + ' is not a valid type string') + type_meta = TypeMeta() + type_meta.name, type_meta.properties = list(json_dict.items())[0] + return type_meta + + def __eq__(self, other): + return self.__dict__ == other.__dict__ + class ParameterMeta(BaseMeta): def __init__(self, name: str = '', diff --git a/sdk/python/tests/dsl/container_op_tests.py b/sdk/python/tests/dsl/container_op_tests.py index 57712485e14..82210c96a43 100644 --- a/sdk/python/tests/dsl/container_op_tests.py +++ b/sdk/python/tests/dsl/container_op_tests.py @@ -87,6 +87,17 @@ def test_to_dict(self): } self.assertEqual(component_meta.to_dict(), golden_meta) + def test_type_meta_from_dict(self): + component_dict = { + 'GCSPath': { + 'bucket_type': 'directory', + 'file_type': 'csv' + } + } + golden_type_meta = TypeMeta(name='GCSPath', properties={'bucket_type': 'directory', + 'file_type': 'csv'}) + self.assertEqual(TypeMeta.from_dict(component_dict), golden_type_meta) + class TestContainerOp(unittest.TestCase): def test_basic(self): From aac35b169594afff228381533172c3a7a203b5aa Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Fri, 1 Mar 2019 12:38:02 -0800 Subject: [PATCH 10/42] small fix --- sdk/python/kfp/dsl/_python_component.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sdk/python/kfp/dsl/_python_component.py b/sdk/python/kfp/dsl/_python_component.py index a5ccb3f5e5b..c6ac23eae18 100644 --- a/sdk/python/kfp/dsl/_python_component.py +++ b/sdk/python/kfp/dsl/_python_component.py @@ -13,8 +13,7 @@ # limitations under the License. from kfp.dsl import ComponentMeta, ParameterMeta, TypeMeta -from kfp.dsl import BaseType -from ._types import _instance_to_dict,_str_to_dict, _check_valid_dict +from ._types import _instance_to_dict,_str_to_dict, _check_valid_dict, BaseType def python_component(name, description=None, base_image=None, target_component_file: str = None): """Decorator for Python component functions. From 5ccc85792096a2f8ee3533964c7804f469db0141 Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Fri, 1 Mar 2019 13:49:58 -0800 Subject: [PATCH 11/42] add unit tests --- sdk/python/kfp/dsl/_python_component.py | 16 ++++++++--- sdk/python/tests/dsl/main.py | 2 ++ .../tests/dsl/python_component_tests.py | 28 +++++++++++++++++++ 3 files changed, 42 insertions(+), 4 deletions(-) create mode 100644 sdk/python/tests/dsl/python_component_tests.py diff --git a/sdk/python/kfp/dsl/_python_component.py b/sdk/python/kfp/dsl/_python_component.py index c6ac23eae18..ae505f6d72c 100644 --- a/sdk/python/kfp/dsl/_python_component.py +++ b/sdk/python/kfp/dsl/_python_component.py @@ -68,8 +68,9 @@ def _annotation_to_typemeta(annotation): arg_type = TypeMeta.from_dict(annotation) else: raise ValueError('Annotation ' + str(annotation) + ' is not valid. Use core types, str, or dict.') + return arg_type -def component(): +def component(func): """Decorator for component functions that use ContainerOp. This is useful to enable type checking in the DSL compiler @@ -79,7 +80,7 @@ def component(): def foobar(model: TFModel(), step: MLStep()): return dsl.ContainerOp() """ - def _component(func): + def _component(*args, **kargs): import inspect fullargspec = inspect.getfullargspec(func) args = fullargspec.args @@ -96,9 +97,16 @@ def _component(func): # Outputs for output in annotations['return']: arg_type = _annotation_to_typemeta(annotations['return'][output]) - component_meta.inputs.append(ParameterMeta(name=output, description='', param_type=arg_type)) + component_meta.outputs.append(ParameterMeta(name=output, description='', param_type=arg_type)) #TODO: add descriptions to the metadata - return func + #docstring parser: + # https://github.com/rr-/docstring_parser + # https://github.com/terrencepreilly/darglint/blob/master/darglint/parse.py + + print(component_meta.serialize()) + #TODO: parse the metadata to the ContainerOp. + container_op = func(*args, **kargs) + return container_op return _component \ No newline at end of file diff --git a/sdk/python/tests/dsl/main.py b/sdk/python/tests/dsl/main.py index 6192a944d88..0828389e9d6 100644 --- a/sdk/python/tests/dsl/main.py +++ b/sdk/python/tests/dsl/main.py @@ -21,6 +21,7 @@ import container_op_tests import ops_group_tests import type_tests +import python_component_tests if __name__ == '__main__': suite = unittest.TestSuite() @@ -29,6 +30,7 @@ suite.addTests(unittest.defaultTestLoader.loadTestsFromModule(container_op_tests)) suite.addTests(unittest.defaultTestLoader.loadTestsFromModule(ops_group_tests)) suite.addTests(unittest.defaultTestLoader.loadTestsFromModule(type_tests)) + suite.addTests(unittest.defaultTestLoader.loadTestsFromModule(python_component_tests)) runner = unittest.TextTestRunner() if not runner.run(suite).wasSuccessful(): sys.exit(1) diff --git a/sdk/python/tests/dsl/python_component_tests.py b/sdk/python/tests/dsl/python_component_tests.py new file mode 100644 index 00000000000..a2908dc048b --- /dev/null +++ b/sdk/python/tests/dsl/python_component_tests.py @@ -0,0 +1,28 @@ +# Copyright 2018 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + + +from kfp.dsl._python_component import component +from kfp.dsl._types import GCSPath, Integer +import unittest + +@component +def componentA(a: {'Schema': {'file_type': 'csv'}}, b: '{"number": {"step": "large"}}', c: GCSPath(path_type='file', file_type='tsv')) -> {'model': Integer()}: + return 7 + +class TestPythonComponent(unittest.TestCase): + + def test_component(self): + """Test component decorator.""" + componentA(1,2,3) \ No newline at end of file From 3081d47eea1943d42b7649a7689b7e4f4f9dfe7b Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Fri, 1 Mar 2019 13:56:12 -0800 Subject: [PATCH 12/42] use python struct for the openapi schema --- sdk/python/kfp/dsl/_types.py | 45 ++++++++++++++++++------------------ 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/sdk/python/kfp/dsl/_types.py b/sdk/python/kfp/dsl/_types.py index 4516cd70edb..c560d74387e 100644 --- a/sdk/python/kfp/dsl/_types.py +++ b/sdk/python/kfp/dsl/_types.py @@ -19,41 +19,41 @@ class BaseType: # Primitive Types class Integer(BaseType): - openapi_schema_validator = '''{ + openapi_schema_validator = { "type": "integer" - }''' + } class String(BaseType): - openapi_schema_validator = '''{ + openapi_schema_validator = { "type": "string" - }''' + } class Float(BaseType): - openapi_schema_validator = '''{ + openapi_schema_validator = { "type": "number" - }''' + } class Bool(BaseType): - openapi_schema_validator = '''{ + openapi_schema_validator = { "type": "boolean" - }''' + } class List(BaseType): - openapi_schema_validator = '''{ + openapi_schema_validator = { "type": "array" - }''' + } class Dict(BaseType): - openapi_schema_validator = '''{ + openapi_schema_validator = { "type": "object", - }''' + } # GCP Types class GCSPath(BaseType): - openapi_schema_validator = '''{ + openapi_schema_validator = { "type": "string", "pattern": "^gs://$" - }''' + } def __init__(self, path_type='', file_type=''): ''' @@ -65,14 +65,13 @@ def __init__(self, path_type='', file_type=''): self.file_type = file_type class GCRPath(BaseType): - openapi_schema_validator = '''{ + openapi_schema_validator = { "type": "string", "pattern": "^(us.|eu.|asia.)?gcr\\.io/.*$" - } - }''' + } class GCPRegion(BaseType): - openapi_schema_validator = '''{ + openapi_schema_validator = { "type": "string", "enum": ["asia-east1","asia-east2","asia-northeast1", "asia-south1","asia-southeast1","australia-southeast1", @@ -80,20 +79,20 @@ class GCPRegion(BaseType): "europe-west3","europe-west4","northamerica-northeast1", "southamerica-east1","us-central1","us-east1", "us-east4","us-west1", "us-west4" ] - }''' + } class GCPProjectID(BaseType): '''MetaGCPProjectID: GCP project id''' - openapi_schema_validator = '''{ + openapi_schema_validator = { "type": "string" - }''' + } # General Types class LocalPath(BaseType): #TODO: add restriction to path - openapi_schema_validator = '''{ + openapi_schema_validator = { "type": "string" - }''' + } class InconsistentTypeException(Exception): '''InconsistencyTypeException is raised when two types are not consistent''' From b0a7314502bcb3a35610157e9e34d1c957da5947 Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Fri, 1 Mar 2019 15:01:21 -0800 Subject: [PATCH 13/42] add default in parameter --- sdk/python/kfp/dsl/_container_op.py | 7 +++++-- sdk/python/tests/dsl/container_op_tests.py | 18 ++++++++++++------ 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/sdk/python/kfp/dsl/_container_op.py b/sdk/python/kfp/dsl/_container_op.py index c8bff86d1ac..2a13ef62fc9 100644 --- a/sdk/python/kfp/dsl/_container_op.py +++ b/sdk/python/kfp/dsl/_container_op.py @@ -59,15 +59,18 @@ class ParameterMeta(BaseMeta): def __init__(self, name: str = '', description: str = '', - param_type: TypeMeta = TypeMeta()): + param_type: TypeMeta = TypeMeta(), + default: str = ''): self.name = name self.description = description self.param_type = param_type + self.default = default def to_dict(self): return {'name': self.name, 'description': self.description, - 'type': self.param_type.to_dict()} + 'type': self.param_type.to_dict(), + 'default': self.default} class ComponentMeta(BaseMeta): def __init__( diff --git a/sdk/python/tests/dsl/container_op_tests.py b/sdk/python/tests/dsl/container_op_tests.py index 82210c96a43..836c83da7e2 100644 --- a/sdk/python/tests/dsl/container_op_tests.py +++ b/sdk/python/tests/dsl/container_op_tests.py @@ -28,7 +28,8 @@ def test_to_dict(self): properties={'bucket_type': 'directory', 'file_type': 'csv' } - ) + ), + default='default1' ), ParameterMeta(name='input2', description='input2 desc', @@ -36,7 +37,8 @@ def test_to_dict(self): properties={'input_data': 'tensor', 'version': '1.8.0' } - ) + ), + default='default2' ), ], outputs=[ParameterMeta(name='output1', @@ -44,7 +46,8 @@ def test_to_dict(self): param_type=TypeMeta(name='Schema', properties={'file_type': 'tsv' } - ) + ), + default='default_output1' ) ] ) @@ -60,7 +63,8 @@ def test_to_dict(self): 'bucket_type': 'directory', 'file_type': 'csv' } - } + }, + 'default': 'default1' }, { 'name': 'input2', @@ -70,7 +74,8 @@ def test_to_dict(self): 'input_data': 'tensor', 'version': '1.8.0' } - } + }, + 'default': 'default2' } ], 'outputs': [ @@ -81,7 +86,8 @@ def test_to_dict(self): 'Schema': { 'file_type': 'tsv' } - } + }, + 'default': 'default_output1' } ] } From b69b807e450c416187540340fb24b6cb620ab406 Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Fri, 1 Mar 2019 15:14:10 -0800 Subject: [PATCH 14/42] add default value --- sdk/python/kfp/dsl/_python_component.py | 9 ++++++++- sdk/python/tests/dsl/python_component_tests.py | 2 +- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/sdk/python/kfp/dsl/_python_component.py b/sdk/python/kfp/dsl/_python_component.py index ae505f6d72c..b31440e4da2 100644 --- a/sdk/python/kfp/dsl/_python_component.py +++ b/sdk/python/kfp/dsl/_python_component.py @@ -86,14 +86,21 @@ def _component(*args, **kargs): args = fullargspec.args annotations = fullargspec.annotations + # defaults + arg_defaults = {} + if fullargspec.defaults: + for arg, default in zip(reversed(fullargspec.args), reversed(fullargspec.defaults)): + arg_defaults[arg] = default + # Construct the ComponentMeta component_meta = ComponentMeta(name=func.__name__, description='') # Inputs for arg in args: arg_type = TypeMeta() + arg_default = arg_defaults[arg] if arg in arg_defaults else '' if arg in annotations: arg_type = _annotation_to_typemeta(annotations[arg]) - component_meta.inputs.append(ParameterMeta(name=arg, description='', param_type=arg_type)) + component_meta.inputs.append(ParameterMeta(name=arg, description='', param_type=arg_type), default=arg_default) # Outputs for output in annotations['return']: arg_type = _annotation_to_typemeta(annotations['return'][output]) diff --git a/sdk/python/tests/dsl/python_component_tests.py b/sdk/python/tests/dsl/python_component_tests.py index a2908dc048b..8519e6f28ce 100644 --- a/sdk/python/tests/dsl/python_component_tests.py +++ b/sdk/python/tests/dsl/python_component_tests.py @@ -18,7 +18,7 @@ import unittest @component -def componentA(a: {'Schema': {'file_type': 'csv'}}, b: '{"number": {"step": "large"}}', c: GCSPath(path_type='file', file_type='tsv')) -> {'model': Integer()}: +def componentA(a: {'Schema': {'file_type': 'csv'}}, b: '{"number": {"step": "large"}}' = 12, c: GCSPath(path_type='file', file_type='tsv') = 13) -> {'model': Integer()}: return 7 class TestPythonComponent(unittest.TestCase): From 57232e4ddacd368515a1d3e49c6bbae6ff57d611 Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Fri, 1 Mar 2019 15:15:23 -0800 Subject: [PATCH 15/42] remove the str restriction for the param default --- sdk/python/kfp/dsl/_container_op.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/python/kfp/dsl/_container_op.py b/sdk/python/kfp/dsl/_container_op.py index 2a13ef62fc9..55e1266ee52 100644 --- a/sdk/python/kfp/dsl/_container_op.py +++ b/sdk/python/kfp/dsl/_container_op.py @@ -60,7 +60,7 @@ def __init__(self, name: str = '', description: str = '', param_type: TypeMeta = TypeMeta(), - default: str = ''): + default = ''): self.name = name self.description = description self.param_type = param_type From 04b86f2ba54c62e510db14e4337ec4ea881dcfa3 Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Fri, 1 Mar 2019 15:18:36 -0800 Subject: [PATCH 16/42] bug fix --- sdk/python/kfp/dsl/_python_component.py | 2 +- sdk/python/tests/dsl/python_component_tests.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/python/kfp/dsl/_python_component.py b/sdk/python/kfp/dsl/_python_component.py index b31440e4da2..35867d5109d 100644 --- a/sdk/python/kfp/dsl/_python_component.py +++ b/sdk/python/kfp/dsl/_python_component.py @@ -100,7 +100,7 @@ def _component(*args, **kargs): arg_default = arg_defaults[arg] if arg in arg_defaults else '' if arg in annotations: arg_type = _annotation_to_typemeta(annotations[arg]) - component_meta.inputs.append(ParameterMeta(name=arg, description='', param_type=arg_type), default=arg_default) + component_meta.inputs.append(ParameterMeta(name=arg, description='', param_type=arg_type, default=arg_default)) # Outputs for output in annotations['return']: arg_type = _annotation_to_typemeta(annotations['return'][output]) diff --git a/sdk/python/tests/dsl/python_component_tests.py b/sdk/python/tests/dsl/python_component_tests.py index 8519e6f28ce..b57ad311f09 100644 --- a/sdk/python/tests/dsl/python_component_tests.py +++ b/sdk/python/tests/dsl/python_component_tests.py @@ -18,7 +18,7 @@ import unittest @component -def componentA(a: {'Schema': {'file_type': 'csv'}}, b: '{"number": {"step": "large"}}' = 12, c: GCSPath(path_type='file', file_type='tsv') = 13) -> {'model': Integer()}: +def componentA(a: {'Schema': {'file_type': 'csv'}}, b: '{"number": {"step": "large"}}' = 12, c: GCSPath(path_type='file', file_type='tsv') = 'gs://hello/world') -> {'model': Integer()}: return 7 class TestPythonComponent(unittest.TestCase): From 622ad68cb8ae4dee70da95bab4cceb841d746b48 Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Fri, 1 Mar 2019 15:27:42 -0800 Subject: [PATCH 17/42] add pipelinemeta --- sdk/python/kfp/dsl/_container_op.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/sdk/python/kfp/dsl/_container_op.py b/sdk/python/kfp/dsl/_container_op.py index 55e1266ee52..9b15c7127a4 100644 --- a/sdk/python/kfp/dsl/_container_op.py +++ b/sdk/python/kfp/dsl/_container_op.py @@ -92,6 +92,25 @@ def to_dict(self): 'outputs': [ output.to_dict() for output in self.outputs ] } +# Add a pipeline level metadata calss here. +# If one day we combine the component and pipeline yaml, ComponentMeta and PipelineMeta will become one, too. +class PipelineMeta(BaseMeta): + def __init__( + self, + name: str = '', + description: str = '', + inputs: List[ParameterMeta] = [] + ): + self.name = name + self.description = description + self.inputs = inputs + + def to_dict(self): + return {'name': self.name, + 'description': self.description, + 'inputs': [ input.to_dict() for input in self.inputs ] + } + class ContainerOp(object): """Represents an op implemented by a docker container image.""" From ac49e68ff388fa17d1f68ed43519d61cfd874f4a Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Fri, 1 Mar 2019 16:54:45 -0800 Subject: [PATCH 18/42] add pipeline metadata --- sdk/python/kfp/dsl/__init__.py | 2 +- sdk/python/kfp/dsl/_container_op.py | 21 ++++++++++++++++++++- sdk/python/kfp/dsl/_pipeline.py | 22 ++++++++++++++++++++++ sdk/python/kfp/dsl/_python_component.py | 21 +-------------------- sdk/python/tests/dsl/pipeline_tests.py | 12 ++++++++++++ 5 files changed, 56 insertions(+), 22 deletions(-) diff --git a/sdk/python/kfp/dsl/__init__.py b/sdk/python/kfp/dsl/__init__.py index d2a2912d65f..01c64ef17c7 100644 --- a/sdk/python/kfp/dsl/__init__.py +++ b/sdk/python/kfp/dsl/__init__.py @@ -15,6 +15,6 @@ from ._pipeline_param import PipelineParam from ._pipeline import Pipeline, pipeline, get_pipeline_conf -from ._container_op import ContainerOp, ComponentMeta, ParameterMeta, TypeMeta +from ._container_op import ContainerOp, PipelineMeta, ComponentMeta, ParameterMeta, TypeMeta from ._ops_group import OpsGroup, ExitHandler, Condition from ._python_component import python_component diff --git a/sdk/python/kfp/dsl/_container_op.py b/sdk/python/kfp/dsl/_container_op.py index 9b15c7127a4..31f64b6fb3f 100644 --- a/sdk/python/kfp/dsl/_container_op.py +++ b/sdk/python/kfp/dsl/_container_op.py @@ -19,7 +19,7 @@ import re from typing import Dict, List from abc import ABCMeta, abstractmethod -from ._types import _check_valid_dict +from ._types import _check_valid_dict, BaseType, _instance_to_dict, _str_to_dict class BaseMeta(object): __metaclass__ = ABCMeta @@ -111,6 +111,25 @@ def to_dict(self): 'inputs': [ input.to_dict() for input in self.inputs ] } +def _annotation_to_typemeta(annotation): + '''_annotation_to_type_meta converts an annotation to an instance of TypeMeta + Args: + annotation(BaseType/str/dict): input/output annotations + Returns: + TypeMeta + ''' + if isinstance(annotation, BaseType): + arg_type = TypeMeta.from_dict(_instance_to_dict(annotation)) + elif isinstance(annotation, str): + arg_type = TypeMeta.from_dict(_str_to_dict(annotation)) + elif isinstance(annotation, dict): + if not _check_valid_dict(annotation): + raise ValueError('Annotation ' + str(annotation) + ' is not a valid type dictionary.') + arg_type = TypeMeta.from_dict(annotation) + else: + raise ValueError('Annotation ' + str(annotation) + ' is not valid. Use core types, str, or dict.') + return arg_type + class ContainerOp(object): """Represents an op implemented by a docker container image.""" diff --git a/sdk/python/kfp/dsl/_pipeline.py b/sdk/python/kfp/dsl/_pipeline.py index e3ad49a13f6..bc32da91788 100644 --- a/sdk/python/kfp/dsl/_pipeline.py +++ b/sdk/python/kfp/dsl/_pipeline.py @@ -14,6 +14,7 @@ from . import _container_op +from ._container_op import PipelineMeta, ParameterMeta, TypeMeta, _annotation_to_typemeta from . import _ops_group import sys @@ -32,6 +33,27 @@ def my_pipeline(a: PipelineParam, b: PipelineParam): ``` """ def _pipeline(func): + import inspect + fullargspec = inspect.getfullargspec(func) + args = fullargspec.args + annotations = fullargspec.annotations + + # Construct the PipelineMeta + pipeline_meta = PipelineMeta(name=func.__name__, description='') + # Inputs + for arg in args: + arg_type = TypeMeta() + if arg in annotations: + arg_type = _annotation_to_typemeta(annotations[arg]) + pipeline_meta.inputs.append(ParameterMeta(name=arg, description='', param_type=arg_type)) + + #TODO: add descriptions to the metadata + #docstring parser: + # https://github.com/rr-/docstring_parser + # https://github.com/terrencepreilly/darglint/blob/master/darglint/parse.py + print(pipeline_meta.serialize()) + #TODO: parse the metadata to the Pipeline. + Pipeline.add_pipeline(name, description, func) return func diff --git a/sdk/python/kfp/dsl/_python_component.py b/sdk/python/kfp/dsl/_python_component.py index 35867d5109d..970101003d0 100644 --- a/sdk/python/kfp/dsl/_python_component.py +++ b/sdk/python/kfp/dsl/_python_component.py @@ -13,7 +13,7 @@ # limitations under the License. from kfp.dsl import ComponentMeta, ParameterMeta, TypeMeta -from ._types import _instance_to_dict,_str_to_dict, _check_valid_dict, BaseType +from ._container_op import _annotation_to_typemeta def python_component(name, description=None, base_image=None, target_component_file: str = None): """Decorator for Python component functions. @@ -51,25 +51,6 @@ def _python_component(func): return _python_component -def _annotation_to_typemeta(annotation): - '''_annotation_to_type_meta converts an annotation to an instance of TypeMeta - Args: - annotation(BaseType/str/dict): input/output annotations - Returns: - TypeMeta - ''' - if isinstance(annotation, BaseType): - arg_type = TypeMeta.from_dict(_instance_to_dict(annotation)) - elif isinstance(annotation, str): - arg_type = TypeMeta.from_dict(_str_to_dict(annotation)) - elif isinstance(annotation, dict): - if not _check_valid_dict(annotation): - raise ValueError('Annotation ' + str(annotation) + ' is not a valid type dictionary.') - arg_type = TypeMeta.from_dict(annotation) - else: - raise ValueError('Annotation ' + str(annotation) + ' is not valid. Use core types, str, or dict.') - return arg_type - def component(func): """Decorator for component functions that use ContainerOp. This is useful to enable type checking in the DSL compiler diff --git a/sdk/python/tests/dsl/pipeline_tests.py b/sdk/python/tests/dsl/pipeline_tests.py index 7c57e4c01b4..5570ccb6f94 100644 --- a/sdk/python/tests/dsl/pipeline_tests.py +++ b/sdk/python/tests/dsl/pipeline_tests.py @@ -14,6 +14,7 @@ from kfp.dsl import Pipeline, PipelineParam, ContainerOp, pipeline +from kfp.dsl._types import GCSPath, Integer import unittest @@ -55,3 +56,14 @@ def my_pipeline2(): self.assertEqual(('p1', 'description1'), Pipeline.get_pipeline_functions()[my_pipeline1]) self.assertEqual(('p2', 'description2'), Pipeline.get_pipeline_functions()[my_pipeline2]) + + def test_decorator_metadata(self): + """Test @pipeline decorator with metadata.""" + @pipeline( + name='p1', + description='description1' + ) + def my_pipeline1(a: {'Schema': {'file_type': 'csv'}}='good', b: Integer()=12): + pass + + self.assertEqual(('p1', 'description1'), Pipeline.get_pipeline_functions()[my_pipeline1]) \ No newline at end of file From e05e5e92242fa91ed4f238a772d783fcd9683dee Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Fri, 1 Mar 2019 17:24:51 -0800 Subject: [PATCH 19/42] ignore annotation if it is not str/BaseType/dict --- sdk/python/kfp/dsl/_container_op.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/python/kfp/dsl/_container_op.py b/sdk/python/kfp/dsl/_container_op.py index 31f64b6fb3f..2b883ec127d 100644 --- a/sdk/python/kfp/dsl/_container_op.py +++ b/sdk/python/kfp/dsl/_container_op.py @@ -127,7 +127,7 @@ def _annotation_to_typemeta(annotation): raise ValueError('Annotation ' + str(annotation) + ' is not a valid type dictionary.') arg_type = TypeMeta.from_dict(annotation) else: - raise ValueError('Annotation ' + str(annotation) + ' is not valid. Use core types, str, or dict.') + return TypeMeta() return arg_type class ContainerOp(object): From 10ceb0994d5923c638385064979599a1e71d1d50 Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Mon, 4 Mar 2019 13:47:49 -0800 Subject: [PATCH 20/42] update param name in the check_type functions remove schema validators for GCRPath, and adjust for GCRPath, GCSPath change _check_valid_dict to _check_valid_type_dict to avoid confusion fix typo in the comments adjust function order for readability --- sdk/python/kfp/dsl/_types.py | 82 +++++++++++++++++------------------- 1 file changed, 38 insertions(+), 44 deletions(-) diff --git a/sdk/python/kfp/dsl/_types.py b/sdk/python/kfp/dsl/_types.py index c560d74387e..c0dacca71dd 100644 --- a/sdk/python/kfp/dsl/_types.py +++ b/sdk/python/kfp/dsl/_types.py @@ -52,7 +52,7 @@ class Dict(BaseType): class GCSPath(BaseType): openapi_schema_validator = { "type": "string", - "pattern": "^gs://$" + "pattern": "^gs://.*$" } def __init__(self, path_type='', file_type=''): @@ -67,18 +67,12 @@ def __init__(self, path_type='', file_type=''): class GCRPath(BaseType): openapi_schema_validator = { "type": "string", - "pattern": "^(us.|eu.|asia.)?gcr\\.io/.*$" + "pattern": "^.*gcr\\.io/.*$" } class GCPRegion(BaseType): openapi_schema_validator = { - "type": "string", - "enum": ["asia-east1","asia-east2","asia-northeast1", - "asia-south1","asia-southeast1","australia-southeast1", - "europe-north1","europe-west1","europe-west2", - "europe-west3","europe-west4","northamerica-northeast1", - "southamerica-east1","us-central1","us-east1", - "us-east4","us-west1", "us-west4" ] + "type": "string" } class GCPProjectID(BaseType): @@ -98,8 +92,26 @@ class InconsistentTypeException(Exception): '''InconsistencyTypeException is raised when two types are not consistent''' pass -def _check_valid_dict(payload): - '''_check_valid_dict_type checks whether a dict is a correct serialization of a type +def check_types(checked_type, expected_type): + '''check_types checks the type consistency. + For each of the attribute in checked_type, there is the same attribute in expected_type with the same value. + However, expected_type could contain more attributes that checked_type does not contain. + Args: + checked_type (BaseType/str/dict): it describes a type from the upstream component output + expected_type (BaseType/str/dict): it describes a type from the downstream component input + ''' + if isinstance(checked_type, BaseType): + checked_type = _instance_to_dict(checked_type) + elif isinstance(checked_type, str): + checked_type = _str_to_dict(checked_type) + if isinstance(expected_type, BaseType): + expected_type = _instance_to_dict(expected_type) + elif isinstance(expected_type, str): + expected_type = _str_to_dict(expected_type) + return _check_dict_types(checked_type, expected_type) + +def _check_valid_type_dict(payload): + '''_check_valid_type_dict checks whether a dict is a correct serialization of a type Args: payload(dict) ''' @@ -115,7 +127,7 @@ def _check_valid_dict(payload): return True def _instance_to_dict(instance): - '''serialize_type serializes the type instance into a json string + '''_instance_to_dict serializes the type instance into a python dictionary Args: instance(BaseType): An instance that describes a type @@ -127,46 +139,28 @@ def _instance_to_dict(instance): def _str_to_dict(payload): import json json_dict = json.loads(payload) - if not _check_valid_dict(json_dict): + if not _check_valid_type_dict(json_dict): raise ValueError(payload + ' is not a valid type string') return json_dict -def _check_dict_types(typeA, typeB): +def _check_dict_types(checked_type, expected_type): '''_check_type_types checks the type consistency. Args: - typeA (dict): A dict that describes a type from the upstream component output - typeB (dict): A dict that describes a type from the downstream component input + checked_type (dict): A dict that describes a type from the upstream component output + expected_type (dict): A dict that describes a type from the downstream component input ''' - typeA_name,_ = list(typeA.items())[0] - typeB_name,_ = list(typeB.items())[0] - if typeA_name != typeB_name: + checked_type_name,_ = list(checked_type.items())[0] + expected_type_name,_ = list(expected_type.items())[0] + if checked_type_name != expected_type_name: return False - type_name = typeA_name - for type_property in typeA[type_name]: - if type_property not in typeB[type_name]: + type_name = checked_type_name + for type_property in checked_type[type_name]: + if type_property not in expected_type[type_name]: print(type_name + ' has a property ' + str(type_property) + ' that the latter does not.') return False - if typeA[type_name][type_property] != typeB[type_name][type_property]: + if checked_type[type_name][type_property] != expected_type[type_name][type_property]: print(type_name + ' has a property ' + str(type_property) + ' with value: ' + - str(typeA[type_name][type_property]) + ' and ' + - str(typeB[type_name][type_property])) + str(checked_type[type_name][type_property]) + ' and ' + + str(expected_type[type_name][type_property])) return False - return True - -def check_types(typeA, typeB): - '''check_types checks the type consistency. - For each of the attribute in typeA, there is the same attribute in typeB with the same value. - However, typeB could contain more attributes that typeA does not contain. - Args: - typeA (BaseType/str/dict): it describes a type from the upstream component output - typeB (BaseType/str/dict): it describes a type from the downstream component input - ''' - if isinstance(typeA, BaseType): - typeA = _instance_to_dict(typeA) - elif isinstance(typeA, str): - typeA = _str_to_dict(typeA) - if isinstance(typeB, BaseType): - typeB = _instance_to_dict(typeB) - elif isinstance(typeB, str): - typeB = _str_to_dict(typeB) - return _check_dict_types(typeA, typeB) \ No newline at end of file + return True \ No newline at end of file From 987e22f078955827a1411796f9f94053bbac5a52 Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Mon, 4 Mar 2019 14:28:03 -0800 Subject: [PATCH 21/42] remove default values for non-primitive types in the function signature update the _check_valid_type_dict name --- sdk/python/kfp/dsl/_container_op.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/sdk/python/kfp/dsl/_container_op.py b/sdk/python/kfp/dsl/_container_op.py index 9b15c7127a4..7d92241c533 100644 --- a/sdk/python/kfp/dsl/_container_op.py +++ b/sdk/python/kfp/dsl/_container_op.py @@ -19,7 +19,7 @@ import re from typing import Dict, List from abc import ABCMeta, abstractmethod -from ._types import _check_valid_dict +from ._types import _check_valid_type_dict class BaseMeta(object): __metaclass__ = ABCMeta @@ -37,16 +37,16 @@ def serialize(self): class TypeMeta(BaseMeta): def __init__(self, name: str = '', - properties: Dict = {}): + properties: Dict = None): self.name = name - self.properties = properties + self.properties = {} if properties is None else properties def to_dict(self): return {self.name: self.properties} @staticmethod def from_dict(json_dict): - if not _check_valid_dict(json_dict): + if not _check_valid_type_dict(json_dict): raise ValueError(json_dict + ' is not a valid type string') type_meta = TypeMeta() type_meta.name, type_meta.properties = list(json_dict.items())[0] @@ -59,11 +59,11 @@ class ParameterMeta(BaseMeta): def __init__(self, name: str = '', description: str = '', - param_type: TypeMeta = TypeMeta(), + param_type: TypeMeta = None, default = ''): self.name = name self.description = description - self.param_type = param_type + self.param_type = TypeMeta() if param_type is None else param_type self.default = default def to_dict(self): @@ -77,13 +77,13 @@ def __init__( self, name: str = '', description: str = '', - inputs: List[ParameterMeta] = [], - outputs: List[ParameterMeta] = [] + inputs: List[ParameterMeta] = None, + outputs: List[ParameterMeta] = None ): self.name = name self.description = description - self.inputs = inputs - self.outputs = outputs + self.inputs = [] if inputs is None else inputs + self.outputs = [] if outputs is None else outputs def to_dict(self): return {'name': self.name, @@ -99,11 +99,11 @@ def __init__( self, name: str = '', description: str = '', - inputs: List[ParameterMeta] = [] + inputs: List[ParameterMeta] = None ): self.name = name self.description = description - self.inputs = inputs + self.inputs = [] if inputs is None else inputs def to_dict(self): return {'name': self.name, From 1c4a96ed4c564f8f1f392ea3f4b221f8dcad1748 Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Mon, 4 Mar 2019 15:15:25 -0800 Subject: [PATCH 22/42] pass metadata from component decorator and task factory to containerOp --- sdk/python/kfp/components/_dsl_bridge.py | 16 +++++++++++++++- sdk/python/kfp/dsl/_container_op.py | 10 ++++++++++ sdk/python/kfp/dsl/_python_component.py | 3 +-- 3 files changed, 26 insertions(+), 3 deletions(-) diff --git a/sdk/python/kfp/components/_dsl_bridge.py b/sdk/python/kfp/components/_dsl_bridge.py index 182b758e970..8e7e588b549 100644 --- a/sdk/python/kfp/components/_dsl_bridge.py +++ b/sdk/python/kfp/components/_dsl_bridge.py @@ -15,6 +15,8 @@ from collections import OrderedDict from ._structures import ConcatPlaceholder, IfPlaceholder, InputValuePlaceholder, InputPathPlaceholder, IsPresentPlaceholder, OutputPathPlaceholder, TaskSpec from ._components import _generate_output_file_name, _default_component_name +from kfp.dsl import ComponentMeta, ParameterMeta, TypeMeta +from kfp.dsl._container_op import _annotation_to_typemeta def create_container_op_from_task(task_spec: TaskSpec): @@ -123,12 +125,13 @@ def expand_argument_list(argument_list): command=expanded_command, arguments=expanded_args, output_paths=output_paths, + component_spec=component_spec ) _dummy_pipeline=None -def _create_container_op_from_resolved_task(name:str, container_image:str, command=None, arguments=None, output_paths=None): +def _create_container_op_from_resolved_task(name:str, container_image:str, command=None, arguments=None, output_paths=None, component_spec=None): from .. import dsl global _dummy_pipeline need_dummy = dsl.Pipeline._default_pipeline is None @@ -148,6 +151,16 @@ def _create_container_op_from_resolved_task(name:str, container_image:str, comma output_paths_for_container_op = {output_name_to_kubernetes[name]: path for name, path in output_paths.items()} + # Construct the ComponentMeta + component_meta = ComponentMeta(name=component_spec.name, description=component_spec.description) + # Inputs + if component_spec.inputs is not None: + for input in component_spec.inputs: + component_meta.inputs.append(ParameterMeta(name=input.name, description=input.description, type=_annotation_to_typemeta(input.type), default=input.default)) + if component_spec.outputs is not None: + for output in component_spec.outputs: + component_meta.outputs.append(ParameterMeta(name=output.name, description=output.description, type=_annotation_to_typemeta(input.type))) + task = dsl.ContainerOp( name=name, image=container_image, @@ -155,6 +168,7 @@ def _create_container_op_from_resolved_task(name:str, container_image:str, comma arguments=arguments, file_outputs=output_paths_for_container_op, ) + task._set_metadata(component_meta) if need_dummy: _dummy_pipeline.__exit__() diff --git a/sdk/python/kfp/dsl/_container_op.py b/sdk/python/kfp/dsl/_container_op.py index edba7ffee2b..06ab2d4bce0 100644 --- a/sdk/python/kfp/dsl/_container_op.py +++ b/sdk/python/kfp/dsl/_container_op.py @@ -174,6 +174,7 @@ def __init__(self, name: str, image: str, command: str=None, arguments: str=None self.pod_annotations = {} self.pod_labels = {} self.num_retries = 0 + self._metadata = ComponentMeta() self.argument_inputs = _extract_pipelineparams([str(arg) for arg in (command or []) + (arguments or [])]) @@ -406,3 +407,12 @@ def set_retry(self, num_retries: int): def __repr__(self): return str({self.__class__.__name__: self.__dict__}) + + def _set_metadata(self, metadata): + '''_set_metadata passes the containerop the metadata information + Args: + metadata (ComponentMeta): component metadata + ''' + if not isinstance(metadata, ComponentMeta): + raise ValueError('_set_medata is expecting ComponentMeta.') + self._metadata = metadata \ No newline at end of file diff --git a/sdk/python/kfp/dsl/_python_component.py b/sdk/python/kfp/dsl/_python_component.py index 970101003d0..bad86a7dfa6 100644 --- a/sdk/python/kfp/dsl/_python_component.py +++ b/sdk/python/kfp/dsl/_python_component.py @@ -92,9 +92,8 @@ def _component(*args, **kargs): # https://github.com/rr-/docstring_parser # https://github.com/terrencepreilly/darglint/blob/master/darglint/parse.py - print(component_meta.serialize()) - #TODO: parse the metadata to the ContainerOp. container_op = func(*args, **kargs) + container_op._set_metadata(component_meta) return container_op return _component \ No newline at end of file From f14036166355402523127c9af34df276386e722e Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Mon, 4 Mar 2019 15:25:49 -0800 Subject: [PATCH 23/42] pass pipeline metadata to Pipeline --- sdk/python/kfp/compiler/compiler.py | 2 +- sdk/python/kfp/dsl/_pipeline.py | 20 ++++++++++++++------ 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/sdk/python/kfp/compiler/compiler.py b/sdk/python/kfp/compiler/compiler.py index da82504d99f..914dedfe448 100644 --- a/sdk/python/kfp/compiler/compiler.py +++ b/sdk/python/kfp/compiler/compiler.py @@ -516,7 +516,7 @@ def _compile(self, pipeline_func): if pipeline_func not in registered_pipeline_functions: raise ValueError('Please use a function with @dsl.pipeline decorator.') - pipeline_name, _ = dsl.Pipeline.get_pipeline_functions()[pipeline_func] + pipeline_name = dsl.Pipeline.get_pipeline_functions()[pipeline_func].name pipeline_name = K8sHelper.sanitize_k8s_name(pipeline_name) # Create the arg list with no default values and call pipeline function. diff --git a/sdk/python/kfp/dsl/_pipeline.py b/sdk/python/kfp/dsl/_pipeline.py index bc32da91788..ab7d04f049b 100644 --- a/sdk/python/kfp/dsl/_pipeline.py +++ b/sdk/python/kfp/dsl/_pipeline.py @@ -39,7 +39,7 @@ def _pipeline(func): annotations = fullargspec.annotations # Construct the PipelineMeta - pipeline_meta = PipelineMeta(name=func.__name__, description='') + pipeline_meta = PipelineMeta(name=name, description=description) # Inputs for arg in args: arg_type = TypeMeta() @@ -51,10 +51,8 @@ def _pipeline(func): #docstring parser: # https://github.com/rr-/docstring_parser # https://github.com/terrencepreilly/darglint/blob/master/darglint/parse.py - print(pipeline_meta.serialize()) - #TODO: parse the metadata to the Pipeline. - Pipeline.add_pipeline(name, description, func) + Pipeline.add_pipeline(pipeline_meta, func) return func return _pipeline @@ -115,9 +113,9 @@ def get_pipeline_functions(): return Pipeline._pipeline_functions @staticmethod - def add_pipeline(name, description, func): + def add_pipeline(pipeline_meta, func): """Add a pipeline function (decorated with @pipeline).""" - Pipeline._pipeline_functions[func] = (name, description) + Pipeline._pipeline_functions[func] = pipeline_meta def __init__(self, name: str): """Create a new instance of Pipeline. @@ -131,6 +129,7 @@ def __init__(self, name: str): self.groups = [_ops_group.OpsGroup('pipeline', name=name)] self.group_id = 0 self.conf = PipelineConf() + self._metadata = PipelineMeta() def __enter__(self): if Pipeline._default_pipeline: @@ -185,4 +184,13 @@ def get_next_group_id(self): self.group_id += 1 return self.group_id + def _set_metadata(self, metadata): + '''_set_metadata passes the containerop the metadata information + Args: + metadata (ComponentMeta): component metadata + ''' + if not isinstance(metadata, PipelineMeta): + raise ValueError('_set_medata is expecting PipelineMeta.') + self._metadata = metadata + From 4e577e3ab6caf6192f2a0f07fa9411c1daf5cfcf Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Mon, 4 Mar 2019 17:57:56 -0800 Subject: [PATCH 24/42] fix unit test --- sdk/python/tests/dsl/pipeline_tests.py | 11 +++++--- .../tests/dsl/python_component_tests.py | 25 ++++++++++++++----- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/sdk/python/tests/dsl/pipeline_tests.py b/sdk/python/tests/dsl/pipeline_tests.py index 5570ccb6f94..a2052d1ced2 100644 --- a/sdk/python/tests/dsl/pipeline_tests.py +++ b/sdk/python/tests/dsl/pipeline_tests.py @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. - +import kfp from kfp.dsl import Pipeline, PipelineParam, ContainerOp, pipeline from kfp.dsl._types import GCSPath, Integer import unittest @@ -54,8 +54,10 @@ def my_pipeline1(): def my_pipeline2(): pass - self.assertEqual(('p1', 'description1'), Pipeline.get_pipeline_functions()[my_pipeline1]) - self.assertEqual(('p2', 'description2'), Pipeline.get_pipeline_functions()[my_pipeline2]) + self.assertEqual('p1', Pipeline.get_pipeline_functions()[my_pipeline1].name) + self.assertEqual('description1', Pipeline.get_pipeline_functions()[my_pipeline1].description) + self.assertEqual('p2', Pipeline.get_pipeline_functions()[my_pipeline2].name) + self.assertEqual('description2', Pipeline.get_pipeline_functions()[my_pipeline2].description) def test_decorator_metadata(self): """Test @pipeline decorator with metadata.""" @@ -66,4 +68,5 @@ def test_decorator_metadata(self): def my_pipeline1(a: {'Schema': {'file_type': 'csv'}}='good', b: Integer()=12): pass - self.assertEqual(('p1', 'description1'), Pipeline.get_pipeline_functions()[my_pipeline1]) \ No newline at end of file + self.assertEqual('p1', Pipeline.get_pipeline_functions()[my_pipeline1].name) + self.assertEqual('description1', Pipeline.get_pipeline_functions()[my_pipeline1].description) \ No newline at end of file diff --git a/sdk/python/tests/dsl/python_component_tests.py b/sdk/python/tests/dsl/python_component_tests.py index b57ad311f09..bf8864ab8bc 100644 --- a/sdk/python/tests/dsl/python_component_tests.py +++ b/sdk/python/tests/dsl/python_component_tests.py @@ -14,15 +14,28 @@ from kfp.dsl._python_component import component +from kfp.dsl._container_op import ComponentMeta, ParameterMeta, TypeMeta from kfp.dsl._types import GCSPath, Integer import unittest +import mock -@component -def componentA(a: {'Schema': {'file_type': 'csv'}}, b: '{"number": {"step": "large"}}' = 12, c: GCSPath(path_type='file', file_type='tsv') = 'gs://hello/world') -> {'model': Integer()}: - return 7 - +@mock.patch('kfp.dsl.ContainerOp') class TestPythonComponent(unittest.TestCase): - def test_component(self): + def test_component(self, ContainerOp): """Test component decorator.""" - componentA(1,2,3) \ No newline at end of file + @component + def componentA(a: {'Schema': {'file_type': 'csv'}}, b: '{"number": {"step": "large"}}' = 12, c: GCSPath(path_type='file', file_type='tsv') = 'gs://hello/world') -> {'model': Integer()}: + return ContainerOp() + + ContainerOp()._set_metadata.return_value = None + componentA(1,2,3) + + golden_meta = ComponentMeta(name='componentA', description='') + golden_meta.inputs.append(ParameterMeta(name='a', description='', param_type=TypeMeta(name='Schema', properties={'file_type': 'csv'}))) + golden_meta.inputs.append(ParameterMeta(name='b', description='', param_type=TypeMeta(name='number', properties={'step': 'large'}), default=12)) + golden_meta.inputs.append(ParameterMeta(name='c', description='', param_type=TypeMeta(name='GCSPath', properties={'path_type':'file', 'file_type': 'tsv'}), default='gs://hello/world')) + golden_meta.inputs.append(ParameterMeta(name='model', description='', param_type=TypeMeta(name='Integer'))) + + ContainerOp()._set_metadata.assert_called_once_with(golden_meta) + From 3d625b3948e717642fbc6da5dab49da3fe98e332 Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Tue, 5 Mar 2019 11:03:19 -0800 Subject: [PATCH 25/42] typo in the comments --- sdk/python/kfp/dsl/_types.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/python/kfp/dsl/_types.py b/sdk/python/kfp/dsl/_types.py index c0dacca71dd..432eb49250e 100644 --- a/sdk/python/kfp/dsl/_types.py +++ b/sdk/python/kfp/dsl/_types.py @@ -144,7 +144,7 @@ def _str_to_dict(payload): return json_dict def _check_dict_types(checked_type, expected_type): - '''_check_type_types checks the type consistency. + '''_check_dict_types checks the type consistency. Args: checked_type (dict): A dict that describes a type from the upstream component output expected_type (dict): A dict that describes a type from the downstream component input From 9c233b364e5ac6247c922905bde139d2cb4a7c31 Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Tue, 5 Mar 2019 11:13:54 -0800 Subject: [PATCH 26/42] move the metadata classes to a separate module --- sdk/python/kfp/dsl/__init__.py | 3 +- sdk/python/kfp/dsl/_container_op.py | 94 +----------------- sdk/python/kfp/dsl/_metadata.py | 107 +++++++++++++++++++++ sdk/python/tests/dsl/container_op_tests.py | 88 ----------------- sdk/python/tests/dsl/main.py | 2 + sdk/python/tests/dsl/metadata_tests.py | 103 ++++++++++++++++++++ 6 files changed, 215 insertions(+), 182 deletions(-) create mode 100644 sdk/python/kfp/dsl/_metadata.py create mode 100644 sdk/python/tests/dsl/metadata_tests.py diff --git a/sdk/python/kfp/dsl/__init__.py b/sdk/python/kfp/dsl/__init__.py index d2a2912d65f..b58ecbae837 100644 --- a/sdk/python/kfp/dsl/__init__.py +++ b/sdk/python/kfp/dsl/__init__.py @@ -15,6 +15,7 @@ from ._pipeline_param import PipelineParam from ._pipeline import Pipeline, pipeline, get_pipeline_conf -from ._container_op import ContainerOp, ComponentMeta, ParameterMeta, TypeMeta +from ._container_op import ContainerOp from ._ops_group import OpsGroup, ExitHandler, Condition from ._python_component import python_component +from ._metadata import ComponentMeta, ParameterMeta, TypeMeta \ No newline at end of file diff --git a/sdk/python/kfp/dsl/_container_op.py b/sdk/python/kfp/dsl/_container_op.py index 7d92241c533..f106faea5d7 100644 --- a/sdk/python/kfp/dsl/_container_op.py +++ b/sdk/python/kfp/dsl/_container_op.py @@ -17,99 +17,7 @@ from . import _pipeline_param from ._pipeline_param import _extract_pipelineparams import re -from typing import Dict, List -from abc import ABCMeta, abstractmethod -from ._types import _check_valid_type_dict - -class BaseMeta(object): - __metaclass__ = ABCMeta - def __init__(self): - pass - - @abstractmethod - def to_dict(self): - pass - - def serialize(self): - import yaml - return yaml.dump(self.to_dict()) - -class TypeMeta(BaseMeta): - def __init__(self, - name: str = '', - properties: Dict = None): - self.name = name - self.properties = {} if properties is None else properties - - def to_dict(self): - return {self.name: self.properties} - - @staticmethod - def from_dict(json_dict): - if not _check_valid_type_dict(json_dict): - raise ValueError(json_dict + ' is not a valid type string') - type_meta = TypeMeta() - type_meta.name, type_meta.properties = list(json_dict.items())[0] - return type_meta - - def __eq__(self, other): - return self.__dict__ == other.__dict__ - -class ParameterMeta(BaseMeta): - def __init__(self, - name: str = '', - description: str = '', - param_type: TypeMeta = None, - default = ''): - self.name = name - self.description = description - self.param_type = TypeMeta() if param_type is None else param_type - self.default = default - - def to_dict(self): - return {'name': self.name, - 'description': self.description, - 'type': self.param_type.to_dict(), - 'default': self.default} - -class ComponentMeta(BaseMeta): - def __init__( - self, - name: str = '', - description: str = '', - inputs: List[ParameterMeta] = None, - outputs: List[ParameterMeta] = None - ): - self.name = name - self.description = description - self.inputs = [] if inputs is None else inputs - self.outputs = [] if outputs is None else outputs - - def to_dict(self): - return {'name': self.name, - 'description': self.description, - 'inputs': [ input.to_dict() for input in self.inputs ], - 'outputs': [ output.to_dict() for output in self.outputs ] - } - -# Add a pipeline level metadata calss here. -# If one day we combine the component and pipeline yaml, ComponentMeta and PipelineMeta will become one, too. -class PipelineMeta(BaseMeta): - def __init__( - self, - name: str = '', - description: str = '', - inputs: List[ParameterMeta] = None - ): - self.name = name - self.description = description - self.inputs = [] if inputs is None else inputs - - def to_dict(self): - return {'name': self.name, - 'description': self.description, - 'inputs': [ input.to_dict() for input in self.inputs ] - } +from typing import Dict class ContainerOp(object): """Represents an op implemented by a docker container image.""" diff --git a/sdk/python/kfp/dsl/_metadata.py b/sdk/python/kfp/dsl/_metadata.py new file mode 100644 index 00000000000..119114315d2 --- /dev/null +++ b/sdk/python/kfp/dsl/_metadata.py @@ -0,0 +1,107 @@ +# Copyright 2018 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from typing import Dict, List +from abc import ABCMeta, abstractmethod +from ._types import _check_valid_type_dict + +class BaseMeta(object): + __metaclass__ = ABCMeta + def __init__(self): + pass + + @abstractmethod + def to_dict(self): + pass + + def serialize(self): + import yaml + return yaml.dump(self.to_dict()) + +class TypeMeta(BaseMeta): + def __init__(self, + name: str = '', + properties: Dict = None): + self.name = name + self.properties = {} if properties is None else properties + + def to_dict(self): + return {self.name: self.properties} + + @staticmethod + def from_dict(json_dict): + if not _check_valid_type_dict(json_dict): + raise ValueError(json_dict + ' is not a valid type string') + type_meta = TypeMeta() + type_meta.name, type_meta.properties = list(json_dict.items())[0] + return type_meta + + def __eq__(self, other): + return self.__dict__ == other.__dict__ + +class ParameterMeta(BaseMeta): + def __init__(self, + name: str = '', + description: str = '', + param_type: TypeMeta = None, + default = ''): + self.name = name + self.description = description + self.param_type = TypeMeta() if param_type is None else param_type + self.default = default + + def to_dict(self): + return {'name': self.name, + 'description': self.description, + 'type': self.param_type.to_dict(), + 'default': self.default} + +class ComponentMeta(BaseMeta): + def __init__( + self, + name: str = '', + description: str = '', + inputs: List[ParameterMeta] = None, + outputs: List[ParameterMeta] = None + ): + self.name = name + self.description = description + self.inputs = [] if inputs is None else inputs + self.outputs = [] if outputs is None else outputs + + def to_dict(self): + return {'name': self.name, + 'description': self.description, + 'inputs': [ input.to_dict() for input in self.inputs ], + 'outputs': [ output.to_dict() for output in self.outputs ] + } + +# Add a pipeline level metadata calss here. +# If one day we combine the component and pipeline yaml, ComponentMeta and PipelineMeta will become one, too. +class PipelineMeta(BaseMeta): + def __init__( + self, + name: str = '', + description: str = '', + inputs: List[ParameterMeta] = None + ): + self.name = name + self.description = description + self.inputs = [] if inputs is None else inputs + + def to_dict(self): + return {'name': self.name, + 'description': self.description, + 'inputs': [ input.to_dict() for input in self.inputs ] + } \ No newline at end of file diff --git a/sdk/python/tests/dsl/container_op_tests.py b/sdk/python/tests/dsl/container_op_tests.py index 836c83da7e2..957694d59c4 100644 --- a/sdk/python/tests/dsl/container_op_tests.py +++ b/sdk/python/tests/dsl/container_op_tests.py @@ -14,96 +14,8 @@ from kfp.dsl import Pipeline, PipelineParam, ContainerOp -from kfp.dsl import ComponentMeta, ParameterMeta, TypeMeta import unittest -class TestComponentMeta(unittest.TestCase): - - def test_to_dict(self): - component_meta = ComponentMeta(name='foobar', - description='foobar example', - inputs=[ParameterMeta(name='input1', - description='input1 desc', - param_type=TypeMeta(name='GCSPath', - properties={'bucket_type': 'directory', - 'file_type': 'csv' - } - ), - default='default1' - ), - ParameterMeta(name='input2', - description='input2 desc', - param_type=TypeMeta(name='TFModel', - properties={'input_data': 'tensor', - 'version': '1.8.0' - } - ), - default='default2' - ), - ], - outputs=[ParameterMeta(name='output1', - description='output1 desc', - param_type=TypeMeta(name='Schema', - properties={'file_type': 'tsv' - } - ), - default='default_output1' - ) - ] - ) - golden_meta = { - 'name': 'foobar', - 'description': 'foobar example', - 'inputs': [ - { - 'name': 'input1', - 'description': 'input1 desc', - 'type': { - 'GCSPath': { - 'bucket_type': 'directory', - 'file_type': 'csv' - } - }, - 'default': 'default1' - }, - { - 'name': 'input2', - 'description': 'input2 desc', - 'type': { - 'TFModel': { - 'input_data': 'tensor', - 'version': '1.8.0' - } - }, - 'default': 'default2' - } - ], - 'outputs': [ - { - 'name': 'output1', - 'description': 'output1 desc', - 'type': { - 'Schema': { - 'file_type': 'tsv' - } - }, - 'default': 'default_output1' - } - ] - } - self.assertEqual(component_meta.to_dict(), golden_meta) - - def test_type_meta_from_dict(self): - component_dict = { - 'GCSPath': { - 'bucket_type': 'directory', - 'file_type': 'csv' - } - } - golden_type_meta = TypeMeta(name='GCSPath', properties={'bucket_type': 'directory', - 'file_type': 'csv'}) - self.assertEqual(TypeMeta.from_dict(component_dict), golden_type_meta) - class TestContainerOp(unittest.TestCase): def test_basic(self): diff --git a/sdk/python/tests/dsl/main.py b/sdk/python/tests/dsl/main.py index 6192a944d88..c04ba186004 100644 --- a/sdk/python/tests/dsl/main.py +++ b/sdk/python/tests/dsl/main.py @@ -21,6 +21,7 @@ import container_op_tests import ops_group_tests import type_tests +import metadata_tests if __name__ == '__main__': suite = unittest.TestSuite() @@ -29,6 +30,7 @@ suite.addTests(unittest.defaultTestLoader.loadTestsFromModule(container_op_tests)) suite.addTests(unittest.defaultTestLoader.loadTestsFromModule(ops_group_tests)) suite.addTests(unittest.defaultTestLoader.loadTestsFromModule(type_tests)) + suite.addTests(unittest.defaultTestLoader.loadTestsFromModule(metadata_tests)) runner = unittest.TextTestRunner() if not runner.run(suite).wasSuccessful(): sys.exit(1) diff --git a/sdk/python/tests/dsl/metadata_tests.py b/sdk/python/tests/dsl/metadata_tests.py new file mode 100644 index 00000000000..18fd5a52621 --- /dev/null +++ b/sdk/python/tests/dsl/metadata_tests.py @@ -0,0 +1,103 @@ +# Copyright 2018 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from kfp.dsl import ComponentMeta, ParameterMeta, TypeMeta +import unittest + +class TestComponentMeta(unittest.TestCase): + + def test_to_dict(self): + component_meta = ComponentMeta(name='foobar', + description='foobar example', + inputs=[ParameterMeta(name='input1', + description='input1 desc', + param_type=TypeMeta(name='GCSPath', + properties={'bucket_type': 'directory', + 'file_type': 'csv' + } + ), + default='default1' + ), + ParameterMeta(name='input2', + description='input2 desc', + param_type=TypeMeta(name='TFModel', + properties={'input_data': 'tensor', + 'version': '1.8.0' + } + ), + default='default2' + ), + ], + outputs=[ParameterMeta(name='output1', + description='output1 desc', + param_type=TypeMeta(name='Schema', + properties={'file_type': 'tsv' + } + ), + default='default_output1' + ) + ] + ) + golden_meta = { + 'name': 'foobar', + 'description': 'foobar example', + 'inputs': [ + { + 'name': 'input1', + 'description': 'input1 desc', + 'type': { + 'GCSPath': { + 'bucket_type': 'directory', + 'file_type': 'csv' + } + }, + 'default': 'default1' + }, + { + 'name': 'input2', + 'description': 'input2 desc', + 'type': { + 'TFModel': { + 'input_data': 'tensor', + 'version': '1.8.0' + } + }, + 'default': 'default2' + } + ], + 'outputs': [ + { + 'name': 'output1', + 'description': 'output1 desc', + 'type': { + 'Schema': { + 'file_type': 'tsv' + } + }, + 'default': 'default_output1' + } + ] + } + self.assertEqual(component_meta.to_dict(), golden_meta) + + def test_type_meta_from_dict(self): + component_dict = { + 'GCSPath': { + 'bucket_type': 'directory', + 'file_type': 'csv' + } + } + golden_type_meta = TypeMeta(name='GCSPath', properties={'bucket_type': 'directory', + 'file_type': 'csv'}) + self.assertEqual(TypeMeta.from_dict(component_dict), golden_type_meta) From 8ddfbb2c6dbb8c0f3c6a29a39870ad31011d9572 Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Tue, 5 Mar 2019 13:00:07 -0800 Subject: [PATCH 27/42] fix unit test --- sdk/python/kfp/dsl/__init__.py | 2 +- sdk/python/kfp/dsl/_metadata.py | 2 +- sdk/python/kfp/dsl/_python_component.py | 3 +-- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/sdk/python/kfp/dsl/__init__.py b/sdk/python/kfp/dsl/__init__.py index b58ecbae837..1cfd6ea906b 100644 --- a/sdk/python/kfp/dsl/__init__.py +++ b/sdk/python/kfp/dsl/__init__.py @@ -18,4 +18,4 @@ from ._container_op import ContainerOp from ._ops_group import OpsGroup, ExitHandler, Condition from ._python_component import python_component -from ._metadata import ComponentMeta, ParameterMeta, TypeMeta \ No newline at end of file +from ._metadata import PipelineMeta, ComponentMeta, ParameterMeta, TypeMeta \ No newline at end of file diff --git a/sdk/python/kfp/dsl/_metadata.py b/sdk/python/kfp/dsl/_metadata.py index dcd7db3ae9a..b3b5438b57f 100644 --- a/sdk/python/kfp/dsl/_metadata.py +++ b/sdk/python/kfp/dsl/_metadata.py @@ -14,7 +14,7 @@ from typing import Dict, List from abc import ABCMeta, abstractmethod -from ._types import _check_valid_type_dict +from ._types import BaseType, _check_valid_type_dict, _str_to_dict, _instance_to_dict class BaseMeta(object): __metaclass__ = ABCMeta diff --git a/sdk/python/kfp/dsl/_python_component.py b/sdk/python/kfp/dsl/_python_component.py index 498ab858135..c3e721766cf 100644 --- a/sdk/python/kfp/dsl/_python_component.py +++ b/sdk/python/kfp/dsl/_python_component.py @@ -12,8 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -from kfp.dsl import ComponentMeta, ParameterMeta, TypeMeta -from ._metadata import _annotation_to_typemeta +from ._metadata import ComponentMeta, ParameterMeta, TypeMeta, _annotation_to_typemeta def python_component(name, description=None, base_image=None, target_component_file: str = None): """Decorator for Python component functions. From a1f30a10d420883a89fd351a3d37a52ba6172fc1 Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Tue, 5 Mar 2019 13:12:19 -0800 Subject: [PATCH 28/42] small change --- sdk/python/tests/dsl/pipeline_tests.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sdk/python/tests/dsl/pipeline_tests.py b/sdk/python/tests/dsl/pipeline_tests.py index a2052d1ced2..56edc16ef86 100644 --- a/sdk/python/tests/dsl/pipeline_tests.py +++ b/sdk/python/tests/dsl/pipeline_tests.py @@ -67,6 +67,6 @@ def test_decorator_metadata(self): ) def my_pipeline1(a: {'Schema': {'file_type': 'csv'}}='good', b: Integer()=12): pass - - self.assertEqual('p1', Pipeline.get_pipeline_functions()[my_pipeline1].name) - self.assertEqual('description1', Pipeline.get_pipeline_functions()[my_pipeline1].description) \ No newline at end of file + pipeline_meta = Pipeline.get_pipeline_functions()[my_pipeline1] + self.assertEqual('p1', pipeline_meta.name) + self.assertEqual('description1', pipeline_meta.description) \ No newline at end of file From 637b9214116300642e730bf2d2c000a7bf192071 Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Tue, 5 Mar 2019 13:30:32 -0800 Subject: [PATCH 29/42] add __eq__ to meta classes not export _metadata classes --- sdk/python/kfp/dsl/__init__.py | 3 +- sdk/python/kfp/dsl/_metadata.py | 6 ++-- sdk/python/tests/dsl/metadata_tests.py | 39 ++++++++++++++++++-------- 3 files changed, 31 insertions(+), 17 deletions(-) diff --git a/sdk/python/kfp/dsl/__init__.py b/sdk/python/kfp/dsl/__init__.py index b58ecbae837..1bf2721e28f 100644 --- a/sdk/python/kfp/dsl/__init__.py +++ b/sdk/python/kfp/dsl/__init__.py @@ -17,5 +17,4 @@ from ._pipeline import Pipeline, pipeline, get_pipeline_conf from ._container_op import ContainerOp from ._ops_group import OpsGroup, ExitHandler, Condition -from ._python_component import python_component -from ._metadata import ComponentMeta, ParameterMeta, TypeMeta \ No newline at end of file +from ._python_component import python_component \ No newline at end of file diff --git a/sdk/python/kfp/dsl/_metadata.py b/sdk/python/kfp/dsl/_metadata.py index 119114315d2..c47dda86f3c 100644 --- a/sdk/python/kfp/dsl/_metadata.py +++ b/sdk/python/kfp/dsl/_metadata.py @@ -29,6 +29,9 @@ def serialize(self): import yaml return yaml.dump(self.to_dict()) + def __eq__(self, other): + return self.__dict__ == other.__dict__ + class TypeMeta(BaseMeta): def __init__(self, name: str = '', @@ -47,9 +50,6 @@ def from_dict(json_dict): type_meta.name, type_meta.properties = list(json_dict.items())[0] return type_meta - def __eq__(self, other): - return self.__dict__ == other.__dict__ - class ParameterMeta(BaseMeta): def __init__(self, name: str = '', diff --git a/sdk/python/tests/dsl/metadata_tests.py b/sdk/python/tests/dsl/metadata_tests.py index 18fd5a52621..1edc00480bf 100644 --- a/sdk/python/tests/dsl/metadata_tests.py +++ b/sdk/python/tests/dsl/metadata_tests.py @@ -15,6 +15,32 @@ from kfp.dsl import ComponentMeta, ParameterMeta, TypeMeta import unittest +class TestTypeMeta(unittest.TestCase): + def test_from_dict(self): + component_dict = { + 'GCSPath': { + 'bucket_type': 'directory', + 'file_type': 'csv' + } + } + golden_type_meta = TypeMeta(name='GCSPath', properties={'bucket_type': 'directory', + 'file_type': 'csv'}) + self.assertEqual(TypeMeta.from_dict(component_dict), golden_type_meta) + + def test_eq(self): + type_a = TypeMeta(name='GCSPath', properties={'bucket_type': 'directory', + 'file_type': 'csv'}) + type_b = TypeMeta(name='GCSPath', properties={'bucket_type': 'directory', + 'file_type': 'tsv'}) + type_c = TypeMeta(name='GCSPatha', properties={'bucket_type': 'directory', + 'file_type': 'csv'}) + type_d = TypeMeta(name='GCSPath', properties={'bucket_type': 'directory', + 'file_type': 'csv'}) + self.assertNotEqual(type_a, type_b) + self.assertNotEqual(type_a, type_c) + self.assertEqual(type_a, type_d) + + class TestComponentMeta(unittest.TestCase): def test_to_dict(self): @@ -89,15 +115,4 @@ def test_to_dict(self): } ] } - self.assertEqual(component_meta.to_dict(), golden_meta) - - def test_type_meta_from_dict(self): - component_dict = { - 'GCSPath': { - 'bucket_type': 'directory', - 'file_type': 'csv' - } - } - golden_type_meta = TypeMeta(name='GCSPath', properties={'bucket_type': 'directory', - 'file_type': 'csv'}) - self.assertEqual(TypeMeta.from_dict(component_dict), golden_type_meta) + self.assertEqual(component_meta.to_dict(), golden_meta) \ No newline at end of file From 2759d6360cf43a221693ae25ddac0bf4d1e73312 Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Tue, 5 Mar 2019 13:39:41 -0800 Subject: [PATCH 30/42] nothing --- sdk/python/kfp/components/_dsl_bridge.py | 4 +--- sdk/python/kfp/dsl/_container_op.py | 1 + sdk/python/tests/dsl/metadata_tests.py | 2 +- sdk/python/tests/dsl/python_component_tests.py | 2 +- 4 files changed, 4 insertions(+), 5 deletions(-) diff --git a/sdk/python/kfp/components/_dsl_bridge.py b/sdk/python/kfp/components/_dsl_bridge.py index 1b0ccf44d72..7b5e5064fee 100644 --- a/sdk/python/kfp/components/_dsl_bridge.py +++ b/sdk/python/kfp/components/_dsl_bridge.py @@ -16,9 +16,7 @@ from typing import Mapping from ._structures import ConcatPlaceholder, IfPlaceholder, InputValuePlaceholder, InputPathPlaceholder, IsPresentPlaceholder, OutputPathPlaceholder, TaskSpec from ._components import _generate_output_file_name, _default_component_name -from kfp.dsl import ComponentMeta, ParameterMeta, TypeMeta -from kfp.dsl._container_op import _annotation_to_typemeta - +from kfp.dsl._metadata import ComponentMeta, ParameterMeta, TypeMeta, _annotation_to_typemeta def create_container_op_from_task(task_spec: TaskSpec): argument_values = task_spec.arguments diff --git a/sdk/python/kfp/dsl/_container_op.py b/sdk/python/kfp/dsl/_container_op.py index 945f81481a2..6bfd232ac5c 100644 --- a/sdk/python/kfp/dsl/_container_op.py +++ b/sdk/python/kfp/dsl/_container_op.py @@ -16,6 +16,7 @@ from . import _pipeline from . import _pipeline_param from ._pipeline_param import _extract_pipelineparams +from ._metadata import ComponentMeta import re from typing import Dict diff --git a/sdk/python/tests/dsl/metadata_tests.py b/sdk/python/tests/dsl/metadata_tests.py index 1edc00480bf..ce48d7811a9 100644 --- a/sdk/python/tests/dsl/metadata_tests.py +++ b/sdk/python/tests/dsl/metadata_tests.py @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -from kfp.dsl import ComponentMeta, ParameterMeta, TypeMeta +from kfp.dsl._metadata import ComponentMeta, ParameterMeta, TypeMeta import unittest class TestTypeMeta(unittest.TestCase): diff --git a/sdk/python/tests/dsl/python_component_tests.py b/sdk/python/tests/dsl/python_component_tests.py index bf8864ab8bc..877e83a07cd 100644 --- a/sdk/python/tests/dsl/python_component_tests.py +++ b/sdk/python/tests/dsl/python_component_tests.py @@ -14,7 +14,7 @@ from kfp.dsl._python_component import component -from kfp.dsl._container_op import ComponentMeta, ParameterMeta, TypeMeta +from kfp.dsl._metadata import ComponentMeta, ParameterMeta, TypeMeta from kfp.dsl._types import GCSPath, Integer import unittest import mock From 23928152176de9191f1ff3ce2f613f2349b729de Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Tue, 5 Mar 2019 13:41:07 -0800 Subject: [PATCH 31/42] fix unit test --- sdk/python/tests/dsl/metadata_tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/python/tests/dsl/metadata_tests.py b/sdk/python/tests/dsl/metadata_tests.py index 1edc00480bf..ce48d7811a9 100644 --- a/sdk/python/tests/dsl/metadata_tests.py +++ b/sdk/python/tests/dsl/metadata_tests.py @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -from kfp.dsl import ComponentMeta, ParameterMeta, TypeMeta +from kfp.dsl._metadata import ComponentMeta, ParameterMeta, TypeMeta import unittest class TestTypeMeta(unittest.TestCase): From e51ecf78e6ad55a5d097b22c31e8306c10f09685 Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Tue, 5 Mar 2019 14:04:26 -0800 Subject: [PATCH 32/42] unit test python component --- sdk/python/tests/dsl/python_component_tests.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/sdk/python/tests/dsl/python_component_tests.py b/sdk/python/tests/dsl/python_component_tests.py index 877e83a07cd..d5a7914c435 100644 --- a/sdk/python/tests/dsl/python_component_tests.py +++ b/sdk/python/tests/dsl/python_component_tests.py @@ -19,23 +19,26 @@ import unittest import mock -@mock.patch('kfp.dsl.ContainerOp') class TestPythonComponent(unittest.TestCase): - def test_component(self, ContainerOp): + def test_component(self): """Test component decorator.""" + + class MockContainerOp: + def _set_metadata(self, component_meta): + self._metadata = component_meta + @component def componentA(a: {'Schema': {'file_type': 'csv'}}, b: '{"number": {"step": "large"}}' = 12, c: GCSPath(path_type='file', file_type='tsv') = 'gs://hello/world') -> {'model': Integer()}: - return ContainerOp() + return MockContainerOp() - ContainerOp()._set_metadata.return_value = None - componentA(1,2,3) + containerOp = componentA(1,2,3) golden_meta = ComponentMeta(name='componentA', description='') golden_meta.inputs.append(ParameterMeta(name='a', description='', param_type=TypeMeta(name='Schema', properties={'file_type': 'csv'}))) golden_meta.inputs.append(ParameterMeta(name='b', description='', param_type=TypeMeta(name='number', properties={'step': 'large'}), default=12)) golden_meta.inputs.append(ParameterMeta(name='c', description='', param_type=TypeMeta(name='GCSPath', properties={'path_type':'file', 'file_type': 'tsv'}), default='gs://hello/world')) - golden_meta.inputs.append(ParameterMeta(name='model', description='', param_type=TypeMeta(name='Integer'))) + golden_meta.outputs.append(ParameterMeta(name='model', description='', param_type=TypeMeta(name='Integer'))) - ContainerOp()._set_metadata.assert_called_once_with(golden_meta) + self.assertEqual(containerOp._metadata, golden_meta) From f3f02c4bfb2e015bba4ef29c61e6cedf8d01e93e Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Tue, 5 Mar 2019 14:14:35 -0800 Subject: [PATCH 33/42] unit test python pipeline --- sdk/python/kfp/dsl/_pipeline.py | 9 ++++++++- sdk/python/tests/dsl/pipeline_tests.py | 9 +++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/sdk/python/kfp/dsl/_pipeline.py b/sdk/python/kfp/dsl/_pipeline.py index cfc3b59ecf3..057f1195627 100644 --- a/sdk/python/kfp/dsl/_pipeline.py +++ b/sdk/python/kfp/dsl/_pipeline.py @@ -38,14 +38,21 @@ def _pipeline(func): args = fullargspec.args annotations = fullargspec.annotations + # defaults + arg_defaults = {} + if fullargspec.defaults: + for arg, default in zip(reversed(fullargspec.args), reversed(fullargspec.defaults)): + arg_defaults[arg] = default + # Construct the PipelineMeta pipeline_meta = PipelineMeta(name=name, description=description) # Inputs for arg in args: arg_type = TypeMeta() + arg_default = arg_defaults[arg] if arg in arg_defaults else '' if arg in annotations: arg_type = _annotation_to_typemeta(annotations[arg]) - pipeline_meta.inputs.append(ParameterMeta(name=arg, description='', param_type=arg_type)) + pipeline_meta.inputs.append(ParameterMeta(name=arg, description='', param_type=arg_type, default=arg_default)) #TODO: add descriptions to the metadata #docstring parser: diff --git a/sdk/python/tests/dsl/pipeline_tests.py b/sdk/python/tests/dsl/pipeline_tests.py index 56edc16ef86..e7dc44cc37d 100644 --- a/sdk/python/tests/dsl/pipeline_tests.py +++ b/sdk/python/tests/dsl/pipeline_tests.py @@ -14,6 +14,7 @@ import kfp from kfp.dsl import Pipeline, PipelineParam, ContainerOp, pipeline +from kfp.dsl._metadata import PipelineMeta, ParameterMeta, TypeMeta from kfp.dsl._types import GCSPath, Integer import unittest @@ -67,6 +68,10 @@ def test_decorator_metadata(self): ) def my_pipeline1(a: {'Schema': {'file_type': 'csv'}}='good', b: Integer()=12): pass + + golden_meta = PipelineMeta(name='p1', description='description1') + golden_meta.inputs.append(ParameterMeta(name='a', description='', param_type=TypeMeta(name='Schema', properties={'file_type': 'csv'}), default='good')) + golden_meta.inputs.append(ParameterMeta(name='b', description='', param_type=TypeMeta(name='Integer'), default=12)) + pipeline_meta = Pipeline.get_pipeline_functions()[my_pipeline1] - self.assertEqual('p1', pipeline_meta.name) - self.assertEqual('description1', pipeline_meta.description) \ No newline at end of file + self.assertEqual(pipeline_meta, golden_meta) \ No newline at end of file From 8312845b8894b83c0eb91a2596f4604c8402ef90 Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Tue, 5 Mar 2019 14:16:09 -0800 Subject: [PATCH 34/42] fix bug: duplicate variable of args --- sdk/python/kfp/dsl/_python_component.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sdk/python/kfp/dsl/_python_component.py b/sdk/python/kfp/dsl/_python_component.py index c3e721766cf..001ed0a6d48 100644 --- a/sdk/python/kfp/dsl/_python_component.py +++ b/sdk/python/kfp/dsl/_python_component.py @@ -63,7 +63,6 @@ def foobar(model: TFModel(), step: MLStep()): def _component(*args, **kargs): import inspect fullargspec = inspect.getfullargspec(func) - args = fullargspec.args annotations = fullargspec.annotations # defaults @@ -75,7 +74,7 @@ def _component(*args, **kargs): # Construct the ComponentMeta component_meta = ComponentMeta(name=func.__name__, description='') # Inputs - for arg in args: + for arg in fullargspec.args: arg_type = TypeMeta() arg_default = arg_defaults[arg] if arg in arg_defaults else '' if arg in annotations: From 31d7b33ade4507acca1565a65dd89bcbdbba2383 Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Tue, 5 Mar 2019 15:12:28 -0800 Subject: [PATCH 35/42] fix unit tests --- sdk/python/kfp/components/_dsl_bridge.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/python/kfp/components/_dsl_bridge.py b/sdk/python/kfp/components/_dsl_bridge.py index 7b5e5064fee..2a673a0ec6e 100644 --- a/sdk/python/kfp/components/_dsl_bridge.py +++ b/sdk/python/kfp/components/_dsl_bridge.py @@ -156,10 +156,10 @@ def _create_container_op_from_resolved_task(name:str, container_image:str, comma # Inputs if component_spec.inputs is not None: for input in component_spec.inputs: - component_meta.inputs.append(ParameterMeta(name=input.name, description=input.description, type=_annotation_to_typemeta(input.type), default=input.default)) + component_meta.inputs.append(ParameterMeta(name=input.name, description=input.description, param_type=_annotation_to_typemeta(input.type), default=input.default)) if component_spec.outputs is not None: for output in component_spec.outputs: - component_meta.outputs.append(ParameterMeta(name=output.name, description=output.description, type=_annotation_to_typemeta(input.type))) + component_meta.outputs.append(ParameterMeta(name=output.name, description=output.description, param_type=_annotation_to_typemeta(output.type))) task = dsl.ContainerOp( name=name, From 7bf701a5c919f0b0374516691fcfc6033582ce42 Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Tue, 5 Mar 2019 15:20:02 -0800 Subject: [PATCH 36/42] move python_component and _component decorator in _component file --- sdk/python/kfp/dsl/__init__.py | 3 ++- sdk/python/kfp/dsl/{_python_component.py => _component.py} | 0 .../dsl/{python_component_tests.py => component_tests.py} | 2 +- sdk/python/tests/dsl/main.py | 4 ++-- 4 files changed, 5 insertions(+), 4 deletions(-) rename sdk/python/kfp/dsl/{_python_component.py => _component.py} (100%) rename sdk/python/tests/dsl/{python_component_tests.py => component_tests.py} (95%) diff --git a/sdk/python/kfp/dsl/__init__.py b/sdk/python/kfp/dsl/__init__.py index 1bf2721e28f..06fae50887b 100644 --- a/sdk/python/kfp/dsl/__init__.py +++ b/sdk/python/kfp/dsl/__init__.py @@ -17,4 +17,5 @@ from ._pipeline import Pipeline, pipeline, get_pipeline_conf from ._container_op import ContainerOp from ._ops_group import OpsGroup, ExitHandler, Condition -from ._python_component import python_component \ No newline at end of file +from ._component import python_component +#TODO: expose the component decorator when ready \ No newline at end of file diff --git a/sdk/python/kfp/dsl/_python_component.py b/sdk/python/kfp/dsl/_component.py similarity index 100% rename from sdk/python/kfp/dsl/_python_component.py rename to sdk/python/kfp/dsl/_component.py diff --git a/sdk/python/tests/dsl/python_component_tests.py b/sdk/python/tests/dsl/component_tests.py similarity index 95% rename from sdk/python/tests/dsl/python_component_tests.py rename to sdk/python/tests/dsl/component_tests.py index b57ad311f09..47516b1a20e 100644 --- a/sdk/python/tests/dsl/python_component_tests.py +++ b/sdk/python/tests/dsl/component_tests.py @@ -13,7 +13,7 @@ # limitations under the License. -from kfp.dsl._python_component import component +from kfp.dsl._component import component from kfp.dsl._types import GCSPath, Integer import unittest diff --git a/sdk/python/tests/dsl/main.py b/sdk/python/tests/dsl/main.py index 8a86cb5d9bf..e994f21d83e 100644 --- a/sdk/python/tests/dsl/main.py +++ b/sdk/python/tests/dsl/main.py @@ -21,7 +21,7 @@ import container_op_tests import ops_group_tests import type_tests -import python_component_tests +import component_tests import metadata_tests if __name__ == '__main__': @@ -31,7 +31,7 @@ suite.addTests(unittest.defaultTestLoader.loadTestsFromModule(container_op_tests)) suite.addTests(unittest.defaultTestLoader.loadTestsFromModule(ops_group_tests)) suite.addTests(unittest.defaultTestLoader.loadTestsFromModule(type_tests)) - suite.addTests(unittest.defaultTestLoader.loadTestsFromModule(python_component_tests)) + suite.addTests(unittest.defaultTestLoader.loadTestsFromModule(component_tests)) suite.addTests(unittest.defaultTestLoader.loadTestsFromModule(metadata_tests)) runner = unittest.TextTestRunner() if not runner.run(suite).wasSuccessful(): From 16c2f8fb5a7f97d88f3cf3aae4fa17cc5a22eae7 Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Tue, 5 Mar 2019 16:07:39 -0800 Subject: [PATCH 37/42] remove the print --- sdk/python/kfp/dsl/_pipeline.py | 1 - 1 file changed, 1 deletion(-) diff --git a/sdk/python/kfp/dsl/_pipeline.py b/sdk/python/kfp/dsl/_pipeline.py index 37ee4a0d0ec..d3e757ec704 100644 --- a/sdk/python/kfp/dsl/_pipeline.py +++ b/sdk/python/kfp/dsl/_pipeline.py @@ -51,7 +51,6 @@ def _pipeline(func): #docstring parser: # https://github.com/rr-/docstring_parser # https://github.com/terrencepreilly/darglint/blob/master/darglint/parse.py - print(pipeline_meta.serialize()) #TODO: parse the metadata to the Pipeline. Pipeline.add_pipeline(name, description, func) From b74c5270296d7563f0b8de7fdbff21ba8f05e0ca Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Tue, 5 Mar 2019 16:13:05 -0800 Subject: [PATCH 38/42] change parameter default value to None --- sdk/python/kfp/dsl/_component.py | 2 +- sdk/python/kfp/dsl/_metadata.py | 2 +- sdk/python/kfp/dsl/_pipeline.py | 2 +- sdk/python/tests/dsl/component_tests.py | 3 +-- 4 files changed, 4 insertions(+), 5 deletions(-) diff --git a/sdk/python/kfp/dsl/_component.py b/sdk/python/kfp/dsl/_component.py index 8ec3c9a998e..1dedb59db7e 100644 --- a/sdk/python/kfp/dsl/_component.py +++ b/sdk/python/kfp/dsl/_component.py @@ -76,7 +76,7 @@ def _component(*args, **kargs): # Inputs for arg in fullargspec.args: arg_type = TypeMeta() - arg_default = arg_defaults[arg] if arg in arg_defaults else '' + arg_default = arg_defaults[arg] if arg in arg_defaults else None if arg in annotations: arg_type = _annotation_to_typemeta(annotations[arg]) component_meta.inputs.append(ParameterMeta(name=arg, description='', param_type=arg_type, default=arg_default)) diff --git a/sdk/python/kfp/dsl/_metadata.py b/sdk/python/kfp/dsl/_metadata.py index 411c26cfe39..739d6b8b66f 100644 --- a/sdk/python/kfp/dsl/_metadata.py +++ b/sdk/python/kfp/dsl/_metadata.py @@ -55,7 +55,7 @@ def __init__(self, name: str = '', description: str = '', param_type: TypeMeta = None, - default = ''): + default = None): self.name = name self.description = description self.param_type = TypeMeta() if param_type is None else param_type diff --git a/sdk/python/kfp/dsl/_pipeline.py b/sdk/python/kfp/dsl/_pipeline.py index 4675ba72198..e9315bb2022 100644 --- a/sdk/python/kfp/dsl/_pipeline.py +++ b/sdk/python/kfp/dsl/_pipeline.py @@ -49,7 +49,7 @@ def _pipeline(func): # Inputs for arg in args: arg_type = TypeMeta() - arg_default = arg_defaults[arg] if arg in arg_defaults else '' + arg_default = arg_defaults[arg] if arg in arg_defaults else None if arg in annotations: arg_type = _annotation_to_typemeta(annotations[arg]) pipeline_meta.inputs.append(ParameterMeta(name=arg, description='', param_type=arg_type, default=arg_default)) diff --git a/sdk/python/tests/dsl/component_tests.py b/sdk/python/tests/dsl/component_tests.py index 594d94ea54a..5515d929588 100644 --- a/sdk/python/tests/dsl/component_tests.py +++ b/sdk/python/tests/dsl/component_tests.py @@ -40,5 +40,4 @@ def componentA(a: {'Schema': {'file_type': 'csv'}}, b: '{"number": {"step": "lar golden_meta.inputs.append(ParameterMeta(name='c', description='', param_type=TypeMeta(name='GCSPath', properties={'path_type':'file', 'file_type': 'tsv'}), default='gs://hello/world')) golden_meta.outputs.append(ParameterMeta(name='model', description='', param_type=TypeMeta(name='Integer'))) - self.assertEqual(containerOp._metadata, golden_meta) - + self.assertEqual(containerOp._metadata, golden_meta) \ No newline at end of file From e1d4ae591f3cdfc278f6f9e95201316f1225b1a3 Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Tue, 5 Mar 2019 16:18:25 -0800 Subject: [PATCH 39/42] add functools wraps around _component decorator --- sdk/python/kfp/dsl/_component.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sdk/python/kfp/dsl/_component.py b/sdk/python/kfp/dsl/_component.py index 1dedb59db7e..df547a37f5e 100644 --- a/sdk/python/kfp/dsl/_component.py +++ b/sdk/python/kfp/dsl/_component.py @@ -60,6 +60,8 @@ def component(func): def foobar(model: TFModel(), step: MLStep()): return dsl.ContainerOp() """ + from functools import wraps + @wraps(func) def _component(*args, **kargs): import inspect fullargspec = inspect.getfullargspec(func) From 803c038689256060fef2d6b07e037fcb09380f33 Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Tue, 5 Mar 2019 22:35:00 -0800 Subject: [PATCH 40/42] TypeMeta accept both str and dict --- sdk/python/kfp/dsl/_metadata.py | 28 +++++++++++++++---------- sdk/python/kfp/dsl/_types.py | 13 +++--------- sdk/python/tests/dsl/component_tests.py | 4 ++-- sdk/python/tests/dsl/metadata_tests.py | 11 +++++++--- sdk/python/tests/dsl/type_tests.py | 18 ++-------------- 5 files changed, 32 insertions(+), 42 deletions(-) diff --git a/sdk/python/kfp/dsl/_metadata.py b/sdk/python/kfp/dsl/_metadata.py index 739d6b8b66f..742ecb3bd12 100644 --- a/sdk/python/kfp/dsl/_metadata.py +++ b/sdk/python/kfp/dsl/_metadata.py @@ -14,7 +14,7 @@ from typing import Dict, List from abc import ABCMeta, abstractmethod -from ._types import BaseType, _check_valid_type_dict, _str_to_dict, _instance_to_dict +from ._types import BaseType, _check_valid_type_dict, _instance_to_dict class BaseMeta(object): __metaclass__ = ABCMeta @@ -39,15 +39,21 @@ def __init__(self, self.name = name self.properties = {} if properties is None else properties - def to_dict(self): - return {self.name: self.properties} + def to_dict_or_str(self): + if self.properties is None: + return self.name + else: + return {self.name: self.properties} @staticmethod - def from_dict(json_dict): - if not _check_valid_type_dict(json_dict): - raise ValueError(json_dict + ' is not a valid type string') + def from_dict_or_str(json): type_meta = TypeMeta() - type_meta.name, type_meta.properties = list(json_dict.items())[0] + if isinstance(json, Dict): + if not _check_valid_type_dict(json): + raise ValueError(json + ' is not a valid type string') + type_meta.name, type_meta.properties = list(json.items())[0] + elif isinstance(json, str): + type_meta.name = json return type_meta class ParameterMeta(BaseMeta): @@ -64,7 +70,7 @@ def __init__(self, def to_dict(self): return {'name': self.name, 'description': self.description, - 'type': self.param_type.to_dict(), + 'type': self.param_type.to_dict_or_str(), 'default': self.default} class ComponentMeta(BaseMeta): @@ -114,13 +120,13 @@ def _annotation_to_typemeta(annotation): TypeMeta ''' if isinstance(annotation, BaseType): - arg_type = TypeMeta.from_dict(_instance_to_dict(annotation)) + arg_type = TypeMeta.from_dict_or_str(_instance_to_dict(annotation)) elif isinstance(annotation, str): - arg_type = TypeMeta.from_dict(_str_to_dict(annotation)) + arg_type = TypeMeta.from_dict_or_str(annotation) elif isinstance(annotation, dict): if not _check_valid_type_dict(annotation): raise ValueError('Annotation ' + str(annotation) + ' is not a valid type dictionary.') - arg_type = TypeMeta.from_dict(annotation) + arg_type = TypeMeta.from_dict_or_str(annotation) else: return TypeMeta() return arg_type diff --git a/sdk/python/kfp/dsl/_types.py b/sdk/python/kfp/dsl/_types.py index 432eb49250e..5796ffd6457 100644 --- a/sdk/python/kfp/dsl/_types.py +++ b/sdk/python/kfp/dsl/_types.py @@ -103,11 +103,11 @@ def check_types(checked_type, expected_type): if isinstance(checked_type, BaseType): checked_type = _instance_to_dict(checked_type) elif isinstance(checked_type, str): - checked_type = _str_to_dict(checked_type) + checked_type = {checked_type: {}} if isinstance(expected_type, BaseType): expected_type = _instance_to_dict(expected_type) elif isinstance(expected_type, str): - expected_type = _str_to_dict(expected_type) + expected_type = {expected_type: {}} return _check_dict_types(checked_type, expected_type) def _check_valid_type_dict(payload): @@ -136,13 +136,6 @@ def _instance_to_dict(instance): ''' return {type(instance).__name__: instance.__dict__} -def _str_to_dict(payload): - import json - json_dict = json.loads(payload) - if not _check_valid_type_dict(json_dict): - raise ValueError(payload + ' is not a valid type string') - return json_dict - def _check_dict_types(checked_type, expected_type): '''_check_dict_types checks the type consistency. Args: @@ -163,4 +156,4 @@ def _check_dict_types(checked_type, expected_type): str(checked_type[type_name][type_property]) + ' and ' + str(expected_type[type_name][type_property])) return False - return True \ No newline at end of file + return True diff --git a/sdk/python/tests/dsl/component_tests.py b/sdk/python/tests/dsl/component_tests.py index 5515d929588..62fa35b5e50 100644 --- a/sdk/python/tests/dsl/component_tests.py +++ b/sdk/python/tests/dsl/component_tests.py @@ -29,7 +29,7 @@ def _set_metadata(self, component_meta): self._metadata = component_meta @component - def componentA(a: {'Schema': {'file_type': 'csv'}}, b: '{"number": {"step": "large"}}' = 12, c: GCSPath(path_type='file', file_type='tsv') = 'gs://hello/world') -> {'model': Integer()}: + def componentA(a: {'Schema': {'file_type': 'csv'}}, b: {'number': {'step': 'large'}} = 12, c: GCSPath(path_type='file', file_type='tsv') = 'gs://hello/world') -> {'model': Integer()}: return MockContainerOp() containerOp = componentA(1,2,3) @@ -40,4 +40,4 @@ def componentA(a: {'Schema': {'file_type': 'csv'}}, b: '{"number": {"step": "lar golden_meta.inputs.append(ParameterMeta(name='c', description='', param_type=TypeMeta(name='GCSPath', properties={'path_type':'file', 'file_type': 'tsv'}), default='gs://hello/world')) golden_meta.outputs.append(ParameterMeta(name='model', description='', param_type=TypeMeta(name='Integer'))) - self.assertEqual(containerOp._metadata, golden_meta) \ No newline at end of file + self.assertEqual(containerOp._metadata, golden_meta) diff --git a/sdk/python/tests/dsl/metadata_tests.py b/sdk/python/tests/dsl/metadata_tests.py index ce48d7811a9..c7d1b9dc27e 100644 --- a/sdk/python/tests/dsl/metadata_tests.py +++ b/sdk/python/tests/dsl/metadata_tests.py @@ -16,7 +16,7 @@ import unittest class TestTypeMeta(unittest.TestCase): - def test_from_dict(self): + def test_from_dict_or_str(self): component_dict = { 'GCSPath': { 'bucket_type': 'directory', @@ -25,7 +25,12 @@ def test_from_dict(self): } golden_type_meta = TypeMeta(name='GCSPath', properties={'bucket_type': 'directory', 'file_type': 'csv'}) - self.assertEqual(TypeMeta.from_dict(component_dict), golden_type_meta) + self.assertEqual(TypeMeta.from_dict_or_str(component_dict), golden_type_meta) + + component_str = 'GCSPath' + golden_type_meta = TypeMeta(name='GCSPath') + self.assertEqual(TypeMeta.from_dict_or_str(component_str), golden_type_meta) + def test_eq(self): type_a = TypeMeta(name='GCSPath', properties={'bucket_type': 'directory', @@ -115,4 +120,4 @@ def test_to_dict(self): } ] } - self.assertEqual(component_meta.to_dict(), golden_meta) \ No newline at end of file + self.assertEqual(component_meta.to_dict(), golden_meta) diff --git a/sdk/python/tests/dsl/type_tests.py b/sdk/python/tests/dsl/type_tests.py index 525b5b5f49e..9f6e6b37852 100644 --- a/sdk/python/tests/dsl/type_tests.py +++ b/sdk/python/tests/dsl/type_tests.py @@ -13,7 +13,7 @@ # limitations under the License. -from kfp.dsl._types import _instance_to_dict, _str_to_dict, check_types, GCSPath +from kfp.dsl._types import _instance_to_dict, check_types, GCSPath import unittest class TestTypes(unittest.TestCase): @@ -29,20 +29,6 @@ def test_class_to_dict(self): } self.assertEqual(golden_dict, gcspath_dict) - def test_str_to_dict(self): - gcspath_str = '{"GCSPath": {"file_type": "csv", "path_type": "file"}}' - gcspath_dict = _str_to_dict(gcspath_str) - golden_dict = { - 'GCSPath': { - 'path_type': 'file', - 'file_type': 'csv' - } - } - self.assertEqual(golden_dict, gcspath_dict) - gcspath_str = '{"file_type": "csv", "path_type": "file"}' - with self.assertRaises(ValueError): - _str_to_dict(gcspath_str) - def test_check_types(self): #Core types typeA = GCSPath(path_type='file', file_type='csv') @@ -78,4 +64,4 @@ def test_check_types(self): self.assertFalse(check_types(typeA, typeB)) self.assertFalse(check_types(typeA, typeC)) self.assertTrue(check_types(typeC, typeA)) - self.assertFalse(check_types(typeA, typeD)) \ No newline at end of file + self.assertFalse(check_types(typeA, typeD)) From 3cc1cae32c8246f84c4201219b0fbc8fa3f5423d Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Wed, 6 Mar 2019 09:15:57 -0800 Subject: [PATCH 41/42] fix indent, add unit test for type as strings --- sdk/python/kfp/dsl/_metadata.py | 2 +- sdk/python/kfp/dsl/_types.py | 4 ++-- sdk/python/tests/dsl/metadata_tests.py | 11 +++++++++++ 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/sdk/python/kfp/dsl/_metadata.py b/sdk/python/kfp/dsl/_metadata.py index 742ecb3bd12..e5ba39675be 100644 --- a/sdk/python/kfp/dsl/_metadata.py +++ b/sdk/python/kfp/dsl/_metadata.py @@ -40,7 +40,7 @@ def __init__(self, self.properties = {} if properties is None else properties def to_dict_or_str(self): - if self.properties is None: + if self.properties is None or len(self.properties) == 0: return self.name else: return {self.name: self.properties} diff --git a/sdk/python/kfp/dsl/_types.py b/sdk/python/kfp/dsl/_types.py index 5796ffd6457..298d32505b6 100644 --- a/sdk/python/kfp/dsl/_types.py +++ b/sdk/python/kfp/dsl/_types.py @@ -103,11 +103,11 @@ def check_types(checked_type, expected_type): if isinstance(checked_type, BaseType): checked_type = _instance_to_dict(checked_type) elif isinstance(checked_type, str): - checked_type = {checked_type: {}} + checked_type = {checked_type: {}} if isinstance(expected_type, BaseType): expected_type = _instance_to_dict(expected_type) elif isinstance(expected_type, str): - expected_type = {expected_type: {}} + expected_type = {expected_type: {}} return _check_dict_types(checked_type, expected_type) def _check_valid_type_dict(payload): diff --git a/sdk/python/tests/dsl/metadata_tests.py b/sdk/python/tests/dsl/metadata_tests.py index c7d1b9dc27e..24a174ce257 100644 --- a/sdk/python/tests/dsl/metadata_tests.py +++ b/sdk/python/tests/dsl/metadata_tests.py @@ -69,6 +69,11 @@ def test_to_dict(self): ), default='default2' ), + ParameterMeta(name='input3', + description='input3 desc', + param_type=TypeMeta(name='Integer'), + default='default3' + ), ], outputs=[ParameterMeta(name='output1', description='output1 desc', @@ -105,6 +110,12 @@ def test_to_dict(self): } }, 'default': 'default2' + }, + { + 'name': 'input3', + 'description': 'input3 desc', + 'type': 'Integer', + 'default': 'default3' } ], 'outputs': [ From b06abd3ce7ca405d2c264158e3cf354421998008 Mon Sep 17 00:00:00 2001 From: Ning Gao Date: Wed, 6 Mar 2019 09:36:49 -0800 Subject: [PATCH 42/42] do not set default value for the name field in ParameterMeta, ComponentMeta, and PipelineMeta --- sdk/python/kfp/dsl/_container_op.py | 2 +- sdk/python/kfp/dsl/_metadata.py | 6 +++--- sdk/python/kfp/dsl/_pipeline.py | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/sdk/python/kfp/dsl/_container_op.py b/sdk/python/kfp/dsl/_container_op.py index 6bfd232ac5c..4fb7d6aee59 100644 --- a/sdk/python/kfp/dsl/_container_op.py +++ b/sdk/python/kfp/dsl/_container_op.py @@ -64,7 +64,7 @@ def __init__(self, name: str, image: str, command: str=None, arguments: str=None self.pod_annotations = {} self.pod_labels = {} self.num_retries = 0 - self._metadata = ComponentMeta() + self._metadata = None self.argument_inputs = _extract_pipelineparams([str(arg) for arg in (command or []) + (arguments or [])]) diff --git a/sdk/python/kfp/dsl/_metadata.py b/sdk/python/kfp/dsl/_metadata.py index e5ba39675be..750ac905c1e 100644 --- a/sdk/python/kfp/dsl/_metadata.py +++ b/sdk/python/kfp/dsl/_metadata.py @@ -58,7 +58,7 @@ def from_dict_or_str(json): class ParameterMeta(BaseMeta): def __init__(self, - name: str = '', + name: str, description: str = '', param_type: TypeMeta = None, default = None): @@ -76,7 +76,7 @@ def to_dict(self): class ComponentMeta(BaseMeta): def __init__( self, - name: str = '', + name: str, description: str = '', inputs: List[ParameterMeta] = None, outputs: List[ParameterMeta] = None @@ -98,7 +98,7 @@ def to_dict(self): class PipelineMeta(BaseMeta): def __init__( self, - name: str = '', + name: str, description: str = '', inputs: List[ParameterMeta] = None ): diff --git a/sdk/python/kfp/dsl/_pipeline.py b/sdk/python/kfp/dsl/_pipeline.py index e9315bb2022..bc3cca183c5 100644 --- a/sdk/python/kfp/dsl/_pipeline.py +++ b/sdk/python/kfp/dsl/_pipeline.py @@ -135,7 +135,7 @@ def __init__(self, name: str): self.groups = [_ops_group.OpsGroup('pipeline', name=name)] self.group_id = 0 self.conf = PipelineConf() - self._metadata = PipelineMeta() + self._metadata = None def __enter__(self): if Pipeline._default_pipeline: