[GitHub] [beam] robertwb commented on a change in pull request #14160: [BEAM-11357] Add the ability to declare annotations to Python.

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

robertwb commented on a change in pull request #14160:
URL: https://github.com/apache/beam/pull/14160#discussion_r589805315



##########
File path: sdks/python/apache_beam/pipeline.py
##########
@@ -1028,6 +1032,26 @@ def __init__(
     self.parts = []  # type: List[AppliedPTransform]
     self.environment_id = environment_id if environment_id else None  # type: Optional[str]
 
+    if annotations is None and transform:

Review comment:
       `transform` is None for the "root" transform of a pipeline.

##########
File path: sdks/python/apache_beam/pipeline_test.py
##########
@@ -924,6 +924,35 @@ def test_requirements(self):
         common_urns.requirements.REQUIRES_BUNDLE_FINALIZATION.urn,
         proto.requirements)
 
+  def test_annotations(self):
+    class EmptyTransform(beam.PTransform):
+      def expand(self, pcoll):
+        return pcoll
+
+      def annotations(self):
+        return {'foo': 'some_string'}
+
+    class NonEmptyTransform(beam.PTransform):
+      def expand(self, pcoll):
+        return pcoll | beam.Map(lambda x: x)
+
+      def annotations(self):
+        return {'foo': b'some_bytes'}
+
+    p = beam.Pipeline()
+    _ = p | beam.Create([]) | EmptyTransform() | NonEmptyTransform()

Review comment:
       Sure. Done.

##########
File path: sdks/python/apache_beam/pipeline.py
##########
@@ -1028,6 +1032,26 @@ def __init__(
     self.parts = []  # type: List[AppliedPTransform]
     self.environment_id = environment_id if environment_id else None  # type: Optional[str]
 
+    if annotations is None and transform:
+
+      def annotation_to_bytes(key, a: Any) -> bytes:
+        if isinstance(a, bytes):
+          return a
+        elif isinstance(a, str):
+          return a.encode('ascii')

Review comment:
       If it's ascii, there's no ambiguity as to how it should be encoded.




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