SPARK-3580: New public method for RDD's to have consistent way of obtain...

[patmcdonough]

...ing the number of RDD partitions across languages.

[SparkQA]

Can one of the admins verify this patch?

[laserson]

fyi: I think you referenced the wrong JIRA

[patmcdonough]

thanks for catching that @laserson! Fixed.

[pwendell]

minor style comment but you can just write this like:

def getNumPartitions: Int = partitions.size

[pwendell]

LGTM pending a minor comment and tests. Jenkins, test this please.

[SparkQA]

QA tests have started for PR 2447 at commit de966fd.

• This patch merges cleanly.

[markhamstra]

Including a (pretty obvious) spark-shell example in the scaladoc of a simple RDD method isn't really consistent with the rest of the RDD API documentation.

[pwendell]

yeah I can clean this up along with the other style issue on merge.

[patmcdonough]

Good point, although it's worth noting this was essentially ported directly from the python API (including the doc). Any doc changes should be consistent across both versions if possible.

[SparkQA]

QA tests have finished for PR 2447 at commit de966fd.

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

[pwendell]

Jenkins, retest this please.

[SparkQA]

QA tests have started for PR 2447 at commit afc4e09.

• This patch merges cleanly.

[SparkQA]

QA tests have finished for PR 2447 at commit afc4e09.

• This patch passes unit tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
  • sealed trait Matrix extends Serializable
  • class SparseMatrix(
  • sealed trait Vector extends Serializable

[pwendell]

Jenkins, retest this please.

[pwendell]

Jenkins, retest this please.

[SparkQA]

QA tests have started for PR 2447 at commit afc4e09.

• This patch merges cleanly.

[SparkQA]

QA tests have finished for PR 2447 at commit afc4e09.

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

[andrewor14]

The changes here look fine. @pwendell @markhamstra any additional thoughts? This PR hasn't had much activity for a while.

[srowen]

FWIW I think:

• needs a rebase
• scaladoc should be consistent with others in RDD.scala -- no shell example
• JavaRDD needs this even more since I think you have to call javaRDD.rdd().partitions().length right now
• Shouldn't it be partitions.length since it's an array? size works but requires an implicit conversion I think. While we're bothering, might as well do the direct efficient thing

The one bummer here is that there are 20-30 usages of partitions.size in the code already. It's not hard to make those use the method. The usual worry about annoying merge conflicts rears its head though. Still it feels worth doing right if at all?

[andrewor14]

Yeah I think it's still worth doing just to enforce consistent APIs across python and Scala. Merge conflicts may be a little annoying but they shouldn't stop us from bringing the Scala API to parity. Also I think it's fine to continue to use the old way internally since the motivation here is simply to expose it as a public API. I agree that we should also do the same for Java while we're at it.

[srowen]

@patmcdonough are you in a position to follow up on the comments above? I'm wondering if this is alive or not or whether it should be closed.

[patmcdonough]

Thanks for following up on this @srowen - I didn't even realize it's still open.

I'll close this out in favor of somebody issuing a new patch as I'm not in a position to address the comments right now (a lot can happen in 6 months I guess). Please shout at me and suggest otherwise if necessary.

CC: @andrewor14 @pwendell