Skip to content

[SPARK-9723][ML] params getordefault should throw more useful error #8567

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 5 commits into from
Closed

[SPARK-9723][ML] params getordefault should throw more useful error #8567

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
98fb736
Make a test for the current state of the world
holdenk Sep 2, 2015
2f6c14d
Try keeping the same exception, but providing a string which explains…
holdenk Sep 2, 2015
1b502b9
Use string interpolation
holdenk Sep 2, 2015
0235a6a
use getOrElse
holdenk Sep 2, 2015
bf324c4
Move logic to correct function (oops)
holdenk Sep 2, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion mllib/src/main/scala/org/apache/spark/ml/param/params.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,8 @@ trait Params extends Identifiable with Serializable {
*/
final def getOrDefault[T](param: Param[T]): T = {
shouldOwn(param)
get(param).orElse(getDefault(param)).get
get(param).orElse(getDefault(param)).getOrElse(
throw new NoSuchElementException(s"Failed to find a default value for ${param.name}"))
}

/** An alias for [[getOrDefault()]]. */
Expand Down
15 changes: 10 additions & 5 deletions mllib/src/test/scala/org/apache/spark/ml/param/ParamsSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ class ParamsSuite extends SparkFunSuite {

assert(inputCol.toString === s"${uid}__inputCol")

intercept[java.util.NoSuchElementException] {
solver.getOrDefault(solver.handleInvalid)
}

intercept[IllegalArgumentException] {
solver.setMaxIter(-1)
}
Expand Down Expand Up @@ -102,12 +106,13 @@ class ParamsSuite extends SparkFunSuite {

test("params") {
val solver = new TestParams()
import solver.{maxIter, inputCol}
import solver.{handleInvalid, maxIter, inputCol}

val params = solver.params
assert(params.length === 2)
assert(params(0).eq(inputCol), "params must be ordered by name")
assert(params(1).eq(maxIter))
assert(params.length === 3)
assert(params(0).eq(handleInvalid), "params must be ordered by name")
assert(params(1).eq(inputCol), "params must be ordered by name")
assert(params(2).eq(maxIter))

assert(!solver.isSet(maxIter))
assert(solver.isDefined(maxIter))
Expand All @@ -122,7 +127,7 @@ class ParamsSuite extends SparkFunSuite {
assert(solver.explainParam(maxIter) ===
"maxIter: maximum number of iterations (>= 0) (default: 10, current: 100)")
assert(solver.explainParams() ===
Seq(inputCol, maxIter).map(solver.explainParam).mkString("\n"))
Seq(handleInvalid, inputCol, maxIter).map(solver.explainParam).mkString("\n"))

assert(solver.getParam("inputCol").eq(inputCol))
assert(solver.getParam("maxIter").eq(maxIter))
Expand Down
5 changes: 3 additions & 2 deletions mllib/src/test/scala/org/apache/spark/ml/param/TestParams.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,12 @@

package org.apache.spark.ml.param

import org.apache.spark.ml.param.shared.{HasInputCol, HasMaxIter}
import org.apache.spark.ml.param.shared.{HasHandleInvalid, HasInputCol, HasMaxIter}
import org.apache.spark.ml.util.Identifiable

/** A subclass of Params for testing. */
class TestParams(override val uid: String) extends Params with HasMaxIter with HasInputCol {
class TestParams(override val uid: String) extends Params with HasHandleInvalid with HasMaxIter
with HasInputCol {

def this() = this(Identifiable.randomUID("testParams"))

Expand Down