[SPARK-40645][CONNECT] Throw exception for Collect() and recommend to use toPandas() #38089
Conversation
|
R: @grundprinzip @HyukjinKwon |
python/pyspark/sql/connect/client.py
Outdated
| return DataFrame.withPlan(SQL(sql_string), self) | ||
|
|
||
| def collect(self, plan: pb2.Plan) -> pandas.DataFrame: | ||
| def toPandas(self, plan: pb2.Plan) -> pandas.DataFrame: |
There was a problem hiding this comment.
I think we should remove this ... ? since this doesn;t exists in SparkSession
There was a problem hiding this comment.
We should make these methods private instead and see how they're used.
There was a problem hiding this comment.
The tricky part is that while the original session does not have this method we need a way to pass the plan to the client.
There was a problem hiding this comment.
I make it private but still have data_frame to call it. We need a way to pass the plan to the session/client.
We can see in the future if this can be replaced.
python/pyspark/sql/connect/client.py
Outdated
| return DataFrame.withPlan(SQL(sql_string), self) | ||
|
|
||
| def collect(self, plan: pb2.Plan) -> pandas.DataFrame: | ||
| def _toPandas(self, plan: pb2.Plan) -> pandas.DataFrame: |
There was a problem hiding this comment.
Actually we should use snake naming rule for private methods. Only API follows the camel naming conversion
There was a problem hiding this comment.
oh I see done!
I am still learning how to write python code for spark....
|
Merged to master. |
4 participants
What changes were proposed in this pull request?
Current connect
Collect()return Pandas DataFrame, which does not match with PySpark DataFrame API which returns aList[Row]:spark/python/pyspark/sql/connect/data_frame.py
Line 227 in ceb8527
spark/python/pyspark/sql/dataframe.py
Line 1119 in ceb8527
The underlying implementation has been generating Pandas DataFrame though. In this case, we can choose to use to
toPandas()and throw exception forCollect()to recommend to usetoPandas().Why are the changes needed?
The goal of the connect project is still to align with existing data frame API as much as possible. In this case, given that
Collect()is not compatible in existing python client, we can choose to disable it for now.Does this PR introduce any user-facing change?
No
How was this patch tested?
UT