claudevdm commented on code in PR #41184:
URL: https://github.com/apache/beam/pull/41184#discussion_r4539487289


##########
sdks/python/apache_beam/typehints/decorators.py:
##########
@@ -175,6 +175,35 @@ def _annotation_namespaces(fn):
   return getattr(target, '__globals__', {}), localns
 
 
+def _is_valid_type_hint(hint):
+  """Whether a resolved annotation only contains valid type hint components.
+
+  A string annotation can resolve to an object that is not a type, e.g. a
+  protobuf enum (an ``EnumTypeWrapper`` instance) used as
+  ``Mapping[str, 'schema_pb2.FeatureType']``. Beam's composite type hints
+  reject such parameters, so these annotations must not be resolved.
+  """
+  if hint is None or hint is Ellipsis:
+    return True
+  if isinstance(hint, (list, tuple)):
+    # Callable parameter lists, e.g. the [int] in Callable[[int], str].
+    return all(_is_valid_type_hint(h) for h in hint)
+  origin = get_origin(hint)
+  if origin is Literal:
+    # Literal arguments are values, not types.
+    return True
+  args = get_args(hint)
+  if origin is typing.Annotated:
+    args = args[:1]  # The remaining arguments are metadata.
+  if args:
+    return all(_is_valid_type_hint(arg) for arg in args)
+  try:

Review Comment:
   `validate_composite_type_param` rejects TypeVars, NewTypes and `type` 
aliases, so any string annotation using one now falls back to `Any`:
   
   ```python
   from __future__ import annotations
   import typing
   import apache_beam as beam
   
   T = typing.TypeVar('T')
   
   def first(xs: list[T]) -> T:
     return xs[0]
   
   p = beam.Pipeline()
   print((p | beam.Create([[1]]) | beam.Map(first)).element_type)
   # master: <class 'int'>    this PR: Any
   ```
   
   Could we validate the converted hint instead? The enum case still falls 
back, since a non-type comes back from `convert_to_beam_type` unchanged. A test 
like the repro above would be good to add too.
   
   ```suggestion
     try:
       validate_composite_type_param(
           convert_to_beam_type(hint), 'Resolved type annotation')
     except (TypeError, ValueError):
       return False
   ```
   



-- 
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]

Reply via email to