[ 
https://issues.apache.org/jira/browse/BEAM-6386?focusedWorklogId=188199&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-188199
 ]

ASF GitHub Bot logged work on BEAM-6386:
----------------------------------------

                Author: ASF GitHub Bot
            Created on: 22/Jan/19 14:12
            Start Date: 22/Jan/19 14:12
    Worklog Time Spent: 10m 
      Work Description: jklukas commented on pull request #7437: [BEAM-6386] 
Add named variant of PTransform::compose()
URL: https://github.com/apache/beam/pull/7437#discussion_r249794108
 
 

 ##########
 File path: 
sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/PTransform.java
 ##########
 @@ -319,4 +319,16 @@ public OutputT expand(InputT input) {
       }
     };
   }
+
+  /** Like {@link #compose(SerializableFunction)}, but with a custom name. */
+  @Experimental
+  public static <InputT extends PInput, OutputT extends POutput>
 
 Review comment:
   The above example is perhaps too complex and specific to justify adding a 
new signature to the API, so let me take a step back.
   
   This signature would be useful in any case where you want to return a simple 
transform from a method. A user might want to make a simple library of 
transforms that can be reused in different pipelines. In such a case, it's 
convenient to be able to give the transform you're returning a custom name.
   
   So, a simpler example:
   
   ```
   public class MyTransforms extends PTransform<...> {
     public static PTransform<...> transform1() {
       return PTransform.compose("MyTransforms.transform1", ...)
     }
   }
   ```
   
   It seems better to repeat the name once where we define the composite 
transform rather than having to repeat the name every time we use it in a 
pipeline:
   
   ```
   input1.apply(MyTransforms.transform1())...
   
   input2.apply(MyTransforms.transform1())...
   ```
   
   It's a better experience for the end pipeline developer if the composite 
transform has a name rather than displaying as "AnonymousTransform", so seems 
worth it to me to add an additional signature here to encourage the practice. 
Perhaps the Javadoc should provide an example like the above and discuss that 
the name specified here is a fallback in case `apply` is called without an 
explicit name.
   
   If, after the above discussion, you still feel iffy about it, I'm perfectly 
happy to go ahead and close this. I appreciate your review and thoughts.
 
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Issue Time Tracking
-------------------

    Worklog Id:     (was: 188199)
    Time Spent: 40m  (was: 0.5h)

> Add named variant of PTransform::compose
> ----------------------------------------
>
>                 Key: BEAM-6386
>                 URL: https://issues.apache.org/jira/browse/BEAM-6386
>             Project: Beam
>          Issue Type: Improvement
>          Components: sdk-java-core
>            Reporter: Jeff Klukas
>            Assignee: Jeff Klukas
>            Priority: Minor
>          Time Spent: 40m
>  Remaining Estimate: 0h
>
> BEAM- 5413 introduced PTransform::compose as a concise way of creating a 
> composition of transforms as a lambda. We should add a variant to allow 
> specifying a name for the returned transform in the same way that {{apply}} 
> can take an explicit name.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to