You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "ueshin (via GitHub)" <gi...@apache.org> on 2023/08/14 19:14:57 UTC
[GitHub] [spark] ueshin opened a new pull request, #42490: [SPARK-44749][PYTHON][FOLLOWUP][TESTS] Add more tests for named arguments in Python UDTF
ueshin opened a new pull request, #42490:
URL: https://github.com/apache/spark/pull/42490
### What changes were proposed in this pull request?
Adds more tests for named arguments in Python UDTF.
### Why are the changes needed?
There are more cases to test.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Added related tests.
--
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] ueshin commented on pull request #42490: [SPARK-44749][PYTHON][FOLLOWUP][TESTS] Add more tests for named arguments in Python UDTF
Posted by "ueshin (via GitHub)" <gi...@apache.org>.
ueshin commented on PR #42490:
URL: https://github.com/apache/spark/pull/42490#issuecomment-1678289394
Thanks! merging 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] allisonwang-db commented on a diff in pull request #42490: [SPARK-44749][PYTHON][FOLLOWUP][TESTS] Add more tests for named arguments in Python UDTF
Posted by "allisonwang-db (via GitHub)" <gi...@apache.org>.
allisonwang-db commented on code in PR #42490:
URL: https://github.com/apache/spark/pull/42490#discussion_r1293939453
##########
python/pyspark/sql/tests/test_udtf.py:
##########
@@ -1874,15 +1874,85 @@ def eval(self, **kwargs):
for i, df in enumerate(
[
- self.spark.sql("SELECT * FROM test_udtf(a=>10, b=>'x')"),
- self.spark.sql("SELECT * FROM test_udtf(b=>'x', a=>10)"),
+ self.spark.sql("SELECT * FROM test_udtf(a => 10, b => 'x')"),
+ self.spark.sql("SELECT * FROM test_udtf(b => 'x', a => 10)"),
TestUDTF(a=lit(10), b=lit("x")),
TestUDTF(b=lit("x"), a=lit(10)),
]
):
with self.subTest(query_no=i):
assertDataFrameEqual(df, [Row(a=10, b="x")])
+ def test_udtf_with_named_arguments_lateral_join(self):
+ @udtf
+ class TestUDTF:
+ @staticmethod
+ def analyze(a, b):
+ return AnalyzeResult(StructType().add("a", a.data_type))
+
+ def eval(self, a, b):
+ yield a,
+
+ self.spark.udtf.register("test_udtf", TestUDTF)
+
+ # lateral join
+ for i, df in enumerate(
+ [
+ self.spark.sql(
+ "SELECT f.* FROM "
+ "VALUES (0, 'x'), (1, 'y') t(a, b), LATERAL test_udtf(a => a, b => b) f"
+ ),
+ self.spark.sql(
+ "SELECT f.* FROM "
+ "VALUES (0, 'x'), (1, 'y') t(a, b), LATERAL test_udtf(b => b, a => a) f"
+ ),
+ ]
+ ):
+ with self.subTest(query_no=i):
+ assertDataFrameEqual(df, [Row(a=0), Row(a=1)])
+
+ def test_udtf_with_named_arguments_and_defaults(self):
+ @udtf
+ class TestUDTF:
+ @staticmethod
+ def analyze(a, b=None):
Review Comment:
If `b` is a parameter with a default value (e.g 100), do we need to also assign a default value (e.g. None) here in the `analyze` method? If we directly use `def analyze(a, b)` does it throw an exception?
--
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] ueshin commented on pull request #42490: [SPARK-44749][PYTHON][FOLLOWUP][TESTS] Add more tests for named arguments in Python UDTF
Posted by "ueshin (via GitHub)" <gi...@apache.org>.
ueshin commented on PR #42490:
URL: https://github.com/apache/spark/pull/42490#issuecomment-1677920645
cc @allisonwang-db
--
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] ueshin commented on a diff in pull request #42490: [SPARK-44749][PYTHON][FOLLOWUP][TESTS] Add more tests for named arguments in Python UDTF
Posted by "ueshin (via GitHub)" <gi...@apache.org>.
ueshin commented on code in PR #42490:
URL: https://github.com/apache/spark/pull/42490#discussion_r1293963530
##########
python/pyspark/sql/tests/test_udtf.py:
##########
@@ -1874,15 +1874,85 @@ def eval(self, **kwargs):
for i, df in enumerate(
[
- self.spark.sql("SELECT * FROM test_udtf(a=>10, b=>'x')"),
- self.spark.sql("SELECT * FROM test_udtf(b=>'x', a=>10)"),
+ self.spark.sql("SELECT * FROM test_udtf(a => 10, b => 'x')"),
+ self.spark.sql("SELECT * FROM test_udtf(b => 'x', a => 10)"),
TestUDTF(a=lit(10), b=lit("x")),
TestUDTF(b=lit("x"), a=lit(10)),
]
):
with self.subTest(query_no=i):
assertDataFrameEqual(df, [Row(a=10, b="x")])
+ def test_udtf_with_named_arguments_lateral_join(self):
+ @udtf
+ class TestUDTF:
+ @staticmethod
+ def analyze(a, b):
+ return AnalyzeResult(StructType().add("a", a.data_type))
+
+ def eval(self, a, b):
+ yield a,
+
+ self.spark.udtf.register("test_udtf", TestUDTF)
+
+ # lateral join
+ for i, df in enumerate(
+ [
+ self.spark.sql(
+ "SELECT f.* FROM "
+ "VALUES (0, 'x'), (1, 'y') t(a, b), LATERAL test_udtf(a => a, b => b) f"
+ ),
+ self.spark.sql(
+ "SELECT f.* FROM "
+ "VALUES (0, 'x'), (1, 'y') t(a, b), LATERAL test_udtf(b => b, a => a) f"
+ ),
+ ]
+ ):
+ with self.subTest(query_no=i):
+ assertDataFrameEqual(df, [Row(a=0), Row(a=1)])
+
+ def test_udtf_with_named_arguments_and_defaults(self):
+ @udtf
+ class TestUDTF:
+ @staticmethod
+ def analyze(a, b=None):
Review Comment:
Yes, if `b` here doesn't have a default value, it will raise an exception.
Both `eval` and `analyze` will be called with the same argument list.
So if `eval` takes `a`, `analyze` should also take `a`, if `eval` takes `a, b`, `analyze` should also take `a, b`, and so on.
It can be `AnalyzeArgument` with the correct parameters, or take `**kwargs`.
```py
def analyze(a, b=AnalyzeArgument(StringType(), None, False)):
```
```py
def analyze(a, **kwargs):
```
but taking `AnalyzeArgument` as default is not recommended if the `data_type` is a struct type, which could cause unexpected behavior similar to taking `[]` or `{}` as default.
--
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] ueshin closed pull request #42490: [SPARK-44749][PYTHON][FOLLOWUP][TESTS] Add more tests for named arguments in Python UDTF
Posted by "ueshin (via GitHub)" <gi...@apache.org>.
ueshin closed pull request #42490: [SPARK-44749][PYTHON][FOLLOWUP][TESTS] Add more tests for named arguments in Python UDTF
URL: https://github.com/apache/spark/pull/42490
--
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