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 2021/12/23 12:24:26 UTC
[GitHub] [spark] mslapek opened a new pull request #35000: [WIP][SPARK-37730][PYTHON]
mslapek opened a new pull request #35000:
URL: https://github.com/apache/spark/pull/35000
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
4. Be sure to keep the PR description updated to reflect all changes.
5. Please write your PR title to summarize what this PR proposes.
6. If possible, provide a concise example to reproduce the issue for a faster review.
7. If you want to add a new configuration, please read the guideline first for naming configurations in
'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
8. If you want to add or modify an error type or message, please read the guideline first in
'core/src/main/resources/error/README.md'.
-->
### What changes were proposed in this pull request?
Replace use of MPLPlot._add_legend_handle (removed in pandas) with MPLPlot._add_legend_handle in histogram and KDE plots.
Based on: https://github.com/pandas-dev/pandas/commit/029907c9d69a0260401b78a016a6c4515d8f1c40
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
Fix of SPARK-37730. plot.hist and plot.kde don't throw AttributeError for pandas=1.3.5.
### Does this PR introduce _any_ user-facing change?
No
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
If no, write 'No'.
-->
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
If benchmark tests were added, please run the benchmarks in GitHub Actions for the consistent environment, and the instructions could accord to: https://spark.apache.org/developer-tools.html#github-workflow-benchmarks.
-->
Tested with existing plot test (for older pandas only).
Tested manually in REPL for pandas=1.3.5.
*QUESTION:* Maybe add plot testing for pandas 1.3?
--
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] AmplabJenkins commented on pull request #35000: [SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._append_legend_handles_labels
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #35000:
URL: https://github.com/apache/spark/pull/35000#issuecomment-1000828827
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/51039/
--
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] mslapek edited a comment on pull request #35000: [SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._append_legend_handles_labels
Posted by GitBox <gi...@apache.org>.
mslapek edited a comment on pull request #35000:
URL: https://github.com/apache/spark/pull/35000#issuecomment-1001577739
It seems that CI wants me to reformat file, which I haven't changed: `python/pyspark/shuffle.py`
I've merged main branch.
--
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] AmplabJenkins removed a comment on pull request #35000: [WIP][SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._add_legend_handle
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #35000:
URL: https://github.com/apache/spark/pull/35000#issuecomment-1000283585
Can one of the admins verify this patch?
--
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] SparkQA commented on pull request #35000: [SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._append_legend_handles_labels
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #35000:
URL: https://github.com/apache/spark/pull/35000#issuecomment-1000801391
**[Test build #146564 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146564/testReport)** for PR 35000 at commit [`0f223f7`](https://github.com/apache/spark/commit/0f223f7c889bd61d04cdc781c1c0bcad7a9d765d).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `class PandasOnSparkHistPlot(PandasHistPlot, HistogramPlotBase, _PandasOnSparkKdeHistCommon):`
* `class PandasOnSparkKdePlot(PandasKdePlot, KdePlotBase, _PandasOnSparkKdeHistCommon):`
--
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] SparkQA removed a comment on pull request #35000: [SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._append_legend_handles_labels
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #35000:
URL: https://github.com/apache/spark/pull/35000#issuecomment-1000790083
**[Test build #146564 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146564/testReport)** for PR 35000 at commit [`0f223f7`](https://github.com/apache/spark/commit/0f223f7c889bd61d04cdc781c1c0bcad7a9d765d).
--
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 change in pull request #35000: [SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._append_legend_handles_labels
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #35000:
URL: https://github.com/apache/spark/pull/35000#discussion_r775300162
##########
File path: python/pyspark/pandas/plot/matplotlib.py
##########
@@ -375,6 +398,7 @@ def _make_plot(self):
kwds = self.kwds.copy()
label = pprint_thing(label if len(label) > 1 else label[0])
+ label = self._mark_right_label(label, index=i)
Review comment:
How about this approach? I think this might be simpler.
```suggestion
# Makes plotting compatible with pandas < 1.3
label = self._mark_right_label(
label, index=i) if hasattr(self, "_mark_right_label") else label
```
--
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] SparkQA removed a comment on pull request #35000: [WIP][SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._add_legend_handle
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #35000:
URL: https://github.com/apache/spark/pull/35000#issuecomment-1000283830
**[Test build #146523 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146523/testReport)** for PR 35000 at commit [`2a33641`](https://github.com/apache/spark/commit/2a33641bb4616d65ccc565391e088e8cefc668c5).
--
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 pull request #35000: [SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._append_legend_handles_labels
Posted by GitBox <gi...@apache.org>.
itholic commented on pull request #35000:
URL: https://github.com/apache/spark/pull/35000#issuecomment-1001269140
+1 for https://github.com/apache/spark/pull/35000#discussion_r775300162 and https://github.com/apache/spark/pull/35000#discussion_r775300495.
LGTM, otherwise 👍
--
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] zero323 commented on pull request #35000: [SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._append_legend_handles_labels
Posted by GitBox <gi...@apache.org>.
zero323 commented on pull request #35000:
URL: https://github.com/apache/spark/pull/35000#issuecomment-1001808392
> It seems that CI wants me to reformat file, which I haven't changed: `python/pyspark/shuffle.py`
>
> I've merged main branch.
We've recently updated (ac13494dd040cd904c5c6870489ce628505a9fd7) Black to 21.12b0 and there is change of behavior compared to older releases.
--
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 #35000: [SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._append_legend_handles_labels
Posted by GitBox <gi...@apache.org>.
HyukjinKwon closed pull request #35000:
URL: https://github.com/apache/spark/pull/35000
--
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] AmplabJenkins removed a comment on pull request #35000: [SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._append_legend_handles_labels
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #35000:
URL: https://github.com/apache/spark/pull/35000#issuecomment-1000809552
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146564/
--
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] AmplabJenkins commented on pull request #35000: [SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._append_legend_handles_labels
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #35000:
URL: https://github.com/apache/spark/pull/35000#issuecomment-1000809552
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146564/
--
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] mslapek commented on pull request #35000: [SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._append_legend_handles_labels
Posted by GitBox <gi...@apache.org>.
mslapek commented on pull request #35000:
URL: https://github.com/apache/spark/pull/35000#issuecomment-1001577739
It seems that CI wants me to reformat file, which I haven't changed: `python/pyspark/shuffle.py`
@HyukjinKwon I guess that CI had suddenly updated black. I suggest to fix black version to `21.5b2` (https://github.com/apache/spark/blob/e6865b3209c566d71a8400984a494cf136b89b22/dev/reformat-python).
https://github.com/mslapek/spark/runs/4641242212?check_suite_focus=true
Other people have the same problem: https://github.com/viirya/spark-1/runs/4641255445?check_suite_focus=true
--
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] AmplabJenkins commented on pull request #35000: [WIP][SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._add_legend_handle
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #35000:
URL: https://github.com/apache/spark/pull/35000#issuecomment-1000319524
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146523/
--
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] AmplabJenkins removed a comment on pull request #35000: [WIP][SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._add_legend_handle
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #35000:
URL: https://github.com/apache/spark/pull/35000#issuecomment-1000284771
--
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 #35000: [SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._append_legend_handles_labels
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #35000:
URL: https://github.com/apache/spark/pull/35000#issuecomment-1000722942
You can rebase to the latest master branch, or push an empty commit e.g.) git commit --allow-empty
--
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 #35000: [WIP][SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._add_legend_handle
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #35000:
URL: https://github.com/apache/spark/pull/35000#issuecomment-1000281016
cc @itholic would you mind taking a look 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] mslapek commented on a change in pull request #35000: [SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._append_legend_handles_labels
Posted by GitBox <gi...@apache.org>.
mslapek commented on a change in pull request #35000:
URL: https://github.com/apache/spark/pull/35000#discussion_r775397552
##########
File path: python/pyspark/pandas/plot/matplotlib.py
##########
@@ -375,6 +398,7 @@ def _make_plot(self):
kwds = self.kwds.copy()
label = pprint_thing(label if len(label) > 1 else label[0])
+ label = self._mark_right_label(label, index=i)
Review comment:
Good idea.
After "some time" the part `if hasattr(...) else label` can be just 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
[GitHub] [spark] AmplabJenkins commented on pull request #35000: [SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._append_legend_handles_labels
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #35000:
URL: https://github.com/apache/spark/pull/35000#issuecomment-1000731959
Can one of the admins verify this patch?
--
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] SparkQA commented on pull request #35000: [WIP][SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._add_legend_handle
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #35000:
URL: https://github.com/apache/spark/pull/35000#issuecomment-1000354272
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50999/
--
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] SparkQA commented on pull request #35000: [WIP][SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._add_legend_handle
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #35000:
URL: https://github.com/apache/spark/pull/35000#issuecomment-1000301207
**[Test build #146523 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146523/testReport)** for PR 35000 at commit [`2a33641`](https://github.com/apache/spark/commit/2a33641bb4616d65ccc565391e088e8cefc668c5).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `class PandasOnSparkHistPlot(PandasHistPlot, HistogramPlotBase, _PandasOnSparkKdeHistCommon):`
* `class PandasOnSparkKdePlot(PandasKdePlot, KdePlotBase, _PandasOnSparkKdeHistCommon):`
--
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] SparkQA commented on pull request #35000: [SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._append_legend_handles_labels
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #35000:
URL: https://github.com/apache/spark/pull/35000#issuecomment-1000805710
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/51039/
--
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] mslapek commented on pull request #35000: [SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._append_legend_handles_labels
Posted by GitBox <gi...@apache.org>.
mslapek commented on pull request #35000:
URL: https://github.com/apache/spark/pull/35000#issuecomment-1001907648
@HyukjinKwon @itholic Thanks for review and help! :smiley:
--
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] AmplabJenkins removed a comment on pull request #35000: [SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._append_legend_handles_labels
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #35000:
URL: https://github.com/apache/spark/pull/35000#issuecomment-1000731959
Can one of the admins verify this patch?
--
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] SparkQA commented on pull request #35000: [WIP][SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._add_legend_handle
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #35000:
URL: https://github.com/apache/spark/pull/35000#issuecomment-1000283830
**[Test build #146523 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146523/testReport)** for PR 35000 at commit [`2a33641`](https://github.com/apache/spark/commit/2a33641bb4616d65ccc565391e088e8cefc668c5).
--
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] mslapek commented on pull request #35000: [WIP][SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._add_legend_handle
Posted by GitBox <gi...@apache.org>.
mslapek commented on pull request #35000:
URL: https://github.com/apache/spark/pull/35000#issuecomment-1000311839
> Thanks for taking a look at this, @mslapek. Mind enabling GitHub Actions in your forked repository? (see also https://github.com/apache/spark/pull/35000/checks?check_run_id=4617900123). Apache Spark leverages the GitHub Actions resources from author's forked repository.
I've enabled workflow only for "Build and test". :man_facepalming: Now all workflows should be enabled.
--
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] zero323 edited a comment on pull request #35000: [SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._append_legend_handles_labels
Posted by GitBox <gi...@apache.org>.
zero323 edited a comment on pull request #35000:
URL: https://github.com/apache/spark/pull/35000#issuecomment-1001808392
@mslapek
> It seems that CI wants me to reformat file, which I haven't changed: `python/pyspark/shuffle.py`
>
> I've merged main branch.
We've recently updated (ac13494dd040cd904c5c6870489ce628505a9fd7) Black to 21.12b0 and there is change of behavior compared to older releases.
--
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 #35000: [WIP][SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._add_legend_handle
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #35000:
URL: https://github.com/apache/spark/pull/35000#issuecomment-1000280675
Thanks for taking a look at this, @mslapek. Mind enabling GitHub Actions in your forked repository? (see also https://github.com/apache/spark/pull/35000/checks?check_run_id=4617900123). Apache Spark leverages the GitHub Actions resources from author's forked repository.
--
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] AmplabJenkins commented on pull request #35000: [WIP][SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._add_legend_handle
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #35000:
URL: https://github.com/apache/spark/pull/35000#issuecomment-1000283585
Can one of the admins verify this patch?
--
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 #35000: [WIP][SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._add_legend_handle
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #35000:
URL: https://github.com/apache/spark/pull/35000#issuecomment-1000280109
ok to test
--
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] AmplabJenkins removed a comment on pull request #35000: [WIP][SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._add_legend_handle
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #35000:
URL: https://github.com/apache/spark/pull/35000#issuecomment-1000354778
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50999/
--
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] AmplabJenkins removed a comment on pull request #35000: [SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._append_legend_handles_labels
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #35000:
URL: https://github.com/apache/spark/pull/35000#issuecomment-1000828827
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/51039/
--
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] AmplabJenkins commented on pull request #35000: [WIP][SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._add_legend_handle
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #35000:
URL: https://github.com/apache/spark/pull/35000#issuecomment-1000284771
Can one of the admins verify this patch?
--
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] AmplabJenkins commented on pull request #35000: [SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._append_legend_handles_labels
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #35000:
URL: https://github.com/apache/spark/pull/35000#issuecomment-1001040626
Can one of the admins verify this patch?
--
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] AmplabJenkins commented on pull request #35000: [WIP][SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._add_legend_handle
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #35000:
URL: https://github.com/apache/spark/pull/35000#issuecomment-1000354778
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50999/
--
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] SparkQA commented on pull request #35000: [SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._append_legend_handles_labels
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #35000:
URL: https://github.com/apache/spark/pull/35000#issuecomment-1000790083
**[Test build #146564 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146564/testReport)** for PR 35000 at commit [`0f223f7`](https://github.com/apache/spark/commit/0f223f7c889bd61d04cdc781c1c0bcad7a9d765d).
--
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] SparkQA commented on pull request #35000: [WIP][SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._add_legend_handle
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #35000:
URL: https://github.com/apache/spark/pull/35000#issuecomment-1000320039
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50999/
--
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 change in pull request #35000: [SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._append_legend_handles_labels
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #35000:
URL: https://github.com/apache/spark/pull/35000#discussion_r775300495
##########
File path: python/pyspark/pandas/plot/matplotlib.py
##########
@@ -383,7 +407,7 @@ def _make_plot(self):
kwds = self._make_plot_keywords(kwds, y)
artists = self._plot(ax, y, column_num=i, stacking_id=stacking_id, **kwds)
- self._add_legend_handle(artists[0], label, index=i)
+ self._append_legend_handles_labels(artists[0], label)
Review comment:
and maybe:
```suggestion
# Makes plotting compatible with pandas < 1.3, see pandas-dev/pandas#40078.
self._add_legend_handle(artists[0], label, index=i) if hasattr(
self, "_append_legend_handles_labels"
) else self._append_legend_handles_labels(artists[0], label)
```
##########
File path: python/pyspark/pandas/plot/matplotlib.py
##########
@@ -375,6 +398,7 @@ def _make_plot(self):
kwds = self.kwds.copy()
label = pprint_thing(label if len(label) > 1 else label[0])
+ label = self._mark_right_label(label, index=i)
Review comment:
How about this approach? I think this might be simpler.
```suggestion
# Makes plotting compatible with pandas < 1.3, see pandas-dev/pandas#40078.
label = self._mark_right_label(
label, index=i) if hasattr(self, "_mark_right_label") else label
```
--
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 #35000: [SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._append_legend_handles_labels
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #35000:
URL: https://github.com/apache/spark/pull/35000#issuecomment-1001267197
we already use latest pandas always. I suspect it's not uploaded to pypi or github actions cannot reach yet if it uses 1.3.5. matplotlib test case is disabled IIRC in CI so that's probably why the tests pass: https://app.codecov.io/gh/apache/spark/blob/master/python/pyspark/pandas/tests/plot/test_series_plot_matplotlib.py
--
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] mslapek commented on pull request #35000: [SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._append_legend_handles_labels
Posted by GitBox <gi...@apache.org>.
mslapek commented on pull request #35000:
URL: https://github.com/apache/spark/pull/35000#issuecomment-1000710774
One hour ago I got result from GitHub action "Report test results": `Error: no artifacts found`.
@HyukjinKwon Maybe all actions must be manually requested to rerun? :confused:
https://github.com/mslapek/spark/actions/runs/1618397788
--
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] SparkQA commented on pull request #35000: [SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._append_legend_handles_labels
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #35000:
URL: https://github.com/apache/spark/pull/35000#issuecomment-1000826220
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/51039/
--
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] mslapek commented on pull request #35000: [SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._append_legend_handles_labels
Posted by GitBox <gi...@apache.org>.
mslapek commented on pull request #35000:
URL: https://github.com/apache/spark/pull/35000#issuecomment-1001494019
> And could you briefly show what you fixed with Before & After example if possible ??
A brief example is given in the issue: https://issues.apache.org/jira/browse/SPARK-37730
I've run locally matplotlib tests:
```
$ python python/pyspark/pandas/tests/plot/test_series_plot_matplotlib.py
```
# After fix (my latest commit a1ca73d6):
## :heavy_check_mark: New pandas
```
$ python -c "import pandas as pd; print(pd.__version__)"
1.3.5
```
```
$ python python/pyspark/pandas/tests/plot/test_series_plot_matplotlib.py
...
Ran 14 tests in 35.059s
OK
```
## :heavy_check_mark: Old pandas
```
$ python -c "import pandas as pd; print(pd.__version__)"
1.2.5
```
```
$ python python/pyspark/pandas/tests/plot/test_series_plot_matplotlib.py
...
Ran 14 tests in 36.258s
OK
```
# Main branch before fix (commit c0cedb13):
## :x: New pandas
```
$ python -c "import pandas as pd; print(pd.__version__)"
1.3.5
```
```
$ python python/pyspark/pandas/tests/plot/test_series_plot_matplotlib.py
...
ERROR: test_single_value_hist (pyspark.pandas.tests.plot.test_series_plot_matplotlib.SeriesPlotMatplotlibTest)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/develop/repo/spark/python/pyspark/pandas/tests/plot/test_series_plot_matplotlib.py", line 391, in test_single_value_hist
ax2 = psdf["single"].plot.hist()
...
File "/home/develop/repo/spark/python/pyspark/pandas/plot/matplotlib.py", line 386, in _make_plot
self._add_legend_handle(artists[0], label, index=i)
AttributeError: 'PandasOnSparkHistPlot' object has no attribute '_add_legend_handle'
...
======================================================================
FAIL: test_pie_plot (pyspark.pandas.tests.plot.test_series_plot_matplotlib.SeriesPlotMatplotlibTest)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/develop/repo/spark/python/pyspark/pandas/tests/plot/test_series_plot_matplotlib.py", line 137, in test_pie_plot
self.assertEqual(bin1, bin2)
AssertionError: b'iVB[159 chars]dpAABSIUlEQVR4nO3dd3gc1d098DPbtLvqvdlqttwLbhhs[28020 chars]gg==' != b'iVB[159 chars]dpAABRr0lEQVR4nO3dd3xedf3//+e5VvZo2szOdO/SAYWC[27868 chars]gg=='
----------------------------------------------------------------------
Ran 14 tests in 33.338s
FAILED (failures=2, errors=4)
```
## :heavy_check_mark: Old pandas
```
$ python -c "import pandas as pd; print(pd.__version__)"
1.2.5
```
```
$ python python/pyspark/pandas/tests/plot/test_series_plot_matplotlib.py
...
Ran 14 tests in 36.266s
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] HyukjinKwon commented on pull request #35000: [SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._append_legend_handles_labels
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #35000:
URL: https://github.com/apache/spark/pull/35000#issuecomment-1001494412
Thanks for testing it thoroughly. LGTM
--
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 #35000: [SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._append_legend_handles_labels
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #35000:
URL: https://github.com/apache/spark/pull/35000#issuecomment-1001824572
I manually ran the related tests locally, and verified that it 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 commented on pull request #35000: [SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._append_legend_handles_labels
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #35000:
URL: https://github.com/apache/spark/pull/35000#issuecomment-1001824642
@itholic would you mind investigating to enable matplotlib 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] HyukjinKwon commented on a change in pull request #35000: [SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._append_legend_handles_labels
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #35000:
URL: https://github.com/apache/spark/pull/35000#discussion_r775300495
##########
File path: python/pyspark/pandas/plot/matplotlib.py
##########
@@ -383,7 +407,7 @@ def _make_plot(self):
kwds = self._make_plot_keywords(kwds, y)
artists = self._plot(ax, y, column_num=i, stacking_id=stacking_id, **kwds)
- self._add_legend_handle(artists[0], label, index=i)
+ self._append_legend_handles_labels(artists[0], label)
Review comment:
and maybe:
```suggestion
# Makes plotting compatible with pandas < 1.3, see pandas-dev/pandas#40078.
self._append_legend_handles_labels(artists[0], label) if hasattr(
self, "_append_legend_handles_labels"
) else self._add_legend_handle(artists[0], label, index=i)
```
--
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 pull request #35000: [SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._append_legend_handles_labels
Posted by GitBox <gi...@apache.org>.
itholic commented on pull request #35000:
URL: https://github.com/apache/spark/pull/35000#issuecomment-1001266717
> ❓ QUESTION: Maybe add plot testing for pandas 1.3.5 on CI? (I've noticed that CI uses pandas=1.3.4, maybe update it to 1.3.5?)
Yeah, I think we can update the pandas version to 1.3.5 in CI, and add related tests.
And could you briefly show what you fixed with Before & After example if possible ??
--
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 edited a comment on pull request #35000: [SPARK-37730][PYTHON] Replace use of MPLPlot._add_legend_handle with MPLPlot._append_legend_handles_labels
Posted by GitBox <gi...@apache.org>.
itholic edited a comment on pull request #35000:
URL: https://github.com/apache/spark/pull/35000#issuecomment-1001269140
+1 for https://github.com/apache/spark/pull/35000#discussion_r775300162 and https://github.com/apache/spark/pull/35000#discussion_r775300495.
Otherwise, LGTM 👍
--
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