[Minor][MLLIB] Refactor toString method in MLLIB

[aihex]

1. predict(predict.toString) has already output prefix “predict” thus it’s duplicated to print ", predict = " again
2. there are some extra spaces

[srowen]

These can use string interpolation. I take your point though it breaks the symmetry a bit and make this toString rely on details of the subclass. How about making the Predict.toString return something more compact like s"$predict ($prob)"?

[aihex]

Modify Predict.scala and decouple the dependency of Node.scala on Predict.scala.

[srowen]

I'm OK with this, though while you're at it, why not make these use string interpolation? s"foo = $foo, ..."

[aihex]

If you're referring to the modification on Predict.scala, I just follow the style of previous code.

[srowen]

Yes, but that's a hold over from Scala 2.9 and before. I think that while we're modifying these strings they can use the 'modern' style used in new code in the project.

[aihex]

Okay, sounds better to use a modern style.

[srowen]

... this still is not using string interpolation everywhere. It's a small thing, but if we're bothering to update these toString methods, go all the way.

[jkbradley]

I'd put a space before the parenthesis: $predict (prob

[aihex]

Get your point and change all toString methods in MLLIB.
There are a large number of uses of 'old' way in the statement like logInfo, which makes it hard to change all them in a moment.

[srowen]

Looks OK, I think Joseph just had one more tiny comment on one toString method, to add a space.

[aihex]

Good to go.
@jkbradley Thank you.

[srowen]

ok to test

[jkbradley]

LGTM pending tests

[SparkQA]

Test build #699 has finished for PR 5687 at commit b6380c6.

• This patch fails Scala style tests.
• This patch merges cleanly.
• This patch adds no public classes.
• This patch adds the following new dependencies:
  • tachyon-0.6.4.jar
  • tachyon-client-0.6.4.jar
• This patch removes the following dependencies:
  • tachyon-0.5.0.jar
  • tachyon-client-0.5.0.jar

[srowen]

@AIHE this fails style checks. I think the lines are too long. Can you fix those up and try ./dev/lint-scala to check it out locally?

[aihex]

@srowen Sure. Pass the style checking and will do that for PRs in the feature.

[SparkQA]

Test build #710 has finished for PR 5687 at commit 9862b9a.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.
• This patch does not change any dependencies.