You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2020/08/27 21:07:58 UTC

[GitHub] [beam] TheNeuralBit opened a new pull request #12705: [BEAM-10720][WIP] Implement StringMethods

TheNeuralBit opened a new pull request #12705:
URL: https://github.com/apache/beam/pull/12705


   This PR adds an implementation for all parallelizable functionality in `pd.Series.str`. The vast majority of operations are trivially elementwise, but there are two exceptions:
   - [`cat`](https://pandas.pydata.org/pandas-docs/stable/reference/api/pandas.Series.str.cat.html) can accept another Series or DataFrame making it a zipping operation, or it may produce a scalar.
   - [`repeat`](https://pandas.pydata.org/pandas-docs/stable/reference/api/pandas.Series.str.repeat.html) can accept a finite list of times to repeat, which is order-sensitive.
   
   Post-Commit Tests Status (on master branch)
   ------------------------------------------------------------------------------------------------
   
   Lang | SDK | Dataflow | Flink | Samza | Spark | Twister2
   --- | --- | --- | --- | --- | --- | ---
   Go | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/) | --- | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/) | --- | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/) | ---
   Java | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/badge/i
 con)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)<br>[![Build Status](htt
 ps://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/)
   Python | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python38/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python38/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_
 Py_VR_Dataflow/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/) | --- | [![Build Status](https://ci-beam.apache.org/job/beam_P
 ostCommit_Python_VR_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/) | ---
   XLang | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Direct/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Direct/lastCompletedBuild/) | --- | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/) | --- | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/) | ---
   
   Pre-Commit Tests Status (on master branch)
   ------------------------------------------------------------------------------------------------
   
   --- |Java | Python | Go | Website | Whitespace
   --- | --- | --- | --- | --- | ---
   Non-portable | [![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_PythonDocker_Cron/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_PythonDocker_Cron/lastCompletedBuild/) <br>[![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_PythonDocs_Cron/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_PythonDocs_Cron/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/be
 am_PreCommit_Go_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_Whitespace_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Whitespace_Cron/lastCompletedBuild/)
   Portable | --- | [![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/) | --- | --- | ---
   
   See [.test-infra/jenkins/README](https://github.com/apache/beam/blob/master/.test-infra/jenkins/README.md) for trigger phrase, status and link of all Jenkins jobs.
   
   
   GitHub Actions Tests Status (on master branch)
   ------------------------------------------------------------------------------------------------
   ![Build python source distribution and wheels](https://github.com/apache/beam/workflows/Build%20python%20source%20distribution%20and%20wheels/badge.svg)
   ![Python tests](https://github.com/apache/beam/workflows/Python%20tests/badge.svg)
   ![Java tests](https://github.com/apache/beam/workflows/Java%20Tests/badge.svg)
   
   See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI.
   


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



[GitHub] [beam] robertwb commented on a change in pull request #12705: [BEAM-10720][WIP] Implement StringMethods

Posted by GitBox <gi...@apache.org>.
robertwb commented on a change in pull request #12705:
URL: https://github.com/apache/beam/pull/12705#discussion_r498571169



##########
File path: sdks/python/apache_beam/dataframe/frames.py
##########
@@ -905,7 +905,48 @@ def checked_callable_index(df):
             preserves_partition_by=partitionings.Singleton()))
 
 class _DeferredStringMethods(frame_base.DeferredBase):
-  pass
+  @frame_base.args_to_kwargs(pd.core.strings.StringMethods)
+  @frame_base.populate_defaults(pd.core.strings.StringMethods)
+  def cat(self, others, join, **kwargs):
+    if others is None:
+      # Concatenate series into a single String
+      requires = partitionings.Singleton()
+      func = lambda df: df.cat(join=join, **kwargs)
+      args = [self._expr]
+
+    elif isinstance(others, frame_base.DeferredBase) or \

Review comment:
       Use ()'s rather than backslashes for continuations. 

##########
File path: sdks/python/apache_beam/dataframe/transforms_test.py
##########
@@ -266,6 +266,17 @@ def check(actual):
           lambda x: {'res': 3 * x}, proxy)
       assert_that(res['res'], equal_to_series(three_series), 'CheckDictOut')
 
+  def test_cat(self):
+    # verify that cat works with a List[Series] sicne this is missing from doctests
+    df = pd.DataFrame({
+        'one': ['A', 'B', 'C'],
+        'two': ['BB', 'CC', 'A'],
+        'three': ['CCC', 'AA', 'B'],
+    })
+    self.run_scenario(df, lambda df: df.two.str.cat([df.three], join='outer'))
+    self.run_scenario(
+        df, lambda df: df.one.str.cat([df.two, df.three], join='outer'))
+

Review comment:
       I think this is because we're trying to partition the `StringMethods` object itself and send it across the stage/shuffle boundary, and that code only expects dataframes and series to be partition-able (expecting everything else to be a single-object scalar. 
   
   One route would be to add StringMethods to the list of things that are partitionable, but that feels a bit odd and won't work well with a columnar backend representation (e.g. how would we represent s.str[:] as an Arrow batch?) Better would be for these expressions to take the underlying series as an argument, and invoke `str` on it themselves as part of the function to be applied. This may take some refactoring of the elementwise operations. (It *may* be OK, as all the others get "fused away", similarly for DateTime, but we should probably add a check that we don't try to partition anything but frames).

##########
File path: sdks/python/apache_beam/dataframe/transforms_test.py
##########
@@ -266,6 +266,18 @@ def check(actual):
           lambda x: {'res': 3 * x}, proxy)
       assert_that(res['res'], equal_to_series(three_series), 'CheckDictOut')
 
+  def test_cat(self):
+    # verify that cat works with a List[Series] sicne this is

Review comment:
       since




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



[GitHub] [beam] robertwb commented on pull request #12705: [BEAM-10720] Finish implementing StringMethods (cat, repeat)

Posted by GitBox <gi...@apache.org>.
robertwb commented on pull request #12705:
URL: https://github.com/apache/beam/pull/12705#issuecomment-707903998


   (Modulo some lint issues that need fixing before merging.)


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



[GitHub] [beam] TheNeuralBit commented on a change in pull request #12705: [BEAM-10720] Finish implementing StringMethods (cat, repeat)

Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on a change in pull request #12705:
URL: https://github.com/apache/beam/pull/12705#discussion_r501926434



##########
File path: sdks/python/apache_beam/dataframe/transforms_test.py
##########
@@ -266,6 +266,17 @@ def check(actual):
           lambda x: {'res': 3 * x}, proxy)
       assert_that(res['res'], equal_to_series(three_series), 'CheckDictOut')
 
+  def test_cat(self):
+    # verify that cat works with a List[Series] sicne this is missing from doctests
+    df = pd.DataFrame({
+        'one': ['A', 'B', 'C'],
+        'two': ['BB', 'CC', 'A'],
+        'three': ['CCC', 'AA', 'B'],
+    })
+    self.run_scenario(df, lambda df: df.two.str.cat([df.three], join='outer'))
+    self.run_scenario(
+        df, lambda df: df.one.str.cat([df.two, df.three], join='outer'))
+

Review comment:
       Thank you!
   
   In order to avoid refactoring the elementwise methods I made DeferredStringMethods store two expressions, one that accesses Series.str, and one for the underlying Series. This seems to get things passing, but I'm not 100% sure this is safe. I'll look at extending `elementwise_method` for this use-case.




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



[GitHub] [beam] TheNeuralBit commented on a change in pull request #12705: [BEAM-10720] Finish implementing StringMethods (cat, repeat)

Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on a change in pull request #12705:
URL: https://github.com/apache/beam/pull/12705#discussion_r501925345



##########
File path: sdks/python/apache_beam/dataframe/transforms_test.py
##########
@@ -266,6 +266,18 @@ def check(actual):
           lambda x: {'res': 3 * x}, proxy)
       assert_that(res['res'], equal_to_series(three_series), 'CheckDictOut')
 
