[GitHub] [beam] lostluck commented on a change in pull request #12393: Support creation of empty PCollection in beam CreateList

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

lostluck commented on a change in pull request #12393:
URL: https://github.com/apache/beam/pull/12393#discussion_r462422357



##########
File path: sdks/go/pkg/beam/create.go
##########
@@ -34,31 +34,44 @@ func Create(s Scope, values ...interface{}) PCollection {
 }
 
 // CreateList inserts a fixed set of values into the pipeline from a slice or
-// array. It is a convenience wrapper over Create.
+// array. Unlike Create this supports the creation of an empty PCollection.
 func CreateList(s Scope, list interface{}) PCollection {
-	var ret []interface{}
 	val := reflect.ValueOf(list)
 	if val.Kind() != reflect.Slice && val.Kind() != reflect.Array {
 		panic(fmt.Sprintf("Input %v must be a slice or array", list))
 	}
+	var ret []interface{}
 	for i := 0; i < val.Len(); i++ {
 		ret = append(ret, val.Index(i).Interface())
 	}
-	return Must(TryCreate(s, ret...))
+	var t reflect.Type
+	if len(ret) == 0 {
+		t = reflect.TypeOf(list).Elem()
+	} else {
+		t = reflect.ValueOf(ret[0]).Type()
+	}
+	return Must(TryCreateList(s, ret, t))
 }
 
 func addCreateCtx(err error, s Scope) error {
 	return errors.WithContextf(err, "inserting Create in scope %s", s)
 }
 
-// TryCreate inserts a fixed set of values into the pipeline. The values must
-// be of the same type.
+// TryCreate inserts a fixed non-empty set of values into the pipeline. The
+// values must be of the same type.
 func TryCreate(s Scope, values ...interface{}) (PCollection, error) {
 	if len(values) == 0 {
 		return PCollection{}, addCreateCtx(errors.New("create has no values"), s)
 	}
 
 	t := reflect.ValueOf(values[0]).Type()
+	return TryCreateList(s, values, t)
+}
+
+// TryCreateList inserts a fixed set of values into the pipeline from a slice or
+// array. The values must be of the same type. Unlike TryCreate this supports
+// the creation of an empty PCollection.
+func TryCreateList(s Scope, values []interface{}, t reflect.Type) (PCollection, error) {

Review comment:
       TryCreateList and CreateList should only differ in returning an error, and not force different parameters.
   
   That said, for convenience it's totally reasonable for an unexported tryCreateList to have this signature for reuse as you've done, and have the exported TryCreateList infer from the raw empty list type.
   




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