Skip to content

[SPARK-21358][Examples] Argument of repartitionandsortwithinpartitions at pyspark#18586

Closed
chie8842 wants to merge 3 commits intoapache:masterfrom
chie8842:SPARK-21358
Closed

[SPARK-21358][Examples] Argument of repartitionandsortwithinpartitions at pyspark#18586
chie8842 wants to merge 3 commits intoapache:masterfrom
chie8842:SPARK-21358

Conversation

@chie8842
Copy link
Contributor

What changes were proposed in this pull request?

At example of repartitionAndSortWithinPartitions at rdd.py, third argument should be True or False.
I proposed fix of example code.

How was this patch tested?

  • I rename test_repartitionAndSortWithinPartitions to test_repartitionAndSortWIthinPartitions_asc to specify boolean argument.
  • I added test_repartitionAndSortWithinPartitions_desc to test False pattern at third argument.

(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)
(If this patch involves UI changes, please attach a screenshot; otherwise, remove this)

Please review http://spark.apache.org/contributing.html before opening a pull request.

@srowen
Copy link
Member

srowen commented Jul 10, 2017

The extra test is nice too, yeah. By the way if you change the title to include [SPARK-21358] instead, it will automatically link to the JIRA.

@chie8842 chie8842 changed the title [Spark 21358][Examples] Argument of repartitionandsortwithinpartitions at pyspark [SPARK-21358][Examples] Argument of repartitionandsortwithinpartitions at pyspark Jul 10, 2017
@SparkQA
Copy link

SparkQA commented Jul 10, 2017

Test build #3840 has finished for PR 18586 at commit fac7a70.

  • This patch fails PySpark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@srowen
Copy link
Member

srowen commented Jul 10, 2017

Your new test looks reasonable, though it failed, because I believe the sorting is only by key. It wouldn't necessarily cause them to be ordered by the second value in the tuple. The example is a little bit misleading because it sort of looks like it is sorting by value too. But, I think you could just adjust the test to expect the actual order as it happens here. The output was sorted by key, correctly.

@chie8842
Copy link
Contributor Author

@srowen Thank you for pointing out about test fail. I fixed it.

@SparkQA
Copy link

SparkQA commented Jul 10, 2017

Test build #3841 has finished for PR 18586 at commit f5804fe.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@rxin
Copy link
Contributor

rxin commented Jul 11, 2017

Merging in master. Thanks.

@asfgit asfgit closed this in c3713fd Jul 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants