diff --git a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/dendrogram/DendrogramOpDesc.scala b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/dendrogram/DendrogramOpDesc.scala index b903c477b13..f60eab6b3e5 100644 --- a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/dendrogram/DendrogramOpDesc.scala +++ b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/dendrogram/DendrogramOpDesc.scala @@ -20,10 +20,11 @@ package org.apache.texera.amber.operator.visualization.dendrogram import com.fasterxml.jackson.annotation.{JsonProperty, JsonPropertyDescription} +import com.fasterxml.jackson.databind.annotation.JsonDeserialize import com.kjetland.jackson.jsonSchema.annotations.JsonSchemaTitle import org.apache.texera.amber.core.tuple.{AttributeType, Schema} import org.apache.texera.amber.pybuilder.PythonTemplateBuilder.PythonTemplateBuilderStringContext -import org.apache.texera.amber.pybuilder.PyStringTypes.EncodableString +import org.apache.texera.amber.pybuilder.PyStringTypes.{EncodableString, PythonLiteral} import org.apache.texera.amber.core.workflow.PortIdentity import org.apache.texera.amber.operator.PythonOperatorDescriptor import org.apache.texera.amber.operator.metadata.annotations.AutofillAttributeName @@ -54,10 +55,13 @@ class DendrogramOpDesc extends PythonOperatorDescriptor { @NotNull(message = "Labels cannot be empty") var labels: EncodableString = "" + // Numeric: scipy compares it against the linkage distances. contentAs names the + // boxed class — Option erases its element type, and a blank must not read as 0. @JsonProperty(defaultValue = "", required = false) @JsonSchemaTitle("Color Threshold") @JsonPropertyDescription("Value at which separation of clusters will be made") - var threshold: EncodableString = "" + @JsonDeserialize(contentAs = classOf[java.lang.Double]) + var threshold: Option[Double] = None override def getOutputSchemas( inputSchemas: Map[PortIdentity, Schema] @@ -78,17 +82,15 @@ class DendrogramOpDesc extends PythonOperatorDescriptor { assert(xVal.nonEmpty, "Value X Column cannot be empty") assert(yVal.nonEmpty, "Value Y Column cannot be empty") assert(labels.nonEmpty, "Labels cannot be empty") - val strippedThreshold: EncodableString = threshold.trim - val isThreshold = - if (strippedThreshold.nonEmpty) pyb"color_threshold=$strippedThreshold" - else "color_threshold=None" + // Unset means None, which is scipy's own 0.7 * max distance. + val thresholdExpr: PythonLiteral = threshold.map(_.toString).getOrElse("None") pyb""" | x = np.array(table[$xVal]) | y = np.array(table[$yVal]) | data = np.column_stack((x, y)) | labels = table[$labels].tolist() | - | fig = ff.create_dendrogram(data, labels=labels, $isThreshold) + | fig = ff.create_dendrogram(data, labels=labels, color_threshold=$thresholdExpr) | fig.update_layout(yaxis_title="Linkage Distance", margin=dict(l=0, r=0, b=0, t=0)) |""" } diff --git a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/dendrogram/DendrogramOpDescSpec.scala b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/dendrogram/DendrogramOpDescSpec.scala index 448693cab7b..b56ffae273a 100644 --- a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/dendrogram/DendrogramOpDescSpec.scala +++ b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/dendrogram/DendrogramOpDescSpec.scala @@ -19,6 +19,8 @@ package org.apache.texera.amber.operator.visualization.dendrogram +import org.apache.texera.amber.operator.LogicalOp +import org.apache.texera.amber.util.JSONUtils.objectMapper import org.scalatest.BeforeAndAfter import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers @@ -83,13 +85,41 @@ class DendrogramOpDescSpec extends AnyFlatSpec with BeforeAndAfter with Matchers code should include("color_threshold=None") } - it should "generate python code carrying a non-empty threshold when configured" in { + it should "generate python code passing a configured threshold as a number" in { opDesc.xVal = "coord_a" opDesc.yVal = "coord_b" opDesc.labels = "label_col" - opDesc.threshold = "42.5" + opDesc.threshold = Some(42.5) val code = opDesc.generatePythonCode() - assert(carries(code, "42.5")) + // A number, not a decoded string: a string raises inside scipy. + code should include("color_threshold=42.5") code should not include "color_threshold=None" } + + /** Reads the shapes a stored workflow can hold. Without `contentAs` a JSON string + * stays unconverted inside the Option and the first use throws. + */ + private def readThreshold(json: String): Option[Double] = + objectMapper + .readValue(s"""{"operatorType":"Dendrogram"$json}""", classOf[LogicalOp]) + .asInstanceOf[DendrogramOpDesc] + .threshold + + "DendrogramOpDesc.threshold" should "deserialize a JSON number" in { + readThreshold(""","threshold":42.5""") shouldBe Some(42.5) + } + + it should "deserialize the numeric string a workflow saved before the field was numeric" in { + readThreshold(""","threshold":"42.5"""") shouldBe Some(42.5) + } + + it should "read an absent, null or blank value as unset rather than as zero" in { + readThreshold("") shouldBe None + readThreshold(""","threshold":null""") shouldBe None + readThreshold(""","threshold":""""") shouldBe None + } + + it should "hold a Double, not the raw JSON value" in { + readThreshold(""","threshold":"42.5"""").map(_ * 2) shouldBe Some(85.0) + } }