+  def test_cat(self):
+    # verify that cat works with a List[Series] sicne this is

Review comment:
       Done

##########
File path: sdks/python/apache_beam/dataframe/frames.py
##########
@@ -905,7 +905,48 @@ def checked_callable_index(df):
             preserves_partition_by=partitionings.Singleton()))
 
 class _DeferredStringMethods(frame_base.DeferredBase):
-  pass
+  @frame_base.args_to_kwargs(pd.core.strings.StringMethods)
+  @frame_base.populate_defaults(pd.core.strings.StringMethods)
+  def cat(self, others, join, **kwargs):
+    if others is None:
+      # Concatenate series into a single String
+      requires = partitionings.Singleton()
+      func = lambda df: df.cat(join=join, **kwargs)
+      args = [self._expr]
+
+    elif isinstance(others, frame_base.DeferredBase) or \

Review comment:
       Done




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



[GitHub] [beam] TheNeuralBit commented on pull request #12705: [BEAM-10720][WIP] Implement StringMethods

Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on pull request #12705:
URL: https://github.com/apache/beam/pull/12705#issuecomment-685012058


   Split this up into two PRs. Now this one depends on #12749, which includes just the elementwise methods (everything but `repeat` and `cat`).


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



[GitHub] [beam] TheNeuralBit commented on pull request #12705: [BEAM-10720][WIP] Implement StringMethods

Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on pull request #12705:
URL: https://github.com/apache/beam/pull/12705#issuecomment-692919644


   > Can you rebase this?
   
   Done


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



