From c86ddf73eaf6ea59356a04350e18cc8a5f218cef Mon Sep 17 00:00:00 2001 From: connor-mccarthy Date: Fri, 18 Mar 2022 11:57:28 -0600 Subject: [PATCH 1/8] migrate modules to >= python 3.7 --- .../kfp/components/component_factory.py | 8 +----- sdk/python/kfp/components/structures.py | 8 +++--- sdk/python/kfp/components/v1_modelbase.py | 27 +++++++------------ 3 files changed, 14 insertions(+), 29 deletions(-) diff --git a/sdk/python/kfp/components/component_factory.py b/sdk/python/kfp/components/component_factory.py index 0a533a95cd1..5033ea421dc 100644 --- a/sdk/python/kfp/components/component_factory.py +++ b/sdk/python/kfp/components/component_factory.py @@ -127,15 +127,9 @@ def _annotation_to_type_struct(annotation): else: type_name = str(annotation.__name__) - # TODO: remove the hack once drop Python 3.6 support. - # In Python 3.6+, typing.Dict, typing.List are not instance of type. - if type_annotations.get_short_type_name( - str(annotation)) in ['List', 'Dict']: - type_name = str(annotation) - elif hasattr( annotation, '__forward_arg__' - ): # Handling typing.ForwardRef('Type_name') (the name was _ForwardRef in python 3.5-3.6) + ): # Handling typing.ForwardRef('Type_name') type_name = str(annotation.__forward_arg__) else: type_name = str(annotation) diff --git a/sdk/python/kfp/components/structures.py b/sdk/python/kfp/components/structures.py index e52fe9d5233..70398b35b41 100644 --- a/sdk/python/kfp/components/structures.py +++ b/sdk/python/kfp/components/structures.py @@ -16,7 +16,7 @@ import dataclasses import itertools import json -from typing import Any, Dict, Mapping, Optional, Sequence, Union +from typing import Any, Dict, Mapping, Optional, OrderedDict, Sequence, Union import pydantic import yaml @@ -305,12 +305,10 @@ class ComponentSpec(BaseModel): implementation: The implementation of the component. Either an executor (container, importer) or a DAG consists of other components. """ - # TODO(ji-yaqi): Update to OrderedDict for inputs and outputs once we drop - # Python 3.6 support name: str description: Optional[str] = None - inputs: Optional[Dict[str, InputSpec]] = None - outputs: Optional[Dict[str, OutputSpec]] = None + inputs: Optional[OrderedDict[str, InputSpec]] = None + outputs: Optional[OrderedDict[str, OutputSpec]] = None implementation: Implementation @pydantic.validator('inputs', 'outputs', allow_reuse=True) diff --git a/sdk/python/kfp/components/v1_modelbase.py b/sdk/python/kfp/components/v1_modelbase.py index 0091aff7ea8..b2cc3ee4638 100644 --- a/sdk/python/kfp/components/v1_modelbase.py +++ b/sdk/python/kfp/components/v1_modelbase.py @@ -11,7 +11,8 @@ # 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. - +# mypy: ignore-errors +# pylint: disable=all import inspect from collections import OrderedDict from collections import abc @@ -67,10 +68,7 @@ def verify_object_against_type(x: Any, typ: Type[T]) -> T: raise TypeError( 'Error: None object is incompatible with type {}'.format(typ)) - #assert isinstance(x, typ.__origin__) - generic_type = typ.__origin__ or getattr( - typ, '__extra__', None - ) #In python <3.7 typing.List.__origin__ == None; Python 3.7 has working __origin__, but no __extra__ TODO: Remove the __extra__ once we move to Python 3.7 + generic_type = typ.__origin__ if generic_type in [ list, List, abc.Sequence, abc.MutableSequence, Sequence, MutableSequence @@ -79,7 +77,7 @@ def verify_object_against_type(x: Any, typ: Type[T]) -> T: raise TypeError( 'Error: Object "{}" is incompatible with type "{}"'.format( x, typ)) - # In Python <3.7 Mapping.__args__ is None. + # In Python 3.9 typ.__args__ does not exist when the generic type does not have subscripts type_args = typ.__args__ if getattr( typ, '__args__', None) is not None else (Any, Any) @@ -96,7 +94,7 @@ def verify_object_against_type(x: Any, typ: Type[T]) -> T: raise TypeError( 'Error: Object "{}" is incompatible with type "{}"'.format( x, typ)) - # In Python <3.7 Mapping.__args__ is None. + # In Python 3.9 typ.__args__ does not exist when the generic type does not have subscripts type_args = typ.__args__ if getattr( typ, '__args__', None) is not None else (Any, Any) @@ -157,9 +155,7 @@ def parse_object_from_struct_based_on_type(struct: Any, typ: Type[T]) -> T: possible_types = list(getattr(typ, '__args__', [Any])) #if type(None) in possible_types and struct is None: #Shortcut for Optional[] tests. Can be removed, but the exceptions will be more noisy. # return None - #Hack for Python <3.7 which for some reason "simplifies" Union[bool, int, ...] to just Union[int, ...] - if int in possible_types: - possible_types = possible_types + [bool] + for possible_type in possible_types: try: obj = parse_object_from_struct_based_on_type( @@ -194,10 +190,7 @@ def parse_object_from_struct_based_on_type(struct: Any, typ: Type[T]) -> T: 'Error: None structure is incompatible with type {}'.format( typ)) - #assert isinstance(x, typ.__origin__) - generic_type = typ.__origin__ or getattr( - typ, '__extra__', None - ) #In python <3.7 typing.List.__origin__ == None; Python 3.7 has working __origin__, but no __extra__ TODO: Remove the __extra__ once we move to Python 3.7 + generic_type = typ.__origin__ if generic_type in [ list, List, abc.Sequence, abc.MutableSequence, Sequence, MutableSequence @@ -206,7 +199,7 @@ def parse_object_from_struct_based_on_type(struct: Any, typ: Type[T]) -> T: raise TypeError( 'Error: Structure "{}" is incompatible with type "{}" - it does not have list type.' .format(struct, typ)) - # In Python <3.7 Mapping.__args__ is None. + # In Python 3.9 typ.__args__ does not exist when the generic type does not have subscripts type_args = typ.__args__ if getattr( typ, '__args__', None) is not None else (Any, Any) @@ -219,12 +212,12 @@ def parse_object_from_struct_based_on_type(struct: Any, typ: Type[T]) -> T: elif generic_type in [ dict, Dict, abc.Mapping, abc.MutableMapping, Mapping, MutableMapping, OrderedDict - ]: #in Python <3.7 there is a difference between abc.Mapping and typing.Mapping + ]: if not isinstance(struct, generic_type): raise TypeError( 'Error: Structure "{}" is incompatible with type "{}" - it does not have dict type.' .format(struct, typ)) - # In Python <3.7 Mapping.__args__ is None. + # In Python 3.9 typ.__args__ does not exist when the generic type does not have subscripts type_args = typ.__args__ if getattr( typ, '__args__', None) is not None else (Any, Any) From e7ccf2810e8c88456f54f7df0e2cec8582b7d911 Mon Sep 17 00:00:00 2001 From: connor-mccarthy Date: Fri, 18 Mar 2022 12:13:34 -0600 Subject: [PATCH 2/8] update test images to greatest supported python version --- sdk/python/kfp/compiler/compiler_test.py | 2 +- .../test_data/pipeline_with_task_final_status_yaml.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/sdk/python/kfp/compiler/compiler_test.py b/sdk/python/kfp/compiler/compiler_test.py index c6926f1a96f..695a4223be1 100644 --- a/sdk/python/kfp/compiler/compiler_test.py +++ b/sdk/python/kfp/compiler/compiler_test.py @@ -553,7 +553,7 @@ def test_use_task_final_status_in_non_exit_op_yaml(self): - {name: message, type: PipelineTaskFinalStatus} implementation: container: - image: python:3.7 + image: python:3.9 command: - echo - {inputValue: message} diff --git a/sdk/python/kfp/compiler_cli_tests/test_data/pipeline_with_task_final_status_yaml.py b/sdk/python/kfp/compiler_cli_tests/test_data/pipeline_with_task_final_status_yaml.py index 1e23fdeab62..0ad48c87478 100644 --- a/sdk/python/kfp/compiler_cli_tests/test_data/pipeline_with_task_final_status_yaml.py +++ b/sdk/python/kfp/compiler_cli_tests/test_data/pipeline_with_task_final_status_yaml.py @@ -25,7 +25,7 @@ - {name: status, type: PipelineTaskFinalStatus} implementation: container: - image: python:3.7 + image: python:3.9 command: - echo - "user input:" @@ -40,7 +40,7 @@ - {name: message, type: String} implementation: container: - image: python:3.7 + image: python:3.9 command: - echo - {inputValue: message} From 567e06765e3d2a1706932a4a9c74323e9416f2fa Mon Sep 17 00:00:00 2001 From: connor-mccarthy Date: Fri, 18 Mar 2022 12:13:49 -0600 Subject: [PATCH 3/8] remove unused import --- sdk/python/kfp/compiler/compiler_test.py | 1 - 1 file changed, 1 deletion(-) diff --git a/sdk/python/kfp/compiler/compiler_test.py b/sdk/python/kfp/compiler/compiler_test.py index 695a4223be1..3039788aac3 100644 --- a/sdk/python/kfp/compiler/compiler_test.py +++ b/sdk/python/kfp/compiler/compiler_test.py @@ -15,7 +15,6 @@ import json import os import shutil -import sys import tempfile import unittest From cbbf5d821fcad99c2ad66ad8bda84ff10d54477a Mon Sep 17 00:00:00 2001 From: connor-mccarthy Date: Fri, 18 Mar 2022 13:43:11 -0600 Subject: [PATCH 4/8] update test data --- .../test_data/pipeline_with_task_final_status_yaml.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/python/kfp/compiler_cli_tests/test_data/pipeline_with_task_final_status_yaml.json b/sdk/python/kfp/compiler_cli_tests/test_data/pipeline_with_task_final_status_yaml.json index 9de2f482df5..2b720865ee2 100644 --- a/sdk/python/kfp/compiler_cli_tests/test_data/pipeline_with_task_final_status_yaml.json +++ b/sdk/python/kfp/compiler_cli_tests/test_data/pipeline_with_task_final_status_yaml.json @@ -66,7 +66,7 @@ "pipeline status:", "{{$.inputs.parameters['status']}}" ], - "image": "python:3.7" + "image": "python:3.9" } }, "exec-print-op": { @@ -75,7 +75,7 @@ "echo", "{{$.inputs.parameters['message']}}" ], - "image": "python:3.7" + "image": "python:3.9" } } } From 7526829813e9a2b2fee782c7e72cb9c4d446bcc0 Mon Sep 17 00:00:00 2001 From: connor-mccarthy Date: Fri, 18 Mar 2022 14:20:29 -0600 Subject: [PATCH 5/8] clean comment --- sdk/python/kfp/components/v1_modelbase.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sdk/python/kfp/components/v1_modelbase.py b/sdk/python/kfp/components/v1_modelbase.py index b2cc3ee4638..c20017bb4e6 100644 --- a/sdk/python/kfp/components/v1_modelbase.py +++ b/sdk/python/kfp/components/v1_modelbase.py @@ -11,8 +11,7 @@ # 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. -# mypy: ignore-errors -# pylint: disable=all + import inspect from collections import OrderedDict from collections import abc From 6e8e1ad2b59825fe3638fb29f9d812fd6d7fc108 Mon Sep 17 00:00:00 2001 From: connor-mccarthy Date: Fri, 18 Mar 2022 15:22:35 -0600 Subject: [PATCH 6/8] undo 3.7 image change --- sdk/python/kfp/compiler/compiler_test.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sdk/python/kfp/compiler/compiler_test.py b/sdk/python/kfp/compiler/compiler_test.py index 3039788aac3..42649752bbc 100644 --- a/sdk/python/kfp/compiler/compiler_test.py +++ b/sdk/python/kfp/compiler/compiler_test.py @@ -15,6 +15,7 @@ import json import os import shutil +import sys import tempfile import unittest @@ -552,7 +553,7 @@ def test_use_task_final_status_in_non_exit_op_yaml(self): - {name: message, type: PipelineTaskFinalStatus} implementation: container: - image: python:3.9 + image: python:3.7 command: - echo - {inputValue: message} @@ -571,9 +572,8 @@ def my_pipeline(text: bool): # pylint: disable=import-outside-toplevel,unused-import,import-error,redefined-outer-name,reimported class V2NamespaceAliasTest(unittest.TestCase): - """Test that imports of both modules and objects are aliased - (e.g. all import path variants work). - """ + """Test that imports of both modules and objects are aliased (e.g. all + import path variants work).""" # Note: The DeprecationWarning is only raised on the first import where # the kfp.v2 module is loaded. Due to the way we run tests in CI/CD, we cannot ensure that the kfp.v2 module will first be loaded in these tests, From 2838a96b042f75872b09d54293d4242eba7771e1 Mon Sep 17 00:00:00 2001 From: Connor McCarthy Date: Fri, 18 Mar 2022 17:50:41 -0600 Subject: [PATCH 7/8] revert 3.9 -> 3.7 --- .../test_data/pipeline_with_task_final_status_yaml.json | 4 ++-- .../test_data/pipeline_with_task_final_status_yaml.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/sdk/python/kfp/compiler_cli_tests/test_data/pipeline_with_task_final_status_yaml.json b/sdk/python/kfp/compiler_cli_tests/test_data/pipeline_with_task_final_status_yaml.json index 2b720865ee2..9de2f482df5 100644 --- a/sdk/python/kfp/compiler_cli_tests/test_data/pipeline_with_task_final_status_yaml.json +++ b/sdk/python/kfp/compiler_cli_tests/test_data/pipeline_with_task_final_status_yaml.json @@ -66,7 +66,7 @@ "pipeline status:", "{{$.inputs.parameters['status']}}" ], - "image": "python:3.9" + "image": "python:3.7" } }, "exec-print-op": { @@ -75,7 +75,7 @@ "echo", "{{$.inputs.parameters['message']}}" ], - "image": "python:3.9" + "image": "python:3.7" } } } diff --git a/sdk/python/kfp/compiler_cli_tests/test_data/pipeline_with_task_final_status_yaml.py b/sdk/python/kfp/compiler_cli_tests/test_data/pipeline_with_task_final_status_yaml.py index 0ad48c87478..1e23fdeab62 100644 --- a/sdk/python/kfp/compiler_cli_tests/test_data/pipeline_with_task_final_status_yaml.py +++ b/sdk/python/kfp/compiler_cli_tests/test_data/pipeline_with_task_final_status_yaml.py @@ -25,7 +25,7 @@ - {name: status, type: PipelineTaskFinalStatus} implementation: container: - image: python:3.9 + image: python:3.7 command: - echo - "user input:" @@ -40,7 +40,7 @@ - {name: message, type: String} implementation: container: - image: python:3.9 + image: python:3.7 command: - echo - {inputValue: message} From b72d22f72d1c974ba564049d8065b77183910201 Mon Sep 17 00:00:00 2001 From: connor-mccarthy Date: Fri, 18 Mar 2022 17:57:13 -0600 Subject: [PATCH 8/8] revert OrderedDict to Dict --- sdk/python/kfp/components/structures.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sdk/python/kfp/components/structures.py b/sdk/python/kfp/components/structures.py index 70398b35b41..14fe3b0e4a1 100644 --- a/sdk/python/kfp/components/structures.py +++ b/sdk/python/kfp/components/structures.py @@ -16,7 +16,7 @@ import dataclasses import itertools import json -from typing import Any, Dict, Mapping, Optional, OrderedDict, Sequence, Union +from typing import Any, Dict, Mapping, Optional, Sequence, Union import pydantic import yaml @@ -307,8 +307,8 @@ class ComponentSpec(BaseModel): """ name: str description: Optional[str] = None - inputs: Optional[OrderedDict[str, InputSpec]] = None - outputs: Optional[OrderedDict[str, OutputSpec]] = None + inputs: Optional[Dict[str, InputSpec]] = None + outputs: Optional[Dict[str, OutputSpec]] = None implementation: Implementation @pydantic.validator('inputs', 'outputs', allow_reuse=True)