Skip to content

[SPARK-40645][CONNECT] Throw exception for Collect() and recommend to use toPandas() #38089

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
amaliujia wants to merge 4 commits into from
Closed

[SPARK-40645][CONNECT] Throw exception for Collect() and recommend to use toPandas() #38089

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e362147
[SPARK-40645] Throw exception for Collect() and recommend to use toPa…
Oct 4, 2022
f129f80
update
Oct 4, 2022
5d73d45
update
Oct 5, 2022
5ce90f7
Apply suggestions from code review
HyukjinKwon Oct 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion python/pyspark/sql/connect/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ def _build_metrics(self, metrics: "pb2.Response.Metrics") -> typing.List[PlanMet
def sql(self, sql_string: str) -> "DataFrame":
return DataFrame.withPlan(SQL(sql_string), self)

def collect(self, plan: pb2.Plan) -> pandas.DataFrame:
def toPandas(self, plan: pb2.Plan) -> pandas.DataFrame:

@HyukjinKwon HyukjinKwon Oct 4, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should remove this ... ? since this doesn;t exists in SparkSession

@HyukjinKwon HyukjinKwon Oct 4, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @grundprinzip FYI

@grundprinzip grundprinzip Oct 4, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should make these methods private instead and see how they're used.

@grundprinzip grundprinzip Oct 4, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@amaliujia amaliujia Oct 4, 2022

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

req = pb2.Request()
req.user_context.user_id = self._user_id
req.plan.CopyFrom(plan)
Expand Down
10 changes: 6 additions & 4 deletions python/pyspark/sql/connect/data_frame.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
TYPE_CHECKING,
)

import pandas

import pyspark.sql.connect.plan as plan
from pyspark.sql.connect.column import (
ColumnOrString,
Expand Down Expand Up @@ -225,11 +227,11 @@ def _print_plan(self) -> str:
return ""

def collect(self):
query = self._plan.collect(self._session)
return self._session.collect(query)
raise NotImplementedError("Please use toPandas().")

def toPandas(self):
return self.collect()
def toPandas(self) -> pandas.DataFrame:
query = self._plan.collect(self._session)
return self._session.toPandas(query)

def explain(self) -> str:
query = self._plan.collect(self._session)
Expand Down
4 changes: 2 additions & 2 deletions python/pyspark/sql/tests/connect/test_spark_connect.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class SparkConnectTests(SparkConnectSQLTestCase):
def test_simple_read(self) -> None:
"""Tests that we can access the Spark Connect GRPC service locally."""
df = self.connect.read.table(self.tbl_name)
data = df.limit(10).collect()
data = df.limit(10).toPandas()
# Check that the limit is applied
assert len(data.index) == 10

Expand All @@ -67,7 +67,7 @@ def conv_udf(x) -> str:

u = udf(conv_udf)
df = self.connect.read.table(self.tbl_name)
result = df.select(u(df.id)).collect()
result = df.select(u(df.id)).toPandas()
assert result is not None

def test_simple_explain_string(self) -> None:
Expand Down