[GitHub] [beam] TheNeuralBit merged pull request #12705: [BEAM-10720] Finish implementing StringMethods (cat, repeat)

Posted by GitBox <gi...@apache.org>.
TheNeuralBit merged pull request #12705:
URL: https://github.com/apache/beam/pull/12705


   


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



[GitHub] [beam] robertwb commented on pull request #12705: [BEAM-10720][WIP] Implement StringMethods

Posted by GitBox <gi...@apache.org>.
robertwb commented on pull request #12705:
URL: https://github.com/apache/beam/pull/12705#issuecomment-692882655


   Can you rebase this?


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



[GitHub] [beam] TheNeuralBit commented on pull request #12705: [BEAM-10720] Finish implementing StringMethods (cat, repeat)

Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on pull request #12705:
URL: https://github.com/apache/beam/pull/12705#issuecomment-707379038


   @robertwb this is ready for another look


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



[GitHub] [beam] TheNeuralBit commented on a change in pull request #12705: [BEAM-10720] Finish implementing StringMethods (cat, repeat)

Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on a change in pull request #12705:
URL: https://github.com/apache/beam/pull/12705#discussion_r501925345



##########
File path: sdks/python/apache_beam/dataframe/transforms_test.py
##########
@@ -266,6 +266,18 @@ def check(actual):
           lambda x: {'res': 3 * x}, proxy)
       assert_that(res['res'], equal_to_series(three_series), 'CheckDictOut')
 
+  def test_cat(self):
+    # verify that cat works with a List[Series] sicne this is

Review comment:
       Done

##########
File path: sdks/python/apache_beam/dataframe/frames.py
##########
@@ -905,7 +905,48 @@ def checked_callable_index(df):
             preserves_partition_by=partitionings.Singleton()))
 
 class _DeferredStringMethods(frame_base.DeferredBase):
-  pass
+  @frame_base.args_to_kwargs(pd.core.strings.StringMethods)
+  @frame_base.populate_defaults(pd.core.strings.StringMethods)
+  def cat(self, others, join, **kwargs):
+    if others is None:
+      # Concatenate series into a single String
+      requires = partitionings.Singleton()
+      func = lambda df: df.cat(join=join, **kwargs)
+      args = [self._expr]
+
+    elif isinstance(others, frame_base.DeferredBase) or \

Review comment:
       Done

##########
File path: sdks/python/apache_beam/dataframe/transforms_test.py
##########
@@ -266,6 +266,17 @@ def check(actual):
           lambda x: {'res': 3 * x}, proxy)
       assert_that(res['res'], equal_to_series(three_series), 'CheckDictOut')
 
