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/09/08 12:21:25 UTC
[GitHub] [spark] Yikun opened a new pull request, #37836: [WIP][SPARK-40339][PS] Implement quantile in Rolling/RollingGroupby/Expanding/ExpandingGroupby
Yikun opened a new pull request, #37836:
URL: https://github.com/apache/spark/pull/37836
### What changes were proposed in this pull request?
Implement quantile in Rolling/RollingGroupby/Expanding/ExpandingGroupby
### Why are the changes needed?
TBD
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
CI passed
--
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] Yikun commented on pull request #37836: [SPARK-40339][SPARK-40342][SPARK-40345][SPARK-40348][PS] Implement quantile in Rolling/RollingGroupby/Expanding/ExpandingGroupby
Posted by GitBox <gi...@apache.org>.
Yikun commented on PR #37836:
URL: https://github.com/apache/spark/pull/37836#issuecomment-1247519104
@HyukjinKwon OK, thanks!
--
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] Yikun commented on pull request #37836: [SPARK-40339][SPARK-40342][SPARK-40345][SPARK-40348][PS] Implement quantile in Rolling/RollingGroupby/Expanding/ExpandingGroupby
Posted by GitBox <gi...@apache.org>.
Yikun commented on PR #37836:
URL: https://github.com/apache/spark/pull/37836#issuecomment-1247496635
@HyukjinKwon @zhengruifeng Thanks
> I think we should also add quantile to window.rst
Hmm, forgot again, I also noticed that `ExpandingGroupby` and `RollingGroupby` functions hasn't doc, so let me create a separate PR to cover doc:
- all functions for ExpandingGroupby
- all functions for RollingGroupby
- quantile for Expanding
- quantile for Rolling
--
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 #37836: [SPARK-40339][SPARK-40342][SPARK-40345][SPARK-40348][PS] Implement quantile in Rolling/RollingGroupby/Expanding/ExpandingGroupby
Posted by GitBox <gi...@apache.org>.
HyukjinKwon closed pull request #37836: [SPARK-40339][SPARK-40342][SPARK-40345][SPARK-40348][PS] Implement quantile in Rolling/RollingGroupby/Expanding/ExpandingGroupby
URL: https://github.com/apache/spark/pull/37836
--
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 #37836: [SPARK-40339][SPARK-40345][SPARK-40345][SPARK-40348][PS] Implement quantile in Rolling/RollingGroupby/Expanding/ExpandingGroupby
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on code in PR #37836:
URL: https://github.com/apache/spark/pull/37836#discussion_r969205360
##########
python/pyspark/pandas/window.py:
##########
@@ -1483,6 +1661,66 @@ def mean(self) -> FrameLike:
"""
return super().mean()
+ def quantile(self, quantile: float, accuracy: int = 10000) -> FrameLike:
+ """
+ Calculate the expanding quantile of the values.
+
+ Returns
+ -------
+ Series or DataFrame
+ Returned object type is determined by the caller of the expanding
+ calculation.
+
+ Parameters
+ ----------
+ quantile : float
+ Value between 0 and 1 providing the quantile to compute.
+ accuracy : int, optional
+ Default accuracy of approximation. Larger value means better accuracy.
+ The relative error can be deduced by 1.0 / accuracy.
+ This is a panda-on-Spark specific parameter.
+
+ Notes
+ -------
Review Comment:
Let's fix this `-` - otherwise the doc build complains IIRC.
--
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 #37836: [SPARK-40339][SPARK-40342][SPARK-40345][SPARK-40348][PS] Implement quantile in Rolling/RollingGroupby/Expanding/ExpandingGroupby
Posted by GitBox <gi...@apache.org>.
zhengruifeng commented on PR #37836:
URL: https://github.com/apache/spark/pull/37836#issuecomment-1247482347
@Yikun I think we should also add `quantile` to `window.rst`
--
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 #37836: [SPARK-40339][SPARK-40342][SPARK-40345][SPARK-40348][PS] Implement quantile in Rolling/RollingGroupby/Expanding/ExpandingGroupby
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on PR #37836:
URL: https://github.com/apache/spark/pull/37836#issuecomment-1245441448
cc @zhengruifeng @xinrong-meng @itholic 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 commented on pull request #37836: [SPARK-40339][SPARK-40342][SPARK-40345][SPARK-40348][PS] Implement quantile in Rolling/RollingGroupby/Expanding/ExpandingGroupby
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on PR #37836:
URL: https://github.com/apache/spark/pull/37836#issuecomment-1247508508
Opps, just made a followup at https://github.com/apache/spark/pull/37890.
Actually `ExpandingGroupby` and `RollingGroupby` aren't documented in pandas side too. I don't know their intention thought. Let's make a quick followup only for `Rolling.quantile` and `Expanding.quantile` for now
--
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] Yikun commented on a diff in pull request #37836: [SPARK-40339][SPARK-40342][SPARK-40345][SPARK-40348][PS] Implement quantile in Rolling/RollingGroupby/Expanding/ExpandingGroupby
Posted by GitBox <gi...@apache.org>.
Yikun commented on code in PR #37836:
URL: https://github.com/apache/spark/pull/37836#discussion_r970431307
##########
python/pyspark/pandas/window.py:
##########
@@ -561,6 +573,101 @@ def mean(self) -> FrameLike:
"""
return super().mean()
+ def quantile(self, quantile: float, accuracy: int = 10000) -> FrameLike:
+ """
+ Calculate the rolling quantile of the values.
+
+ .. versionadded:: 3.4.0
+
+ Parameters
+ ----------
+ quantile : float
+ Value between 0 and 1 providing the quantile to compute.
+ accuracy : int, optional
+ Default accuracy of approximation. Larger value means better accuracy.
+ The relative error can be deduced by 1.0 / accuracy.
+ This is a panda-on-Spark specific parameter.
+
+ Returns
+ -------
+ Series or DataFrame
+ Returned object type is determined by the caller of the rolling
+ calculation.
+
+ Notes
+ -----
+ `quantile` in pandas-on-Spark are using distributed percentile approximation
+ algorithm unlike pandas, the result might different with pandas, also `interpolation`
+ parameters are not supported yet.
Review Comment:
`parameter is not supported yet`, will address!
Frankly, the `interpolation` almost impossible to support, because we are using distributed percentile approximation algorithm unlike pandas. So we might don't want to add the note.
--
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 #37836: [SPARK-40339][SPARK-40342][SPARK-40345][SPARK-40348][PS] Implement quantile in Rolling/RollingGroupby/Expanding/ExpandingGroupby
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on PR #37836:
URL: https://github.com/apache/spark/pull/37836#issuecomment-1247473695
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] Yikun commented on pull request #37836: [WIP][SPARK-40339][PS] Implement quantile in Rolling/RollingGroupby/Expanding/ExpandingGroupby
Posted by GitBox <gi...@apache.org>.
Yikun commented on PR #37836:
URL: https://github.com/apache/spark/pull/37836#issuecomment-1240644413
Consider test changes is huge, I just spilt it into a separate patch: https://github.com/apache/spark/pull/37835
--
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 #37836: [SPARK-40339][SPARK-40342][SPARK-40345][SPARK-40348][PS] Implement quantile in Rolling/RollingGroupby/Expanding/ExpandingGroupby
Posted by GitBox <gi...@apache.org>.
zhengruifeng commented on PR #37836:
URL: https://github.com/apache/spark/pull/37836#issuecomment-1247546159
> Actually ExpandingGroupby and RollingGroupby aren't documented in pandas side too.
that's true, when I check the missing function in https://issues.apache.org/jira/browse/SPARK-40327, it's quite confusing that there is no document and I need to manually check whether a function exists.
--
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] itholic commented on a diff in pull request #37836: [SPARK-40339][SPARK-40342][SPARK-40345][SPARK-40348][PS] Implement quantile in Rolling/RollingGroupby/Expanding/ExpandingGroupby
Posted by GitBox <gi...@apache.org>.
itholic commented on code in PR #37836:
URL: https://github.com/apache/spark/pull/37836#discussion_r970207129
##########
python/pyspark/pandas/window.py:
##########
@@ -561,6 +573,101 @@ def mean(self) -> FrameLike:
"""
return super().mean()
+ def quantile(self, quantile: float, accuracy: int = 10000) -> FrameLike:
+ """
+ Calculate the rolling quantile of the values.
+
+ .. versionadded:: 3.4.0
+
+ Parameters
+ ----------
+ quantile : float
+ Value between 0 and 1 providing the quantile to compute.
+ accuracy : int, optional
+ Default accuracy of approximation. Larger value means better accuracy.
+ The relative error can be deduced by 1.0 / accuracy.
+ This is a panda-on-Spark specific parameter.
+
+ Returns
+ -------
+ Series or DataFrame
+ Returned object type is determined by the caller of the rolling
+ calculation.
+
+ Notes
+ -----
+ `quantile` in pandas-on-Spark are using distributed percentile approximation
+ algorithm unlike pandas, the result might different with pandas, also `interpolation`
+ parameters are not supported yet.
Review Comment:
nit: "parameters are not supported yet" -> "parameter is not supported yet" ?
Also can we comment it as "TODO" above function definition ?
e.g.
```python
# TODO: support `interpolation` parameter.
def quantile(self, quantile: float, accuracy: int = 10000) -> FrameLike:
...
```
##########
python/pyspark/pandas/window.py:
##########
@@ -561,6 +573,101 @@ def mean(self) -> FrameLike:
"""
return super().mean()
+ def quantile(self, quantile: float, accuracy: int = 10000) -> FrameLike:
+ """
+ Calculate the rolling quantile of the values.
+
+ .. versionadded:: 3.4.0
+
+ Parameters
+ ----------
+ quantile : float
+ Value between 0 and 1 providing the quantile to compute.
+ accuracy : int, optional
+ Default accuracy of approximation. Larger value means better accuracy.
+ The relative error can be deduced by 1.0 / accuracy.
+ This is a panda-on-Spark specific parameter.
+
+ Returns
+ -------
+ Series or DataFrame
+ Returned object type is determined by the caller of the rolling
+ calculation.
+
+ Notes
+ -----
+ `quantile` in pandas-on-Spark are using distributed percentile approximation
+ algorithm unlike pandas, the result might different with pandas, also `interpolation`
+ parameters are not supported yet.
+
+ the current implementation of this API uses Spark's Window without
+ specifying partition specification. This leads to move all data into
+ single partition in single machine and could cause serious
+ performance degradation. Avoid this method against very large dataset.
+
+ See Also
+ --------
+ Series.rolling : Calling object with Series data.
+ DataFrame.rolling : Calling object with DataFrames.
+ Series.quantile : Equivalent method for Series.
+ DataFrame.quantile : Equivalent method for DataFrame.
Review Comment:
Follow the description from pandas' ?
<img width="548" alt="Screen Shot 2022-09-14 at 1 08 58 PM" src="https://user-images.githubusercontent.com/44108233/190057481-0c0c239c-674d-4189-8625-41af2664ae2e.png">
--
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] Yikun commented on a diff in pull request #37836: [SPARK-40339][SPARK-40342][SPARK-40345][SPARK-40348][PS] Implement quantile in Rolling/RollingGroupby/Expanding/ExpandingGroupby
Posted by GitBox <gi...@apache.org>.
Yikun commented on code in PR #37836:
URL: https://github.com/apache/spark/pull/37836#discussion_r970431630
##########
python/pyspark/pandas/window.py:
##########
@@ -561,6 +573,101 @@ def mean(self) -> FrameLike:
"""
return super().mean()
+ def quantile(self, quantile: float, accuracy: int = 10000) -> FrameLike:
+ """
+ Calculate the rolling quantile of the values.
+
+ .. versionadded:: 3.4.0
+
+ Parameters
+ ----------
+ quantile : float
+ Value between 0 and 1 providing the quantile to compute.
+ accuracy : int, optional
+ Default accuracy of approximation. Larger value means better accuracy.
+ The relative error can be deduced by 1.0 / accuracy.
+ This is a panda-on-Spark specific parameter.
+
+ Returns
+ -------
+ Series or DataFrame
+ Returned object type is determined by the caller of the rolling
+ calculation.
+
+ Notes
+ -----
+ `quantile` in pandas-on-Spark are using distributed percentile approximation
+ algorithm unlike pandas, the result might different with pandas, also `interpolation`
+ parameters are not supported yet.
+
+ the current implementation of this API uses Spark's Window without
+ specifying partition specification. This leads to move all data into
+ single partition in single machine and could cause serious
+ performance degradation. Avoid this method against very large dataset.
+
+ See Also
+ --------
+ Series.rolling : Calling object with Series data.
+ DataFrame.rolling : Calling object with DataFrames.
+ Series.quantile : Equivalent method for Series.
+ DataFrame.quantile : Equivalent method for DataFrame.
Review Comment:
will address, thanks!
--
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 #37836: [SPARK-40339][SPARK-40345][SPARK-40345][SPARK-40348][PS] Implement quantile in Rolling/RollingGroupby/Expanding/ExpandingGroupby
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on code in PR #37836:
URL: https://github.com/apache/spark/pull/37836#discussion_r969205126
##########
python/pyspark/pandas/window.py:
##########
@@ -561,6 +573,101 @@ def mean(self) -> FrameLike:
"""
return super().mean()
+ def quantile(self, quantile: float, accuracy: int = 10000) -> FrameLike:
+ """
+ Calculate the rolling quantile of the values.
+
+ .. versionadded:: 3.4.0
+
+ Parameters
+ ----------
+ quantile : float
+ Value between 0 and 1 providing the quantile to compute.
+ accuracy : int, optional
+ Default accuracy of approximation. Larger value means better accuracy.
+ The relative error can be deduced by 1.0 / accuracy.
+ This is a panda-on-Spark specific parameter.
+
+ Returns
+ -------
+ Series or DataFrame
+ Returned object type is determined by the caller of the rolling
+ calculation.
+
+ Notes
+ -------
Review Comment:
```suggestion
-----
```
--
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