[GitHub] [beam] udim commented on a change in pull request #12009: [BEAM-10258] Support type hint annotations on PTransform's expand()

[GitBox <gi...@apache.org>]

udim commented on a change in pull request #12009:
URL: https://github.com/apache/beam/pull/12009#discussion_r441005641



##########
File path: sdks/python/apache_beam/transforms/ptransform.py
##########
@@ -72,7 +72,7 @@ class and wrapper class that allows lambda functions to be used as
 from apache_beam.transforms.display import HasDisplayData
 from apache_beam.typehints import native_type_compatibility
 from apache_beam.typehints import typehints
-from apache_beam.typehints.decorators import TypeCheckError
+from apache_beam.typehints.decorators import TypeCheckError, IOTypeHints, get_type_hints

Review comment:
       Please stay consistent with existing code: one import per line.

##########
File path: sdks/python/apache_beam/pvalue.py
##########
@@ -158,6 +159,20 @@ def __ne__(self, other):
   def __hash__(self):
     return hash((self.tag, self.producer))
 
+  class PCollectionTypeConstraint(SequenceTypeConstraint):

Review comment:
       I would really prefer it if all type constraint classes were in the same module (typehints).

##########
File path: sdks/python/apache_beam/pvalue.py
##########
@@ -141,7 +142,7 @@ def __or__(self, ptransform):
     return self.pipeline.apply(ptransform, self)
 
 
-class PCollection(PValue, Generic[T]):
+class PCollection(PValue, CompositeTypeHint):

Review comment:
       This should stay `Generic[T]`, as it provides the same `[]` functionality but is understood by tools like mypy and is familiar to Python users.

##########
File path: sdks/python/apache_beam/pvalue.py
##########
@@ -425,8 +441,7 @@ def _from_runtime_iterable(it, options):
 
   def _view_options(self):
     return {
-        'data': self._data,
-        # For non-fn-api runners.
+        'data': self._data,  # For non-fn-api runners.

Review comment:
       Are these whitespace changes coming from yapf? This particularly seems to have moved the comment to the line above it.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org