+  def test_cat(self):
+    # verify that cat works with a List[Series] sicne this is missing from doctests
+    df = pd.DataFrame({
+        'one': ['A', 'B', 'C'],
+        'two': ['BB', 'CC', 'A'],
+        'three': ['CCC', 'AA', 'B'],
+    })
+    self.run_scenario(df, lambda df: df.two.str.cat([df.three], join='outer'))
+    self.run_scenario(
+        df, lambda df: df.one.str.cat([df.two, df.three], join='outer'))
+

Review comment:
       Thank you!
   
   In order to avoid refactoring the elementwise methods I made DeferredStringMethods store two expressions, one that accesses Series.str, and one for the underlying Series. This seems to get things passing, but I'm not 100% sure this is safe. I'll look at extending `elementwise_method` for this use-case.

##########
File path: sdks/python/apache_beam/dataframe/transforms_test.py
##########
@@ -266,6 +266,17 @@ def check(actual):
           lambda x: {'res': 3 * x}, proxy)
       assert_that(res['res'], equal_to_series(three_series), 'CheckDictOut')
 
+  def test_cat(self):
+    # verify that cat works with a List[Series] sicne this is missing from doctests
+    df = pd.DataFrame({
+        'one': ['A', 'B', 'C'],
+        'two': ['BB', 'CC', 'A'],
+        'three': ['CCC', 'AA', 'B'],
+    })
+    self.run_scenario(df, lambda df: df.two.str.cat([df.three], join='outer'))
+    self.run_scenario(
+        df, lambda df: df.one.str.cat([df.two, df.three], join='outer'))
+

Review comment:
       Thank you!
   
   In order to avoid refactoring the elementwise methods I made DeferredStringMethods store two expressions, one that accesses Series.str, and one for the underlying Series. This seems to get things passing, but I'm not 100% sure this is safe. I'll look at extending `elementwise_method` for this use-case instead.




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



[GitHub] [beam] TheNeuralBit commented on a change in pull request #12705: [BEAM-10720] Finish implementing StringMethods (cat, repeat)

Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on a change in pull request #12705:
URL: https://github.com/apache/beam/pull/12705#discussion_r503567244



##########
File path: sdks/python/apache_beam/dataframe/transforms_test.py
##########
@@ -266,6 +266,17 @@ def check(actual):
           lambda x: {'res': 3 * x}, proxy)
       assert_that(res['res'], equal_to_series(three_series), 'CheckDictOut')
 
+  def test_cat(self):
+    # verify that cat works with a List[Series] sicne this is missing from doctests
+    df = pd.DataFrame({
+        'one': ['A', 'B', 'C'],
+        'two': ['BB', 'CC', 'A'],
+        'three': ['CCC', 'AA', 'B'],
+    })
+    self.run_scenario(df, lambda df: df.two.str.cat([df.three], join='outer'))
+    self.run_scenario(
+        df, lambda df: df.one.str.cat([df.two, df.three], join='outer'))
+

Review comment:
       Done




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



[GitHub] [beam] TheNeuralBit commented on a change in pull request #12705: [BEAM-10720] Finish implementing StringMethods (cat, repeat)

Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on a change in pull request #12705:
URL: https://github.com/apache/beam/pull/12705#discussion_r501926434



##########
File path: sdks/python/apache_beam/dataframe/transforms_test.py
##########
@@ -266,6 +266,17 @@ def check(actual):
           lambda x: {'res': 3 * x}, proxy)
       assert_that(res['res'], equal_to_series(three_series), 'CheckDictOut')
 
+  def test_cat(self):
+    # verify that cat works with a List[Series] sicne this is missing from doctests
+    df = pd.DataFrame({
+        'one': ['A', 'B', 'C'],
+        'two': ['BB', 'CC', 'A'],
+        'three': ['CCC', 'AA', 'B'],
+    })
+    self.run_scenario(df, lambda df: df.two.str.cat([df.three], join='outer'))
+    self.run_scenario(
+        df, lambda df: df.one.str.cat([df.two, df.three], join='outer'))
+

