srowen commented on a change in pull request #17654: [SPARK-20351] [ML] Add
trait hasTrainingSummary to replace the duplicate code
URL: https://github.com/apache/spark/pull/17654#discussion_r241761507
##########
File path:
mllib/src/main/scala/org/apache/spark/ml/regression/LinearRegression.scala
##########
@@ -647,33 +647,20 @@ class LinearRegressionModel private[ml] (
@Since("1.3.0") val intercept: Double,
@Since("2.3.0") val scale: Double)
extends RegressionModel[Vector, LinearRegressionModel]
- with LinearRegressionParams with GeneralMLWritable {
+ with LinearRegressionParams with GeneralMLWritable
+ with HasTrainingSummary[LinearRegressionTrainingSummary] {
private[ml] def this(uid: String, coefficients: Vector, intercept: Double) =
this(uid, coefficients, intercept, 1.0)
- private var trainingSummary: Option[LinearRegressionTrainingSummary] = None
-
override val numFeatures: Int = coefficients.size
/**
* Gets summary (e.g. residuals, mse, r-squared ) of model on training set.
An exception is
* thrown if `trainingSummary == None`.
*/
@Since("1.5.0")
- def summary: LinearRegressionTrainingSummary = trainingSummary.getOrElse {
- throw new SparkException("No training summary available for this
LinearRegressionModel")
- }
-
- private[regression]
- def setSummary(summary: Option[LinearRegressionTrainingSummary]): this.type
= {
- this.trainingSummary = summary
- this
- }
-
- /** Indicates whether a training summary exists for this model instance. */
- @Since("1.5.0")
- def hasSummary: Boolean = trainingSummary.isDefined
+ override def summary: LinearRegressionTrainingSummary = super.summary
Review comment:
On the one hand, you don't need these overrides for this to work correctly,
right? but I suppose it's necessary to preserve the `@Since` tag, which varies
across implementations. But these were mostly introduced in 1.5.0, and where
they have a later `@Since` tag, it matches when the class was introduced. I
think it would also be coherent, for Spark 3.0, to remove these overrides, and
mark the methods in the new trait as `@Since 1.5.0`. The result would be
similar to what would happen if this had been introduced at the start. I don't
feel strongly about it but what do you think? would clean up the code a little
more.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
