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/26 13:47:50 UTC
[GitHub] [spark] HyukjinKwon opened a new pull request, #39223: [WIP][SPARK-41717][CONNECT] Implement the command logic for print and _repr_html_
HyukjinKwon opened a new pull request, #39223:
URL: https://github.com/apache/spark/pull/39223
### What changes were proposed in this pull request?
TBD
### Why are the changes needed?
TBD
### Does this PR introduce _any_ user-facing change?
TBD
### How was this patch tested?
TBD
--
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] grundprinzip commented on a diff in pull request #39223: [SPARK-41717][CONNECT] Deduplicate print and _repr_html_ at LogicalPlan
Posted by GitBox <gi...@apache.org>.
grundprinzip commented on code in PR #39223:
URL: https://github.com/apache/spark/pull/39223#discussion_r1057438149
##########
python/pyspark/sql/connect/plan.py:
##########
@@ -265,13 +277,13 @@ class Project(LogicalPlan):
def __init__(self, child: Optional["LogicalPlan"], *columns: "ColumnOrName") -> None:
super().__init__(child)
- self._raw_columns = list(columns)
Review Comment:
Why this change?
--
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 #39223: [SPARK-41717][CONNECT] Deduplicate print and _repr_html_ at LogicalPlan
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on code in PR #39223:
URL: https://github.com/apache/spark/pull/39223#discussion_r1057447849
##########
python/pyspark/sql/connect/plan.py:
##########
@@ -95,14 +96,68 @@ def to_proto(self, session: "SparkConnectClient", debug: bool = False) -> proto.
return plan
- # TODO(SPARK-41717): Implement the command logic for print and _repr_html_
+ def _parameters_to_print(self, parameters: Mapping[str, Any]) -> Mapping[str, Any]:
+ params = {}
Review Comment:
Sure, let me add some docs.
--
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 #39223: [SPARK-41717][CONNECT] Deduplicate print and _repr_html_ at LogicalPlan
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on code in PR #39223:
URL: https://github.com/apache/spark/pull/39223#discussion_r1057448159
##########
python/pyspark/sql/connect/plan.py:
##########
@@ -207,7 +240,7 @@ def print(self, indent: int = 0) -> str:
def _repr_html_(self) -> str:
Review Comment:
This wasn't removed because `LocalRelation` contains `data` that's PyArrow table, and this isn't printed out pretty together with our format out of the box. So it was manually overridden here.
--
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 #39223: [SPARK-41717][CONNECT] Deduplicate print and _repr_html_ at LogicalPlan
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on code in PR #39223:
URL: https://github.com/apache/spark/pull/39223#discussion_r1057448398
##########
python/pyspark/sql/connect/plan.py:
##########
@@ -265,13 +277,13 @@ class Project(LogicalPlan):
def __init__(self, child: Optional["LogicalPlan"], *columns: "ColumnOrName") -> None:
super().__init__(child)
- self._raw_columns = list(columns)
Review Comment:
Because this one was only the exception that does not follow the variable naming style in this file. All are either `name` or `_name`, and the automatic printing logic relies on this naming.
--
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 #39223: [SPARK-41717][CONNECT] Implement the command logic for print and _repr_html_
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on PR #39223:
URL: https://github.com/apache/spark/pull/39223#issuecomment-1365522709
cc @amaliujia @zhengruifeng @grundprinzip 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 a diff in pull request #39223: [SPARK-41717][CONNECT] Deduplicate print and _repr_html_ at LogicalPlan
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on code in PR #39223:
URL: https://github.com/apache/spark/pull/39223#discussion_r1057503071
##########
python/pyspark/sql/connect/plan.py:
##########
@@ -95,14 +96,68 @@ def to_proto(self, session: "SparkConnectClient", debug: bool = False) -> proto.
return plan
- # TODO(SPARK-41717): Implement the command logic for print and _repr_html_
+ def _parameters_to_print(self, parameters: Mapping[str, Any]) -> Mapping[str, Any]:
+ params = {}
Review Comment:
Here https://github.com/apache/spark/pull/39237
--
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] grundprinzip commented on pull request #39223: [SPARK-41717][CONNECT] Deduplicate print and _repr_html_ at LogicalPlan
Posted by GitBox <gi...@apache.org>.
grundprinzip commented on PR #39223:
URL: https://github.com/apache/spark/pull/39223#issuecomment-1365610022
Thanks for doing it. Please prepare a follow-up PR that adds documentation to the parameter generating function because that is not "trivial" and thus deserves more explanation.
--
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 #39223: [SPARK-41717][CONNECT] Deduplicate print and _repr_html_ at LogicalPlan
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on PR #39223:
URL: https://github.com/apache/spark/pull/39223#issuecomment-1365536717
Related tests passed.
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] HyukjinKwon closed pull request #39223: [SPARK-41717][CONNECT] Deduplicate print and _repr_html_ at LogicalPlan
Posted by GitBox <gi...@apache.org>.
HyukjinKwon closed pull request #39223: [SPARK-41717][CONNECT] Deduplicate print and _repr_html_ at LogicalPlan
URL: https://github.com/apache/spark/pull/39223
--
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] grundprinzip commented on a diff in pull request #39223: [SPARK-41717][CONNECT] Deduplicate print and _repr_html_ at LogicalPlan
Posted by GitBox <gi...@apache.org>.
grundprinzip commented on code in PR #39223:
URL: https://github.com/apache/spark/pull/39223#discussion_r1057437411
##########
python/pyspark/sql/connect/plan.py:
##########
@@ -95,14 +96,68 @@ def to_proto(self, session: "SparkConnectClient", debug: bool = False) -> proto.
return plan
- # TODO(SPARK-41717): Implement the command logic for print and _repr_html_
+ def _parameters_to_print(self, parameters: Mapping[str, Any]) -> Mapping[str, Any]:
+ params = {}
Review Comment:
The logic of this method is complicated enough to deserve a docstring.
--
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] grundprinzip commented on a diff in pull request #39223: [SPARK-41717][CONNECT] Deduplicate print and _repr_html_ at LogicalPlan
Posted by GitBox <gi...@apache.org>.
grundprinzip commented on code in PR #39223:
URL: https://github.com/apache/spark/pull/39223#discussion_r1057438069
##########
python/pyspark/sql/connect/plan.py:
##########
@@ -207,7 +240,7 @@ def print(self, indent: int = 0) -> str:
def _repr_html_(self) -> str:
Review Comment:
Why is this method not removed?
--
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