[SPARK-10575][Spark Core]Wrapped RDD.takeSample with Scope

[vinodkc]

Remove return statements in RDD.takeSample and wrap it withScope

[SparkQA]

Test build #42370 has finished for PR 8730 at commit cf2c466.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
  • case class IntersectNode(conf: SQLConf, left: LocalNode, right: LocalNode)
  • case class SampleNode(
  • case class TakeOrderedAndProjectNode(

[SparkQA]

Test build #42378 has finished for PR 8730 at commit afe9396.

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

[SparkQA]

Test build #42381 has finished for PR 8730 at commit c06c6e5.

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

[holdenk]

One thing people have asked me to do is copy the description from JIRA over to the description in the pull request.

[vinodkc]

Can we retest?, it is failing in unrelated module

[holdenk]

yup, just say "jenkins retest this please" in a comment.

[vinodkc]

jenkins retest this please

[SparkQA]

Test build #42399 has finished for PR 8730 at commit c06c6e5.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
  • class MultilayerPerceptronClassifier(JavaEstimator, HasFeaturesCol, HasLabelCol, HasPredictionCol,
  • class MultilayerPerceptronClassificationModel(JavaModel):
  • class MinMaxScaler(JavaEstimator, HasInputCol, HasOutputCol):
  • class MinMaxScalerModel(JavaModel):
  • ("thresholds", "Thresholds in multi-class classification to adjust the probability of " +
  • class HasElasticNetParam(Params):
  • class HasFitIntercept(Params):
  • class HasStandardization(Params):
  • class HasThresholds(Params):
  • thresholds = Param(Params._dummy(), "thresholds", "Thresholds in multi-class classification to adjust the probability of predicting each class. Array must have length equal to the number of classes, with values >= 0. The class with largest value p/t is predicted, where p is the original probability of that class and t is the class' threshold.")
  • self.thresholds = Param(self, "thresholds", "Thresholds in multi-class classification to adjust the probability of predicting each class. Array must have length equal to the number of classes, with values >= 0. The class with largest value p/t is predicted, where p is the original probability of that class and t is the class' threshold.")
  • case class IntersectNode(conf: SQLConf, left: LocalNode, right: LocalNode)
  • case class SampleNode(
  • case class TakeOrderedAndProjectNode(

[SparkQA]

Test build #42398 has finished for PR 8730 at commit c06c6e5.

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

[holdenk]

LGTM, maybe @andrewor14, the author of the TODO this resolves and who seems to have been the last committer to work on takeSample, can take a look at this.

[andrewor14]

Hm, the only difference here is that if num == 0 we still do a count(), whereas before we just return quickly. I think we should preserve the old behavior even though it adds another layer of nesting and unfortunately makes the code harder to read.

[andrewor14]

Thanks for the ping. I was just looking at this in parallel. I had 1 minor comment but otherwise this looks good.

[SparkQA]

Test build #42457 has finished for PR 8730 at commit 7fa497d.

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

[SparkQA]

Test build #42459 has finished for PR 8730 at commit 4e6ce62.

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

[SparkQA]

Test build #42466 has finished for PR 8730 at commit 70de2b4.

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

[andrewor14]

these two lines have a lot of style errors... they should be

if (initialCount == 0) {
  new Array[T](0)
} else {
  ...
}

[andrewor14]

OK I'm merging this after fixing the style myself. @vinodkc thanks for providing the fix.