Review comment:
       Thank you!
   
   In order to avoid refactoring the elementwise methods I made DeferredStringMethods store two expressions, one that accesses Series.str, and one for the underlying Series. This seems to get things passing, but I'm not 100% sure this is safe. I'll look at extending `elementwise_method` for this use-case instead.




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



[GitHub] [beam] TheNeuralBit commented on a change in pull request #12705: [BEAM-10720][WIP] Implement StringMethods

Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on a change in pull request #12705:
URL: https://github.com/apache/beam/pull/12705#discussion_r478697347



##########
File path: sdks/python/apache_beam/dataframe/transforms_test.py
##########
@@ -266,6 +266,17 @@ def check(actual):
           lambda x: {'res': 3 * x}, proxy)
       assert_that(res['res'], equal_to_series(three_series), 'CheckDictOut')
 
+  def test_cat(self):
+    # verify that cat works with a List[Series] sicne this is missing from doctests
+    df = pd.DataFrame({
+        'one': ['A', 'B', 'C'],
+        'two': ['BB', 'CC', 'A'],
+        'three': ['CCC', 'AA', 'B'],
+    })
+    self.run_scenario(df, lambda df: df.two.str.cat([df.three], join='outer'))
+    self.run_scenario(
+        df, lambda df: df.one.str.cat([df.two, df.three], join='outer'))
+

Review comment:
       I've marked this PR WIP because this test does not pass. It yields the following error, that I'm not sure how to diagnose. @robertwb can you provide any insight here?
   
   ```
   apache_beam/runners/common.py:806: in _invoke_process_per_window                                                                                                                                                                        
       [si[global_window] for si in self.side_inputs]))                                                                                                                                                                                    
   apache_beam/runners/common.py:806: in <listcomp>                                                                                                                                                                                        
       [si[global_window] for si in self.side_inputs]))                                                                                                                                                                                    
   apache_beam/runners/worker/bundle_processor.py:422: in __getitem__                                                                                                                                                                      
       self._cache[target_window] = self._side_input_data.view_fn(raw_view)                                                                                                                                                                
   apache_beam/pvalue.py:395: in <lambda>                                                                                                                                                                                                  
       lambda iterable: from_runtime_iterable(iterable, view_options))                                                                                                                                                                     
   _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _                                                                                                             
                                                                                                                                                                                                                                           
   it = <apache_beam.runners.worker.bundle_processor._StateBackedIterable object at 0x7f3b0e5cf310>                                                                                                                                        
   options = {'coder': WindowedValueCoder[FastPrimitivesCoder], 'window_mapping_fn': <function _global_window_mapping_fn at 0x7f3b1b31b710>}
                                                                                                                                                                                                                                           
       @staticmethod                                                                                                   
       def _from_runtime_iterable(it, options):                                                                                                                                                                                            
         head = list(itertools.islice(it, 2))                                                                                                                                                                                              
         if not head:                                                                                                  
           return options.get('default', EmptySideInput())                                                             
         elif len(head) == 1:                                                                                          
           return head[0]                                                                                                                                                                                                                  
         raise ValueError(                                                                                                                                                                                                                 
             'PCollection of size %d with more than one element accessed as a '                                                                                                                                                            
             'singleton view. First two elements encountered are "%s", "%s".' %                                                                                                                                                            
   >         (len(head), str(head[0]), str(head[1])))                                                                                                                                                                                      
   E     ValueError: PCollection of size 2 with more than one element accessed as a singleton view. First two elements encountered are "<pandas.core.strings.StringMethods object at 0x7f3b0e32ec90>", "<pandas.core.strings.StringMethods 
   object at 0x7f3b0e3d7510>". [while running 'DataframeTransform/Eval/[ComputedExpression[cat_Series_139891620772240]]:139891619912080/FlatMap(evaluate)/FlatMap(evaluate)']  
   ```




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