diff --git a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/KNNTrainer/SklearnAdvancedKNNParameters.java b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/KNNTrainer/SklearnAdvancedKNNParameters.java index 7bb8c9dd9ac..9ab29dbe618 100644 --- a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/KNNTrainer/SklearnAdvancedKNNParameters.java +++ b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/KNNTrainer/SklearnAdvancedKNNParameters.java @@ -27,8 +27,12 @@ public enum SklearnAdvancedKNNParameters implements ParamClass { weights("weights", "str"), algorithm("algorithm", "str"), leaf_size("leaf_size", "int"), - metric("metric", "int"), - metric_params("metric_params", "str"); + // A metric is named, not measured: "minkowski" and the rest of the accepted + // set are words, so int() rejects every value scikit-learn would take. + metric("metric", "str"), + // The only one that is not a scalar. scikit-learn wants a mapping of extra + // keyword arguments for the metric, so the user's text is read as JSON. + metric_params("metric_params", "json.loads"); private final String name; private final String type; diff --git a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/base/SklearnAdvancedBaseDesc.scala b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/base/SklearnAdvancedBaseDesc.scala index 3127fa91232..87a59bdcf25 100644 --- a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/base/SklearnAdvancedBaseDesc.scala +++ b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/base/SklearnAdvancedBaseDesc.scala @@ -98,10 +98,14 @@ abstract class SklearnMLOperatorDescriptor[T <: ParamClass] extends PythonOperat val stringList = getParameter(paraList) val trainingParam = stringList(1) val paramString = stringList(0) + // A hyperparameter's declared type is emitted as the callable that converts the + // user's text, and the one taking a mapping names json.loads, so json is imported + // whether or not the operator at hand offers such a parameter. val finalCode = pyb""" |from pytexera import * | + |import json |import pandas as pd |${getImportStatements} | diff --git a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/KNNTrainer/SklearnAdvancedKNNClassifierTrainerOpDescSpec.scala b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/KNNTrainer/SklearnAdvancedKNNClassifierTrainerOpDescSpec.scala index 7c62b0cf2c7..7d3acae13e1 100644 --- a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/KNNTrainer/SklearnAdvancedKNNClassifierTrainerOpDescSpec.scala +++ b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/KNNTrainer/SklearnAdvancedKNNClassifierTrainerOpDescSpec.scala @@ -19,11 +19,33 @@ package org.apache.texera.amber.operator.machineLearning.sklearnAdvanced.KNNTrainer -import org.apache.texera.amber.operator.machineLearning.sklearnAdvanced.base.SklearnMLOperatorDescriptor +import org.apache.texera.amber.operator.machineLearning.sklearnAdvanced.base.{ + HyperParameters, + SklearnMLOperatorDescriptor +} import org.scalatest.flatspec.AnyFlatSpec class SklearnAdvancedKNNClassifierTrainerOpDescSpec extends AnyFlatSpec { + private def hyperParam( + parameter: SklearnAdvancedKNNParameters, + value: String + ): HyperParameters[SklearnAdvancedKNNParameters] = { + val hp = new HyperParameters[SklearnAdvancedKNNParameters] + hp.parameter = parameter + hp.parametersSource = false + hp.value = value + hp + } + + private def codeFor(paraList: HyperParameters[SklearnAdvancedKNNParameters]*): String = { + val d = new SklearnAdvancedKNNClassifierTrainerOpDesc + d.paraList = paraList.toList + d.selectedFeatures = List("f1") + d.groundTruthAttribute = "label" + d.generatePythonCode() + } + "SklearnAdvancedKNNClassifierTrainerOpDesc.getImportStatements" should "return the canonical KNeighborsClassifier import" in { val d = new SklearnAdvancedKNNClassifierTrainerOpDesc @@ -58,4 +80,35 @@ class SklearnAdvancedKNNClassifierTrainerOpDescSpec extends AnyFlatSpec { } assert(matched) } + + // The declared type is emitted as the callable that converts what the user + // typed, so a hyperparameter is only usable when that callable can return + // something scikit-learn accepts. These two could not: `metric` names a word + // from a fixed set, and `metric_params` a mapping. + "SklearnAdvancedKNNParameters.metric" should + "convert with str, the accepted metrics being words" in { + assert( + codeFor(hyperParam(SklearnAdvancedKNNParameters.metric, "minkowski")) + .contains("metric = str (") + ) + } + + "SklearnAdvancedKNNParameters.metric_params" should + "convert with json.loads, and the template must import json for it" in { + val code = codeFor(hyperParam(SklearnAdvancedKNNParameters.metric_params, """{"p": 2}""")) + assert(code.contains("metric_params = json.loads (")) + assert(code.contains("import json")) + } + + it should "apply both conversions in the model call, not only in the summary" in { + val code = codeFor( + hyperParam(SklearnAdvancedKNNParameters.metric, "minkowski"), + hyperParam(SklearnAdvancedKNNParameters.metric_params, """{"p": 2}""") + ) + // The values themselves travel base64-encoded, so what is checked here is + // which callable each one is handed to. + val modelCall = code.linesIterator.find(_.contains("model = KNeighborsClassifier(")).get + assert(modelCall.contains("metric = str (")) + assert(modelCall.contains("metric_params = json.loads (")) + } } diff --git a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/base/SklearnAdvancedBaseDescSpec.scala b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/base/SklearnAdvancedBaseDescSpec.scala index ba620af298a..f649601875f 100644 --- a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/base/SklearnAdvancedBaseDescSpec.scala +++ b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/base/SklearnAdvancedBaseDescSpec.scala @@ -94,6 +94,9 @@ class SklearnAdvancedBaseDescSpec extends AnyFlatSpec with Matchers { val d = newOp(List(hyperParam("n_neighbors", "int", fromWorkflow = false, value = "5"))) val code = d.generatePythonCode() code should include("from pytexera import *") + // Unconditional: a hyperparameter's declared type is emitted as the callable + // that converts the user's text, and the one taking a mapping names json.loads. + code should include("import json") code should include("from sklearn.neighbors import KNeighborsClassifier") code should include("class ProcessTableOperator(UDFTableOperator):") code should include("def process_table(")