From dc830441559eb4a863a547689228f559926aa257 Mon Sep 17 00:00:00 2001 From: Jack McCluskey Date: Tue, 24 Jan 2023 19:58:27 +0000 Subject: [PATCH 1/3] Update typehint code to allow primitive composite types in python 3.9+ --- .../native_type_compatibility_test.py | 4 +++ .../python/apache_beam/typehints/typehints.py | 5 ++-- .../apache_beam/typehints/typehints_test.py | 27 ++++++++++--------- 3 files changed, 22 insertions(+), 14 deletions(-) diff --git a/sdks/python/apache_beam/typehints/native_type_compatibility_test.py b/sdks/python/apache_beam/typehints/native_type_compatibility_test.py index 8dcff9fc2d7d..36dc853067c9 100644 --- a/sdks/python/apache_beam/typehints/native_type_compatibility_test.py +++ b/sdks/python/apache_beam/typehints/native_type_compatibility_test.py @@ -123,6 +123,10 @@ def test_convert_to_beam_type_with_builtin_types(self): 'nested builtin', dict[str, list[tuple[float]]], typehints.Dict[str, typehints.List[typehints.Tuple[float]]]), + ( + 'builtin nested tuple', + tuple[str, list], + typehints.Tuple[str, typehints.List[typehints.Any]],) ] for test_case in test_cases: diff --git a/sdks/python/apache_beam/typehints/typehints.py b/sdks/python/apache_beam/typehints/typehints.py index 71d56ae4b4f9..39a166b048df 100644 --- a/sdks/python/apache_beam/typehints/typehints.py +++ b/sdks/python/apache_beam/typehints/typehints.py @@ -389,10 +389,11 @@ def validate_composite_type_param(type_param, error_msg_prefix): if sys.version_info.major == 3 and sys.version_info.minor >= 10: if isinstance(type_param, types.UnionType): is_not_type_constraint = False - is_forbidden_type = ( + if sys.version_info.major == 3 and sys.version_info.minor < 9: + is_not_type_constraint = is_not_type_constraint or ( isinstance(type_param, type) and type_param in DISALLOWED_PRIMITIVE_TYPES) - if is_not_type_constraint or is_forbidden_type: + if is_not_type_constraint: raise TypeError( '%s must be a non-sequence, a type, or a TypeConstraint. %s' ' is an instance of %s.' % diff --git a/sdks/python/apache_beam/typehints/typehints_test.py b/sdks/python/apache_beam/typehints/typehints_test.py index d8893a3f474a..1f49440eb037 100644 --- a/sdks/python/apache_beam/typehints/typehints_test.py +++ b/sdks/python/apache_beam/typehints/typehints_test.py @@ -379,9 +379,10 @@ def test_getitem_params_must_be_type_or_constraint(self): typehints.Tuple[5, [1, 3]] self.assertTrue(e.exception.args[0].startswith(expected_error_prefix)) - with self.assertRaises(TypeError) as e: - typehints.Tuple[list, dict] - self.assertTrue(e.exception.args[0].startswith(expected_error_prefix)) + if sys.version_info < (3, 9): + with self.assertRaises(TypeError) as e: + typehints.Tuple[list, dict] + self.assertTrue(e.exception.args[0].startswith(expected_error_prefix)) def test_compatibility_arbitrary_length(self): self.assertNotCompatible( @@ -665,8 +666,9 @@ def test_getitem_param_must_have_length_2(self): e.exception.args[0]) def test_key_type_must_be_valid_composite_param(self): - with self.assertRaises(TypeError): - typehints.Dict[list, int] + if sys.version_info < (3, 9): + with self.assertRaises(TypeError): + typehints.Dict[list, int] def test_value_type_must_be_valid_composite_param(self): with self.assertRaises(TypeError): @@ -765,13 +767,14 @@ def test_builtin_and_type_compatibility(self): class BaseSetHintTest: class CommonTests(TypeHintTestCase): def test_getitem_invalid_composite_type_param(self): - with self.assertRaises(TypeError) as e: - self.beam_type[list] - self.assertEqual( - "Parameter to a {} hint must be a non-sequence, a " - "type, or a TypeConstraint. {} is an instance of " - "type.".format(self.string_type, list), - e.exception.args[0]) + if sys.version_info < (3, 9): + with self.assertRaises(TypeError) as e: + self.beam_type[list] + self.assertEqual( + "Parameter to a {} hint must be a non-sequence, a " + "type, or a TypeConstraint. {} is an instance of " + "type.".format(self.string_type, list), + e.exception.args[0]) def test_compatibility(self): hint1 = self.beam_type[typehints.List[str]] From fbbcb885f7a86b3bd5acfa8bdec9dd19e2312f10 Mon Sep 17 00:00:00 2001 From: jrmccluskey Date: Tue, 24 Jan 2023 20:01:29 +0000 Subject: [PATCH 2/3] linting --- .../native_type_compatibility_test.py | 28 +++++++++---------- .../python/apache_beam/typehints/typehints.py | 3 +- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/sdks/python/apache_beam/typehints/native_type_compatibility_test.py b/sdks/python/apache_beam/typehints/native_type_compatibility_test.py index 36dc853067c9..e7b69932a08e 100644 --- a/sdks/python/apache_beam/typehints/native_type_compatibility_test.py +++ b/sdks/python/apache_beam/typehints/native_type_compatibility_test.py @@ -114,20 +114,20 @@ def test_convert_to_beam_type(self): def test_convert_to_beam_type_with_builtin_types(self): if sys.version_info >= (3, 9): - test_cases = [ - ('builtin dict', dict[str, int], typehints.Dict[str, int]), - ('builtin list', list[str], typehints.List[str]), - ('builtin tuple', tuple[str], typehints.Tuple[str]), - ('builtin set', set[str], typehints.Set[str]), - ( - 'nested builtin', - dict[str, list[tuple[float]]], - typehints.Dict[str, typehints.List[typehints.Tuple[float]]]), - ( - 'builtin nested tuple', - tuple[str, list], - typehints.Tuple[str, typehints.List[typehints.Any]],) - ] + test_cases = [('builtin dict', dict[str, int], typehints.Dict[str, int]), + ('builtin list', list[str], typehints.List[str]), + ('builtin tuple', tuple[str], typehints.Tuple[str]), + ('builtin set', set[str], typehints.Set[str]), + ( + 'nested builtin', + dict[str, list[tuple[float]]], + typehints.Dict[str, + typehints.List[typehints.Tuple[float]]]), + ( + 'builtin nested tuple', + tuple[str, list], + typehints.Tuple[str, typehints.List[typehints.Any]], + )] for test_case in test_cases: description = test_case[0] diff --git a/sdks/python/apache_beam/typehints/typehints.py b/sdks/python/apache_beam/typehints/typehints.py index 39a166b048df..b6b1574a0edb 100644 --- a/sdks/python/apache_beam/typehints/typehints.py +++ b/sdks/python/apache_beam/typehints/typehints.py @@ -391,7 +391,8 @@ def validate_composite_type_param(type_param, error_msg_prefix): is_not_type_constraint = False if sys.version_info.major == 3 and sys.version_info.minor < 9: is_not_type_constraint = is_not_type_constraint or ( - isinstance(type_param, type) and type_param in DISALLOWED_PRIMITIVE_TYPES) + isinstance(type_param, type) and + type_param in DISALLOWED_PRIMITIVE_TYPES) if is_not_type_constraint: raise TypeError( From 0de90b4468bd66ce22522b5c3f01a75b1b40f832 Mon Sep 17 00:00:00 2001 From: jrmccluskey Date: Wed, 25 Jan 2023 15:30:46 +0000 Subject: [PATCH 3/3] address comments --- sdks/python/apache_beam/typehints/typehints.py | 2 ++ .../apache_beam/typehints/typehints_test.py | 15 +++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/sdks/python/apache_beam/typehints/typehints.py b/sdks/python/apache_beam/typehints/typehints.py index b6b1574a0edb..7ea05f27cc87 100644 --- a/sdks/python/apache_beam/typehints/typehints.py +++ b/sdks/python/apache_beam/typehints/typehints.py @@ -389,6 +389,8 @@ def validate_composite_type_param(type_param, error_msg_prefix): if sys.version_info.major == 3 and sys.version_info.minor >= 10: if isinstance(type_param, types.UnionType): is_not_type_constraint = False + # Pre-Python 3.9 compositve type-hinting with built-in types was not + # supported, the typing module equivalents should be used instead. if sys.version_info.major == 3 and sys.version_info.minor < 9: is_not_type_constraint = is_not_type_constraint or ( isinstance(type_param, type) and diff --git a/sdks/python/apache_beam/typehints/typehints_test.py b/sdks/python/apache_beam/typehints/typehints_test.py index 1f49440eb037..1f6b98eccbea 100644 --- a/sdks/python/apache_beam/typehints/typehints_test.py +++ b/sdks/python/apache_beam/typehints/typehints_test.py @@ -383,6 +383,11 @@ def test_getitem_params_must_be_type_or_constraint(self): with self.assertRaises(TypeError) as e: typehints.Tuple[list, dict] self.assertTrue(e.exception.args[0].startswith(expected_error_prefix)) + else: + try: + typehints.Tuple[list, dict] + except TypeError: + self.fail("built-in composite raised TypeError unexpectedly") def test_compatibility_arbitrary_length(self): self.assertNotCompatible( @@ -669,6 +674,11 @@ def test_key_type_must_be_valid_composite_param(self): if sys.version_info < (3, 9): with self.assertRaises(TypeError): typehints.Dict[list, int] + else: + try: + typehints.Tuple[list, int] + except TypeError: + self.fail("built-in composite raised TypeError unexpectedly") def test_value_type_must_be_valid_composite_param(self): with self.assertRaises(TypeError): @@ -775,6 +785,11 @@ def test_getitem_invalid_composite_type_param(self): "type, or a TypeConstraint. {} is an instance of " "type.".format(self.string_type, list), e.exception.args[0]) + else: + try: + self.beam_type[list] + except TypeError: + self.fail("built-in composite raised TypeError unexpectedly") def test_compatibility(self): hint1 = self.beam_type[typehints.List[str]]