You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/04/08 16:51:18 UTC
[GitHub] [spark] EnricoMi opened a new pull request, #36120: [SPARK-38833][PYTHON][SQL] Allow applyInPandas to return empty DataFrame without columns
EnricoMi opened a new pull request, #36120:
URL: https://github.com/apache/spark/pull/36120
### What changes were proposed in this pull request?
Methods `wrap_cogrouped_map_pandas_udf` and `wrap_grouped_map_pandas_udf` in `python/pyspark/worker.py` do not need to reject `pd.DataFrame`s with no columns return by udf when that DataFrame is empty (zero rows). This allows to return empty DataFrames without the need to define columns. The DataFrame is empty after all!
### Why are the changes needed?
Returning an empty DataFrame from the lambda given to `applyInPandas` should be as easy as this:
```python
return pd.DataFrame([])
```
However, PySpark requires that empty DataFrame to have the right _number_ of columns. This seems redundant as the schema is already defined in the `applyInPandas` call. Returning a non-empty DataFrame does not require defining columns.
Here is an example to reproduce:
```python
import pandas as pd
from pyspark.sql.functions import pandas_udf, ceil
df = spark.createDataFrame(
[(1, 1.0), (1, 2.0), (2, 3.0), (2, 5.0), (2, 10.0)],
("id", "v"))
def mean_func(key, pdf):
if key == (1,):
return pd.DataFrame([])
else:
return pd.DataFrame([key + (pdf.v.mean(),)])
df.groupby("id").applyInPandas(mean_func, schema="id long, v double").show()
```
### Does this PR introduce _any_ user-facing change?
It changes the behaviour of the following calls to allow returning empty `pd.DataFrame` without defining columns. The PySpark DataFrame returned by `applyInPandas` is unchanged:
- `df.groupby(…).applyInPandas(…)`
- `df.cogroup(…).applyInPandas(…)`
### How was this patch tested?
Tests are added that test `applyInPandas` returning
- empty DataFrame with no columns
- empty DataFrame with the wrong number of columns
- non-empty DataFrame with wrong number of columns
- something other than `pd.DataFrame`
TODO:
- test cogroup
- check mapInPandas
- look for other methods returning pd.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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] awdavidson commented on a diff in pull request #36120: [SPARK-38833][PYTHON][SQL] Allow applyInPandas to return empty DataFrame without columns
Posted by GitBox <gi...@apache.org>.
awdavidson commented on code in PR #36120:
URL: https://github.com/apache/spark/pull/36120#discussion_r846519415
##########
python/pyspark/sql/tests/test_pandas_map.py:
##########
@@ -95,17 +98,61 @@ def func(iterator):
actual = df.repartition(1).mapInPandas(func, "a long").collect()
self.assertEqual(set((r.a for r in actual)), set(range(100)))
+ def test_other_than_dataframe(self):
+ def bad_iter(_):
+ return iter([1])
+
+ with QuietTest(self.sc):
+ with self.assertRaisesRegex(
+ PythonException,
+ "Return type of the user-defined function should be Pandas.DataFrame, "
+ "but is <class 'int'>",
+ ):
+ self.spark.range(10, numPartitions=3).mapInPandas(
+ bad_iter, "a int, b string"
+ ).count()
+
def test_empty_iterator(self):
def empty_iter(_):
return iter([])
- self.assertEqual(self.spark.range(10).mapInPandas(empty_iter, "a int, b string").count(), 0)
+ mapped = self.spark.range(10, numPartitions=3).mapInPandas(empty_iter, "a int, b string")
+ self.assertEqual(mapped.count(), 0)
- def test_empty_rows(self):
- def empty_rows(_):
+ def test_empty_dataframes(self):
+ def empty_dataframes(_):
return iter([pd.DataFrame({"a": []})])
- self.assertEqual(self.spark.range(10).mapInPandas(empty_rows, "a int").count(), 0)
+ mapped = self.spark.range(10, numPartitions=3).mapInPandas(empty_dataframes, "a int")
+ self.assertEqual(mapped.count(), 0)
+
+ def test_empty_dataframes_without_columns(self):
+ def empty_dataframes_wo_columns(iterator):
+ for pdf in iterator:
+ yield pdf
+ # after yielding all elements of the iterator, also yield one dataframe without columns
+ yield pd.DataFrame([])
+
+ mapped = self.spark.range(10, numPartitions=3).toDF("id").mapInPandas(empty_dataframes_wo_columns, "id int")
+ self.assertEqual(mapped.count(), 10)
+
+ def test_empty_dataframes_with_less_columns(self):
+ def empty_dataframes_with_less_columns(iterator):
+ for pdf in iterator:
+ yield pdf
+ # after yielding all elements of the iterator, also yield one dataframe with less columns
+ yield pd.DataFrame([(1,)], columns=["id"])
+
+ with QuietTest(self.sc):
+ with self.assertRaisesRegex(
+ PythonException,
+ "KeyError: 'value'",
+ ):
+ self.spark.range(10, numPartitions=3) \
Review Comment:
nit: let’s wrap this block in a single parenthesis block rather than `\`
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] EnricoMi commented on pull request #36120: [SPARK-38833][PYTHON][SQL] Allow applyInPandas to return empty DataFrame without columns
Posted by GitBox <gi...@apache.org>.
EnricoMi commented on PR #36120:
URL: https://github.com/apache/spark/pull/36120#issuecomment-1097870597
Thanks a lot!
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] EnricoMi commented on pull request #36120: [SPARK-38833][PYTHON][SQL] Allow applyInPandas to return empty DataFrame without columns
Posted by GitBox <gi...@apache.org>.
EnricoMi commented on PR #36120:
URL: https://github.com/apache/spark/pull/36120#issuecomment-1094118772
@awdavidson would you say `mapInPandas` should be made behave similarly to `applyInPandas` or rather not touch existing relaxation on DataFrame schema constraints? To summarize, `mapInPandas` does not raise error when returned DataFrame has more columns than specified in schema argument, while `applyInPandas` does raise an error.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #36120: [SPARK-38833][PYTHON][SQL] Allow applyInPandas to return empty DataFrame without columns
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on PR #36120:
URL: https://github.com/apache/spark/pull/36120#issuecomment-1097686023
Merged to master.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] EnricoMi commented on pull request #36120: [SPARK-38833][PYTHON][SQL] Allow applyInPandas to return empty DataFrame without columns
Posted by GitBox <gi...@apache.org>.
EnricoMi commented on PR #36120:
URL: https://github.com/apache/spark/pull/36120#issuecomment-1094118175
Thanks for the quick review!
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #36120: [SPARK-38833][PYTHON][SQL] Allow applyInPandas to return empty DataFrame without columns
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on PR #36120:
URL: https://github.com/apache/spark/pull/36120#issuecomment-1094573425
Hm, making sense to me, yeah. Will take a closer look soon.
cc @BryanCutler @viirya @ueshin FYI
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon closed pull request #36120: [SPARK-38833][PYTHON][SQL] Allow applyInPandas to return empty DataFrame without columns
Posted by GitBox <gi...@apache.org>.
HyukjinKwon closed pull request #36120: [SPARK-38833][PYTHON][SQL] Allow applyInPandas to return empty DataFrame without columns
URL: https://github.com/apache/spark/pull/36120
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #36120: [SPARK-38833][PYTHON][SQL] Allow applyInPandas to return empty DataFrame without columns
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on PR #36120:
URL: https://github.com/apache/spark/pull/36120#issuecomment-1094110056
Can one of the admins verify this patch?
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] awdavidson commented on pull request #36120: [SPARK-38833][PYTHON][SQL] Allow applyInPandas to return empty DataFrame without columns
Posted by GitBox <gi...@apache.org>.
awdavidson commented on PR #36120:
URL: https://github.com/apache/spark/pull/36120#issuecomment-1094124683
> @awdavidson would you say `mapInPandas` should be made behave similarly to `applyInPandas` or rather not touch existing relaxation on DataFrame schema constraints? To summarize, `mapInPandas` does not raise error when returned DataFrame has more columns than specified in schema argument, while `applyInPandas` does raise an error.
I guess I would expect the output to have the specified schema so IMO it would make sense to raise an error when the result is different. It’ll be good to see what others say
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org