You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "xinrong-meng (via GitHub)" <gi...@apache.org> on 2024/10/24 02:08:12 UTC
[PR] [SPARK-49999][PYTHON][CONNECT] Support optional "column" parameter in box, kde and hist plots [spark]
xinrong-meng opened a new pull request, #48628:
URL: https://github.com/apache/spark/pull/48628
### What changes were proposed in this pull request?
Support optional "column" parameter in box, kde and hist plots. Now when column is not provided, all columns Support for the optional “column” parameter has been added in box, kde, and hist plots. Now, when the column is not provided, all columns of valid types (NumericType, DateType, TimestampType) will be used to build the plots.
### Why are the changes needed?
- Reach parity with Pandas (on Spark) default behavior.
- Simplify usage by reducing the need for explicitly specifying columns.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Unit tests.
### Was this patch authored or co-authored using generative AI tooling?
No.
--
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
Re: [PR] [SPARK-49999][PYTHON][CONNECT] Support optional "column" parameter in box, kde and hist plots [spark]
Posted by "xinrong-meng (via GitHub)" <gi...@apache.org>.
xinrong-meng commented on code in PR #48628:
URL: https://github.com/apache/spark/pull/48628#discussion_r1815847602
##########
python/pyspark/sql/plot/plotly.py:
##########
@@ -214,3 +209,35 @@ def plot_histogram(data: "DataFrame", **kwargs: Any) -> "Figure":
fig["layout"]["xaxis"]["title"] = "value"
fig["layout"]["yaxis"]["title"] = "count"
return fig
+
+
+def process_column_param(column: Optional[Union[str, List[str]]], data: "DataFrame") -> List[str]:
+ """
+ Processes the provided column parameter for a DataFrame.
+ - If `column` is None, returns a list of numeric or datetime columns from the DataFrame.
+ - If `column` is a string, converts it to a list first.
+ - If `column` is a list, it checks if all specified columns exist in the DataFrame
+ and are of valid types (NumericType, DateType, or TimestampType).
+ - Raises a PySparkTypeError if any column in the list is not present in the DataFrame
+ or has an invalid type.
+ """
+ valid_types = (NumericType, DateType, TimestampType)
Review Comment:
Good catch!
Box plots seem to support NumericType only.
Density and histogram plots will accept `np.datetime64` in PS, as https://github.com/apache/spark/blob/master/python/pyspark/pandas/plot/core.py#L142.
--
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
Re: [PR] [SPARK-49999][PYTHON][CONNECT] Support optional "column" parameter in box, kde and hist plots [spark]
Posted by "xinrong-meng (via GitHub)" <gi...@apache.org>.
xinrong-meng closed pull request #48628: [SPARK-49999][PYTHON][CONNECT] Support optional "column" parameter in box, kde and hist plots
URL: https://github.com/apache/spark/pull/48628
--
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
Re: [PR] [SPARK-49999][PYTHON][CONNECT] Support optional "column" parameter in box, kde and hist plots [spark]
Posted by "xinrong-meng (via GitHub)" <gi...@apache.org>.
xinrong-meng commented on PR #48628:
URL: https://github.com/apache/spark/pull/48628#issuecomment-2436581452
Hi @Pshak-20000 thank you for reaching out!
A good starting point would be to look at the open issues on the [Apache Spark JIRA](https://issues.apache.org/jira/projects/SPARK/issues).That will give you a good idea of the areas you can contribute to.
Also, participating in code reviews is a great way to get familiar with the codebase.
You can find more detailed guidance on https://spark.apache.org/contributing.html good luck with your journey into Spark contributions!
--
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
Re: [PR] [SPARK-49999][PYTHON][CONNECT] Support optional "column" parameter in box, kde and hist plots [spark]
Posted by "xinrong-meng (via GitHub)" <gi...@apache.org>.
xinrong-meng commented on code in PR #48628:
URL: https://github.com/apache/spark/pull/48628#discussion_r1815847602
##########
python/pyspark/sql/plot/plotly.py:
##########
@@ -214,3 +209,35 @@ def plot_histogram(data: "DataFrame", **kwargs: Any) -> "Figure":
fig["layout"]["xaxis"]["title"] = "value"
fig["layout"]["yaxis"]["title"] = "count"
return fig
+
+
+def process_column_param(column: Optional[Union[str, List[str]]], data: "DataFrame") -> List[str]:
+ """
+ Processes the provided column parameter for a DataFrame.
+ - If `column` is None, returns a list of numeric or datetime columns from the DataFrame.
+ - If `column` is a string, converts it to a list first.
+ - If `column` is a list, it checks if all specified columns exist in the DataFrame
+ and are of valid types (NumericType, DateType, or TimestampType).
+ - Raises a PySparkTypeError if any column in the list is not present in the DataFrame
+ or has an invalid type.
+ """
+ valid_types = (NumericType, DateType, TimestampType)
Review Comment:
Good catch!
Box plots seem to support NumericType only.
Density and histogram plots will accept `np.datetime64` in PS, as https://github.com/apache/spark/blob/master/python/pyspark/pandas/plot/core.py#L142.
--
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
Re: [PR] [SPARK-49999][PYTHON][CONNECT] Support optional "column" parameter in box, kde and hist plots [spark]
Posted by "Pshak-20000 (via GitHub)" <gi...@apache.org>.
Pshak-20000 commented on PR #48628:
URL: https://github.com/apache/spark/pull/48628#issuecomment-2438663048
Hi @xinrong-meng ,
Thanks for your reply! I'm new here, and I’m eager to contribute. I’ll check out the open issues on the Apache Spark JIRA and the contributing guide.
If there’s anything specific I can help with, please let me know. I’d really appreciate any guidance!
--
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
Re: [PR] [SPARK-49999][PYTHON][CONNECT] Support optional "column" parameter in box, kde and hist plots [spark]
Posted by "xinrong-meng (via GitHub)" <gi...@apache.org>.
xinrong-meng commented on code in PR #48628:
URL: https://github.com/apache/spark/pull/48628#discussion_r1815903298
##########
python/pyspark/sql/plot/plotly.py:
##########
@@ -214,3 +209,35 @@ def plot_histogram(data: "DataFrame", **kwargs: Any) -> "Figure":
fig["layout"]["xaxis"]["title"] = "value"
fig["layout"]["yaxis"]["title"] = "count"
return fig
+
+
+def process_column_param(column: Optional[Union[str, List[str]]], data: "DataFrame") -> List[str]:
+ """
+ Processes the provided column parameter for a DataFrame.
+ - If `column` is None, returns a list of numeric or datetime columns from the DataFrame.
+ - If `column` is a string, converts it to a list first.
+ - If `column` is a list, it checks if all specified columns exist in the DataFrame
+ and are of valid types (NumericType, DateType, or TimestampType).
+ - Raises a PySparkTypeError if any column in the list is not present in the DataFrame
+ or has an invalid type.
+ """
+ valid_types = (NumericType, DateType, TimestampType)
Review Comment:
Modified to accept NumericType data only, 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
Re: [PR] [SPARK-49999][PYTHON][CONNECT] Support optional "column" parameter in box, kde and hist plots [spark]
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #48628:
URL: https://github.com/apache/spark/pull/48628#discussion_r1815844510
##########
python/pyspark/sql/plot/plotly.py:
##########
@@ -214,3 +209,35 @@ def plot_histogram(data: "DataFrame", **kwargs: Any) -> "Figure":
fig["layout"]["xaxis"]["title"] = "value"
fig["layout"]["yaxis"]["title"] = "count"
return fig
+
+
+def process_column_param(column: Optional[Union[str, List[str]]], data: "DataFrame") -> List[str]:
+ """
+ Processes the provided column parameter for a DataFrame.
+ - If `column` is None, returns a list of numeric or datetime columns from the DataFrame.
+ - If `column` is a string, converts it to a list first.
+ - If `column` is a list, it checks if all specified columns exist in the DataFrame
+ and are of valid types (NumericType, DateType, or TimestampType).
+ - Raises a PySparkTypeError if any column in the list is not present in the DataFrame
+ or has an invalid type.
+ """
+ valid_types = (NumericType, DateType, TimestampType)
Review Comment:
Does these plot actually accept `DateType` and `TimestampType`?
it seems we only support `NumericType` in the PS side?
https://github.com/apache/spark/blob/54edfd3cdb1b2325b04734c3b9bdc289598de558/python/pyspark/pandas/plot/plotly.py#L156-L160
--
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
Re: [PR] [SPARK-49999][PYTHON][CONNECT] Support optional "column" parameter in box, kde and hist plots [spark]
Posted by "xinrong-meng (via GitHub)" <gi...@apache.org>.
xinrong-meng commented on PR #48628:
URL: https://github.com/apache/spark/pull/48628#issuecomment-2440353442
Merged to master, 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
Re: [PR] [SPARK-49999][PYTHON][CONNECT] Support optional "column" parameter in box, kde and hist plots [spark]
Posted by "Pshak-20000 (via GitHub)" <gi...@apache.org>.
Pshak-20000 commented on PR #48628:
URL: https://github.com/apache/spark/pull/48628#issuecomment-2435065372
Hi ,
I want to contribute to the project and can help out. Please let me know what to do!
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
Re: [PR] [SPARK-49999][PYTHON][CONNECT] Support optional "column" parameter in box, kde and hist plots [spark]
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #48628:
URL: https://github.com/apache/spark/pull/48628#discussion_r1817989132
##########
python/pyspark/sql/plot/plotly.py:
##########
@@ -214,3 +209,35 @@ def plot_histogram(data: "DataFrame", **kwargs: Any) -> "Figure":
fig["layout"]["xaxis"]["title"] = "value"
fig["layout"]["yaxis"]["title"] = "count"
return fig
+
+
+def process_column_param(column: Optional[Union[str, List[str]]], data: "DataFrame") -> List[str]:
+ """
+ Processes the provided column parameter for a DataFrame.
+ - If `column` is None, returns a list of numeric or datetime columns from the DataFrame.
+ - If `column` is a string, converts it to a list first.
+ - If `column` is a list, it checks if all specified columns exist in the DataFrame
+ and are of valid types (NumericType, DateType, or TimestampType).
+ - Raises a PySparkTypeError if any column in the list is not present in the DataFrame
+ or has an invalid type.
+ """
+ valid_types = (NumericType, DateType, TimestampType)
Review Comment:
let's also adjust the PR description
--
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