TheNeuralBit commented on a change in pull request #14170:
URL: https://github.com/apache/beam/pull/14170#discussion_r589888530



##########
File path: sdks/python/apache_beam/dataframe/convert_test.py
##########
@@ -92,6 +92,11 @@ def test_convert_scalar(self):
       pc_sum = convert.to_pcollection(s.sum())
       assert_that(pc_sum, equal_to([6]))
 
+  def test_convert_non_deferred(self):
+    with beam.Pipeline() as p:
+      pc_sum = convert.to_pcollection(pd.Series([1, 2, 3]), pipeline=p)
+      assert_that(pc_sum, equal_to([1, 2, 3]))
+

Review comment:
       Is it worth adding some sanity checks for mixed cases?

##########
File path: sdks/python/apache_beam/dataframe/convert.py
##########
@@ -123,6 +129,8 @@ def to_pcollection(
   pass them all at the same time to this method.
 
   Args:
+    label: (optional, default "ToPCollection(...)"") the label to use for the
+        conversion transform.

Review comment:
       Could you also update the docstring to note this works with non-deferred 
dataframes, and update the typehint on `dataframes`




----------------------------------------------------------------
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:
[email protected]


Reply via email to