This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-7234-4f66fd580bfbf6d0ea56902c6ee04103bf06b2a8 in repository https://gitbox.apache.org/repos/asf/texera.git
commit 351ce201e91fa124987502c9235d16a8d07ab61c Author: Kary Zheng <[email protected]> AuthorDate: Mon Aug 3 12:42:32 2026 -0700 fix(dendrogram): pass the color threshold to scipy as a number (#7234) ### What changes were proposed in this PR? Dendrogram's Color Threshold could not be set at all: - the field was declared as a string and spliced into the generated Python as a decode expression, so scipy received `'3'` rather than `3` and raised `UFuncTypeError` comparing it against the linkage distances - the only values that plotted were a blank field and the literal `default`, and scipy documents those as equivalent — both mean 0.7 × max distance — so nothing a user could type ever changed the coloring This PR declares the field as `Option[Double]`: - the number is spliced as a literal, so scipy gets a number - an unset threshold stays `None`, the same 0.7 × max distance a blank field already meant - `@JsonDeserialize(contentAs = ...)` names the boxed class: Scala erases `Option`'s element type, so without it Jackson leaves the raw JSON value inside the Option and the first use throws `ClassCastException`, and the primitive class would read a blank as 0 — every link colored the same rather than "unset" Compatibility: a numeric string saved earlier still reads as a number, and those workflows were failing before this change anyway. A workflow that stored the literal `default` no longer loads; clearing the field plots the identical chart. ### Any related issues, documentation, discussions? Fixes #7232. ### How was this PR tested? - the operator's existing spec updated: a configured threshold is asserted to reach the template as `color_threshold=42.5`, not as a decoded string - deserialization tests for a JSON number, a numeric string, blank, null and absent — plus one that uses the value as a number, the case a round trip cannot catch - ran the generated Python against a pandas DataFrame: `color_threshold=3.0` plots a figure and so does the unset case, while passing the same value as a string, which is what the operator does today, raises `UFuncTypeError` - whole workflow-operator module: 2020 tests passing, `scalafmtCheck` clean ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (claude-opus-5[1m]) --- .../dendrogram/DendrogramOpDesc.scala | 16 +++++----- .../dendrogram/DendrogramOpDescSpec.scala | 36 ++++++++++++++++++++-- 2 files changed, 42 insertions(+), 10 deletions(-) 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 b903c477b1..f60eab6b3e 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 448693cab7..b56ffae273 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) + } }
