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/12/28 08:57:48 UTC
[GitHub] [spark] beliefer opened a new pull request, #39262: [SPARK-41069][CONNECT][PYTHON] Implement `DataFrame.approxQuantile` and `DataFrame.stat.approxQuantile`
beliefer opened a new pull request, #39262:
URL: https://github.com/apache/spark/pull/39262
### What changes were proposed in this pull request?
Implement `DataFrame.stat.approxQuantile` with a proto message
Implement `DataFrame.stat.approxQuantile` for scala API
Implement `DataFrame.stat.approxQuantile` for python API
### Why are the changes needed?
for Connect API coverage
### Does this PR introduce _any_ user-facing change?
'No'. New API
### How was this patch tested?
New test cases.
--
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 a diff in pull request #39262: [SPARK-41069][CONNECT][PYTHON] Implement `DataFrame.approxQuantile` and `DataFrame.stat.approxQuantile`
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on code in PR #39262:
URL: https://github.com/apache/spark/pull/39262#discussion_r1058894403
##########
python/pyspark/sql/tests/connect/test_connect_basic.py:
##########
@@ -1013,6 +1013,42 @@ def test_stat_cov(self):
self.spark.read.table(self.tbl_name2).stat.cov("col1", "col3"),
)
+ def test_stat_approx_quantile(self):
+ # SPARK-41069: Test the stat.approxQuantile method
+ result = self.connect.read.table(self.tbl_name2).stat.approxQuantile(
+ ["col1", "col3"], [0.1, 0.5, 0.9], 0.1
+ )
+ self.assertEqual(len(result), 2)
+ self.assertEqual(len(result[0]), 3)
+ self.assertEqual(len(result[1]), 3)
+
+ with self.assertRaisesRegex(
+ TypeError, "col should be a string, list or tuple, but got <class 'int'>"
+ ):
+ self.connect.read.table(self.tbl_name2).stat.approxQuantile(1, [0.1, 0.5, 0.9], 0.1)
+ with self.assertRaisesRegex(TypeError, "columns should be strings, but got <class 'int'>"):
+ self.connect.read.table(self.tbl_name2).stat.approxQuantile([1], [0.1, 0.5, 0.9], 0.1)
+ with self.assertRaisesRegex(TypeError, "probabilities should be a list or tuple"):
+ self.connect.read.table(self.tbl_name2).stat.approxQuantile(["col1", "col3"], 0.1, 0.1)
+ with self.assertRaises(ValueError) as context:
Review Comment:
hm, I think `assertRaisesRegex` is better since you;re doing `"probabilities should be numerical (float, int) in [0,1]" in str(context.exception)`? Using regex would be shorter at least.
--
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] beliefer commented on a diff in pull request #39262: [SPARK-41069][CONNECT][PYTHON] Implement `DataFrame.approxQuantile` and `DataFrame.stat.approxQuantile`
Posted by GitBox <gi...@apache.org>.
beliefer commented on code in PR #39262:
URL: https://github.com/apache/spark/pull/39262#discussion_r1058892673
##########
python/pyspark/sql/tests/connect/test_connect_basic.py:
##########
@@ -1013,6 +1013,42 @@ def test_stat_cov(self):
self.spark.read.table(self.tbl_name2).stat.cov("col1", "col3"),
)
+ def test_stat_approx_quantile(self):
+ # SPARK-41069: Test the stat.approxQuantile method
+ result = self.connect.read.table(self.tbl_name2).stat.approxQuantile(
Review Comment:
The result is different between each execution.
--
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] zhengruifeng commented on a diff in pull request #39262: [SPARK-41069][CONNECT][PYTHON] Implement `DataFrame.approxQuantile` and `DataFrame.stat.approxQuantile`
Posted by GitBox <gi...@apache.org>.
zhengruifeng commented on code in PR #39262:
URL: https://github.com/apache/spark/pull/39262#discussion_r1058843822
##########
connector/connect/common/src/main/protobuf/spark/connect/relations.proto:
##########
@@ -70,6 +70,7 @@ message Relation {
StatCrosstab crosstab = 101;
StatDescribe describe = 102;
StatCov cov = 103;
+ StatApproxQuantile approx_quantile = 104;
Review Comment:
let's merge https://github.com/apache/spark/pull/39236 first
##########
python/pyspark/sql/tests/connect/test_connect_basic.py:
##########
@@ -1013,6 +1013,42 @@ def test_stat_cov(self):
self.spark.read.table(self.tbl_name2).stat.cov("col1", "col3"),
)
+ def test_stat_approx_quantile(self):
+ # SPARK-41069: Test the stat.approxQuantile method
+ result = self.connect.read.table(self.tbl_name2).stat.approxQuantile(
+ ["col1", "col3"], [0.1, 0.5, 0.9], 0.1
Review Comment:
please also add a case with single column
##########
python/pyspark/sql/tests/connect/test_connect_basic.py:
##########
@@ -1013,6 +1013,42 @@ def test_stat_cov(self):
self.spark.read.table(self.tbl_name2).stat.cov("col1", "col3"),
)
+ def test_stat_approx_quantile(self):
+ # SPARK-41069: Test the stat.approxQuantile method
+ result = self.connect.read.table(self.tbl_name2).stat.approxQuantile(
+ ["col1", "col3"], [0.1, 0.5, 0.9], 0.1
+ )
+ self.assertEqual(len(result), 2)
+ self.assertEqual(len(result[0]), 3)
+ self.assertEqual(len(result[1]), 3)
+
+ with self.assertRaisesRegex(
+ TypeError, "col should be a string, list or tuple, but got <class 'int'>"
+ ):
+ self.connect.read.table(self.tbl_name2).stat.approxQuantile(1, [0.1, 0.5, 0.9], 0.1)
+ with self.assertRaisesRegex(TypeError, "columns should be strings, but got <class 'int'>"):
+ self.connect.read.table(self.tbl_name2).stat.approxQuantile([1], [0.1, 0.5, 0.9], 0.1)
+ with self.assertRaisesRegex(TypeError, "probabilities should be a list or tuple"):
+ self.connect.read.table(self.tbl_name2).stat.approxQuantile(["col1", "col3"], 0.1, 0.1)
+ with self.assertRaises(ValueError) as context:
Review Comment:
why not using `with self. assertRaisesRegex(ValueError, "probabilities should be numerical")`
##########
python/pyspark/sql/tests/connect/test_connect_basic.py:
##########
@@ -1013,6 +1013,42 @@ def test_stat_cov(self):
self.spark.read.table(self.tbl_name2).stat.cov("col1", "col3"),
)
+ def test_stat_approx_quantile(self):
+ # SPARK-41069: Test the stat.approxQuantile method
+ result = self.connect.read.table(self.tbl_name2).stat.approxQuantile(
+ ["col1", "col3"], [0.1, 0.5, 0.9], 0.1
+ )
+ self.assertEqual(len(result), 2)
+ self.assertEqual(len(result[0]), 3)
+ self.assertEqual(len(result[1]), 3)
+
+ with self.assertRaisesRegex(
+ TypeError, "col should be a string, list or tuple, but got <class 'int'>"
+ ):
+ self.connect.read.table(self.tbl_name2).stat.approxQuantile(1, [0.1, 0.5, 0.9], 0.1)
+ with self.assertRaisesRegex(TypeError, "columns should be strings, but got <class 'int'>"):
+ self.connect.read.table(self.tbl_name2).stat.approxQuantile([1], [0.1, 0.5, 0.9], 0.1)
+ with self.assertRaisesRegex(TypeError, "probabilities should be a list or tuple"):
+ self.connect.read.table(self.tbl_name2).stat.approxQuantile(["col1", "col3"], 0.1, 0.1)
+ with self.assertRaises(ValueError) as context:
+ self.connect.read.table(self.tbl_name2).stat.approxQuantile(
+ ["col1", "col3"], [-0.1], 0.1
+ )
+ self.assertTrue(
+ "probabilities should be numerical (float, int) in [0,1]" in str(context.exception)
+ )
+ with self.assertRaises(TypeError) as context:
Review Comment:
ditto
##########
python/pyspark/sql/tests/connect/test_connect_basic.py:
##########
@@ -1013,6 +1013,42 @@ def test_stat_cov(self):
self.spark.read.table(self.tbl_name2).stat.cov("col1", "col3"),
)
+ def test_stat_approx_quantile(self):
+ # SPARK-41069: Test the stat.approxQuantile method
+ result = self.connect.read.table(self.tbl_name2).stat.approxQuantile(
Review Comment:
let's compare the results with PySpark
--
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] beliefer commented on a diff in pull request #39262: [SPARK-41069][CONNECT][PYTHON] Implement `DataFrame.approxQuantile` and `DataFrame.stat.approxQuantile`
Posted by GitBox <gi...@apache.org>.
beliefer commented on code in PR #39262:
URL: https://github.com/apache/spark/pull/39262#discussion_r1058889983
##########
connector/connect/common/src/main/protobuf/spark/connect/relations.proto:
##########
@@ -70,6 +70,7 @@ message Relation {
StatCrosstab crosstab = 101;
StatDescribe describe = 102;
StatCov cov = 103;
+ StatApproxQuantile approx_quantile = 104;
Review Comment:
OK
--
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] beliefer commented on a diff in pull request #39262: [SPARK-41069][CONNECT][PYTHON] Implement `DataFrame.approxQuantile` and `DataFrame.stat.approxQuantile`
Posted by GitBox <gi...@apache.org>.
beliefer commented on code in PR #39262:
URL: https://github.com/apache/spark/pull/39262#discussion_r1058893340
##########
python/pyspark/sql/tests/connect/test_connect_basic.py:
##########
@@ -1013,6 +1013,42 @@ def test_stat_cov(self):
self.spark.read.table(self.tbl_name2).stat.cov("col1", "col3"),
)
+ def test_stat_approx_quantile(self):
+ # SPARK-41069: Test the stat.approxQuantile method
+ result = self.connect.read.table(self.tbl_name2).stat.approxQuantile(
+ ["col1", "col3"], [0.1, 0.5, 0.9], 0.1
+ )
+ self.assertEqual(len(result), 2)
+ self.assertEqual(len(result[0]), 3)
+ self.assertEqual(len(result[1]), 3)
+
+ with self.assertRaisesRegex(
+ TypeError, "col should be a string, list or tuple, but got <class 'int'>"
+ ):
+ self.connect.read.table(self.tbl_name2).stat.approxQuantile(1, [0.1, 0.5, 0.9], 0.1)
+ with self.assertRaisesRegex(TypeError, "columns should be strings, but got <class 'int'>"):
+ self.connect.read.table(self.tbl_name2).stat.approxQuantile([1], [0.1, 0.5, 0.9], 0.1)
+ with self.assertRaisesRegex(TypeError, "probabilities should be a list or tuple"):
+ self.connect.read.table(self.tbl_name2).stat.approxQuantile(["col1", "col3"], 0.1, 0.1)
+ with self.assertRaises(ValueError) as context:
Review Comment:
It works worse.
--
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 #39262: [SPARK-41069][CONNECT][PYTHON] Implement `DataFrame.approxQuantile` and `DataFrame.stat.approxQuantile`
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on PR #39262:
URL: https://github.com/apache/spark/pull/39262#issuecomment-1368141508
LGTM @beliefer mind revasing this please?
--
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] beliefer commented on a diff in pull request #39262: [SPARK-41069][CONNECT][PYTHON] Implement `DataFrame.approxQuantile` and `DataFrame.stat.approxQuantile`
Posted by GitBox <gi...@apache.org>.
beliefer commented on code in PR #39262:
URL: https://github.com/apache/spark/pull/39262#discussion_r1059210878
##########
python/pyspark/sql/tests/connect/test_connect_basic.py:
##########
@@ -1013,6 +1013,42 @@ def test_stat_cov(self):
self.spark.read.table(self.tbl_name2).stat.cov("col1", "col3"),
)
+ def test_stat_approx_quantile(self):
+ # SPARK-41069: Test the stat.approxQuantile method
+ result = self.connect.read.table(self.tbl_name2).stat.approxQuantile(
+ ["col1", "col3"], [0.1, 0.5, 0.9], 0.1
Review Comment:
OK
--
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] beliefer commented on a diff in pull request #39262: [SPARK-41069][CONNECT][PYTHON] Implement `DataFrame.approxQuantile` and `DataFrame.stat.approxQuantile`
Posted by GitBox <gi...@apache.org>.
beliefer commented on code in PR #39262:
URL: https://github.com/apache/spark/pull/39262#discussion_r1058893340
##########
python/pyspark/sql/tests/connect/test_connect_basic.py:
##########
@@ -1013,6 +1013,42 @@ def test_stat_cov(self):
self.spark.read.table(self.tbl_name2).stat.cov("col1", "col3"),
)
+ def test_stat_approx_quantile(self):
+ # SPARK-41069: Test the stat.approxQuantile method
+ result = self.connect.read.table(self.tbl_name2).stat.approxQuantile(
+ ["col1", "col3"], [0.1, 0.5, 0.9], 0.1
+ )
+ self.assertEqual(len(result), 2)
+ self.assertEqual(len(result[0]), 3)
+ self.assertEqual(len(result[1]), 3)
+
+ with self.assertRaisesRegex(
+ TypeError, "col should be a string, list or tuple, but got <class 'int'>"
+ ):
+ self.connect.read.table(self.tbl_name2).stat.approxQuantile(1, [0.1, 0.5, 0.9], 0.1)
+ with self.assertRaisesRegex(TypeError, "columns should be strings, but got <class 'int'>"):
+ self.connect.read.table(self.tbl_name2).stat.approxQuantile([1], [0.1, 0.5, 0.9], 0.1)
+ with self.assertRaisesRegex(TypeError, "probabilities should be a list or tuple"):
+ self.connect.read.table(self.tbl_name2).stat.approxQuantile(["col1", "col3"], 0.1, 0.1)
+ with self.assertRaises(ValueError) as context:
Review Comment:
It works worse.
--
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] zhengruifeng closed pull request #39262: [SPARK-41069][CONNECT][PYTHON] Implement `DataFrame.approxQuantile` and `DataFrame.stat.approxQuantile`
Posted by GitBox <gi...@apache.org>.
zhengruifeng closed pull request #39262: [SPARK-41069][CONNECT][PYTHON] Implement `DataFrame.approxQuantile` and `DataFrame.stat.approxQuantile`
URL: https://github.com/apache/spark/pull/39262
--
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] beliefer commented on a diff in pull request #39262: [SPARK-41069][CONNECT][PYTHON] Implement `DataFrame.approxQuantile` and `DataFrame.stat.approxQuantile`
Posted by GitBox <gi...@apache.org>.
beliefer commented on code in PR #39262:
URL: https://github.com/apache/spark/pull/39262#discussion_r1059263255
##########
python/pyspark/sql/tests/connect/test_connect_basic.py:
##########
@@ -1013,6 +1013,42 @@ def test_stat_cov(self):
self.spark.read.table(self.tbl_name2).stat.cov("col1", "col3"),
)
+ def test_stat_approx_quantile(self):
+ # SPARK-41069: Test the stat.approxQuantile method
+ result = self.connect.read.table(self.tbl_name2).stat.approxQuantile(
+ ["col1", "col3"], [0.1, 0.5, 0.9], 0.1
+ )
+ self.assertEqual(len(result), 2)
+ self.assertEqual(len(result[0]), 3)
+ self.assertEqual(len(result[1]), 3)
+
+ with self.assertRaisesRegex(
+ TypeError, "col should be a string, list or tuple, but got <class 'int'>"
+ ):
+ self.connect.read.table(self.tbl_name2).stat.approxQuantile(1, [0.1, 0.5, 0.9], 0.1)
+ with self.assertRaisesRegex(TypeError, "columns should be strings, but got <class 'int'>"):
+ self.connect.read.table(self.tbl_name2).stat.approxQuantile([1], [0.1, 0.5, 0.9], 0.1)
+ with self.assertRaisesRegex(TypeError, "probabilities should be a list or tuple"):
+ self.connect.read.table(self.tbl_name2).stat.approxQuantile(["col1", "col3"], 0.1, 0.1)
+ with self.assertRaises(ValueError) as context:
Review Comment:
Got it.
--
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] beliefer commented on pull request #39262: [SPARK-41069][CONNECT][PYTHON] Implement `DataFrame.approxQuantile` and `DataFrame.stat.approxQuantile`
Posted by GitBox <gi...@apache.org>.
beliefer commented on PR #39262:
URL: https://github.com/apache/spark/pull/39262#issuecomment-1368151438
> LGTM @beliefer mind revasing this please?
OK. rebased.
--
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] zhengruifeng commented on pull request #39262: [SPARK-41069][CONNECT][PYTHON] Implement `DataFrame.approxQuantile` and `DataFrame.stat.approxQuantile`
Posted by GitBox <gi...@apache.org>.
zhengruifeng commented on PR #39262:
URL: https://github.com/apache/spark/pull/39262#issuecomment-1368160745
merged into master, thank you @beliefer
--
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] beliefer commented on pull request #39262: [SPARK-41069][CONNECT][PYTHON] Implement `DataFrame.approxQuantile` and `DataFrame.stat.approxQuantile`
Posted by GitBox <gi...@apache.org>.
beliefer commented on PR #39262:
URL: https://github.com/apache/spark/pull/39262#issuecomment-1367067211
ping @HyukjinKwon @zhengruifeng @grundprinzip @amaliujia
--
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] beliefer commented on pull request #39262: [SPARK-41069][CONNECT][PYTHON] Implement `DataFrame.approxQuantile` and `DataFrame.stat.approxQuantile`
Posted by GitBox <gi...@apache.org>.
beliefer commented on PR #39262:
URL: https://github.com/apache/spark/pull/39262#issuecomment-1368176994
@zhengruifeng @HyukjinKwon Thank you!
--
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