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

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

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



##########
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. Differently from Create this supports the creation of an empty
+// PCollection.
 func CreateList(s Scope, list interface{}) PCollection {
-	var ret []interface{}
 	val := reflect.ValueOf(list)
+	var ret []interface{}
+	for i := 0; i < val.Len(); i++ {
+		ret = append(ret, val.Index(i).Interface())
+	}
 	if val.Kind() != reflect.Slice && val.Kind() != reflect.Array {
 		panic(fmt.Sprintf("Input %v must be a slice or array", list))
 	}
-	for i := 0; i < val.Len(); i++ {
-		ret = append(ret, val.Index(i).Interface())
+	if val.Len() == 0 {
+		t := reflect.TypeOf(list).Elem()
+		return Must(TryCreateList(s, t, ret))
 	}
-	return Must(TryCreate(s, ret...))
+	t := reflect.ValueOf(ret[0]).Type()
+	return Must(TryCreateList(s, t, ret))
 }
 
 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, t, values)
+}
+
+// TryCreateList inserts a fixed set of values into the pipeline from a slice or
+// array. The values must be of the same type. Differently from TryCreate this

Review comment:
       Same suggestion as above:
   ```suggestion
   // array. The values must be of the same type. Unlike TryCreate this
   ```

##########
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. Differently from Create this supports the creation of an empty

Review comment:
       Optional, but how about this wording?:
   ```suggestion
   // array. Unlike Create this supports the creation of an empty
   ```

##########
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. Differently from Create this supports the creation of an empty
+// PCollection.
 func CreateList(s Scope, list interface{}) PCollection {
-	var ret []interface{}
 	val := reflect.ValueOf(list)
+	var ret []interface{}
+	for i := 0; i < val.Len(); i++ {

Review comment:
       It seems better to have the error check below happen before this loop (unless you have a specific reason for putting the loop first).




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