jrmccluskey commented on code in PR #25054:
URL: https://github.com/apache/beam/pull/25054#discussion_r1081423204
##########
sdks/python/apache_beam/typehints/typehints_test.py:
##########
@@ -525,14 +525,13 @@ def
test_type_check_invalid_composite_type_arbitrary_length(self):
def test_normalize_with_builtin_tuple(self):
if sys.version_info >= (3, 9):
- with self.assertRaises(TypeError) as e:
- typehints.normalize(tuple[int, int], False)
+ expected_beam_type = typehints.Tuple[int, int]
+ converted_beam_type = typehints.normalize(tuple[int, int], False)
+ self.assertEqual(converted_beam_type, expected_beam_type)
- self.assertEqual(
- 'PEP 585 generic type hints like tuple[int, int] are not yet '
- 'supported, use typing module containers instead. See equivalents '
- 'listed at https://docs.python.org/3/library/typing.html',
- e.exception.args[0])
+ def test_builtin_and_type_compatibility(self):
+ self.assertCompatible(tuple, typing.Tuple)
+ self.assertCompatible(tuple[int, int], typing.Tuple[int, int])
Review Comment:
```suggestion
def test_builtin_and_type_compatibility(self):
if sys.version_info >= (3, 9):
self.assertCompatible(tuple, typing.Tuple)
self.assertCompatible(tuple[int, int], typing.Tuple[int, int])
```
##########
sdks/python/apache_beam/typehints/typehints_test.py:
##########
@@ -741,14 +739,15 @@ def test_match_type_variables(self):
def test_normalize_with_builtin_dict(self):
if sys.version_info >= (3, 9):
- with self.assertRaises(TypeError) as e:
- typehints.normalize(dict[int, str], False)
+ expected_beam_type = typehints.Dict[str, int]
+ converted_beam_type = typehints.normalize(dict[str, int], False)
+ self.assertEqual(converted_beam_type, expected_beam_type)
- self.assertEqual(
- 'PEP 585 generic type hints like dict[int, str] are not yet '
- 'supported, use typing module containers instead. See equivalents '
- 'listed at https://docs.python.org/3/library/typing.html',
- e.exception.args[0])
+ def test_builtin_and_type_compatibility(self):
+ self.assertCompatible(dict, typing.Dict)
+ self.assertCompatible(dict[str, int], typing.Dict[str, int])
+ self.assertCompatible(
+ dict[str, list[int]], typing.Dict[str, typing.List[int]])
Review Comment:
```suggestion
def test_builtin_and_type_compatibility(self):
if sys.version_info >= (3, 9):
self.assertCompatible(dict, typing.Dict)
self.assertCompatible(dict[str, int], typing.Dict[str, int])
self.assertCompatible(
dict[str, list[int]], typing.Dict[str, typing.List[int]])
```
##########
sdks/python/apache_beam/typehints/typehints.py:
##########
@@ -1186,20 +1186,18 @@ def __getitem__(self, type_params):
def normalize(x, none_as_type=False):
# None is inconsistantly used for Any, unknown, or NoneType.
+
+ # Avoid circular imports
+ from apache_beam.typehints import native_type_compatibility
+
+ if sys.version_info >= (3, 9) and isinstance(type, types.GenericAlias):
+ x = native_type_compatibility.convert_builtin_to_typing(x)
Review Comment:
Based on the pre-commit failures it seems like this conversion isn't
happening
##########
sdks/python/apache_beam/typehints/typehints_test.py:
##########
@@ -595,14 +594,13 @@ def
test_enforce_list_type_constraint_invalid_composite_type(self):
def test_normalize_with_builtin_list(self):
if sys.version_info >= (3, 9):
- with self.assertRaises(TypeError) as e:
- typehints.normalize(list[int], False)
+ expected_beam_type = typehints.List[int]
+ converted_beam_type = typehints.normalize(list[int], False)
+ self.assertEqual(converted_beam_type, expected_beam_type)
- self.assertEqual(
- 'PEP 585 generic type hints like list[int] are not yet supported, '
- 'use typing module containers instead. See equivalents listed '
- 'at https://docs.python.org/3/library/typing.html',
- e.exception.args[0])
+ def test_builtin_and_type_compatibility(self):
+ self.assertCompatible(list, typing.List)
+ self.assertCompatible(list[int], typing.List[int])
Review Comment:
```suggestion
def test_builtin_and_type_compatibility(self):
if sys.version_info >= (3, 9):
self.assertCompatible(list, typing.List)
self.assertCompatible(list[int], typing.List[int])
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
