This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/release/v1.2/pr-7306-a9f39e2c6616669d470e1f4996d5e54a75b30c2c in repository https://gitbox.apache.org/repos/asf/texera.git
commit 7a5037c7f87fb5697e27358968a1166ad23116bb Author: Kary Zheng <[email protected]> AuthorDate: Tue Aug 4 14:28:13 2026 -0700 fix(dendrogram, v1.2): pass the color threshold to scipy as a number (#7306) ### What changes were proposed in this PR? Backport of #7234 to `release/v1.2`. The automated backport, #7291, landed with cherry-pick conflict markers on a branch in this repository that I cannot push to, so this PR carries the resolved commit from my fork instead. The conflict was in `createDendrogram()`: `release/v1.2` still has the older body, and the cherry-picked commit rewrites the same lines. The resolution takes the cherry-picked side. It is line-for-line identical to what landed on `main`, plus the three assert messages that already exist on `main` and that the accompanying spec asserts on. ### Any related issues, documentation, discussions? Backport of #7234, which closed #7232. Supersedes #7291. ### How was this PR tested? `DendrogramOpDescSpec` on this branch: 9 tests, all passing. `scalafmtCheckAll` and `scalafixAll --check` are both clean. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (claude-opus-5[1m]) --- .../dendrogram/DendrogramOpDesc.scala | 22 ++-- .../dendrogram/DendrogramOpDescSpec.scala | 125 +++++++++++++++++++++ 2 files changed, 137 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 e92281369b..859826fb73 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 @@ -49,10 +50,13 @@ class DendrogramOpDesc extends PythonOperatorDescriptor { @AutofillAttributeName 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] @@ -70,20 +74,18 @@ class DendrogramOpDesc extends PythonOperatorDescriptor { ) private def createDendrogram(): PythonTemplateBuilder = { - assert(xVal.nonEmpty) - assert(yVal.nonEmpty) - assert(labels.nonEmpty) - val strippedThreshold: EncodableString = threshold.trim - val isThreshold = - if (strippedThreshold.nonEmpty) pyb"color_threshold=$strippedThreshold" - else "color_threshold=None" + 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") + // 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 new file mode 100644 index 0000000000..b56ffae273 --- /dev/null +++ b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/dendrogram/DendrogramOpDescSpec.scala @@ -0,0 +1,125 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +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 + +import java.nio.charset.StandardCharsets +import java.util.Base64 + +class DendrogramOpDescSpec extends AnyFlatSpec with BeforeAndAfter with Matchers { + + var opDesc: DendrogramOpDesc = _ + + before { + opDesc = new DendrogramOpDesc() + } + + private def b64(s: String): String = + Base64.getEncoder.encodeToString(s.getBytes(StandardCharsets.UTF_8)) + + private def carries(output: String, name: String): Boolean = + output.contains(name) || output.contains(b64(name)) + + private def fieldPart(msg: String): String = + msg.toLowerCase.replace("cannot be empty", "") + + // createDendrogram() is private; generatePythonCode() is the public + // entry point that reaches its asserts. + it should "throw AssertionError naming the X column when all fields are empty" in { + val ex = intercept[AssertionError](opDesc.generatePythonCode()) + ex.getMessage should not be null + ex.getMessage should include("cannot be empty") + fieldPart(ex.getMessage) should include("x") + } + + it should "throw AssertionError naming the Y column when only xVal and labels are set" in { + opDesc.xVal = "coord_a" + opDesc.labels = "label_col" + val ex = intercept[AssertionError](opDesc.generatePythonCode()) + ex.getMessage should not be null + ex.getMessage should include("cannot be empty") + fieldPart(ex.getMessage) should include("y") + } + + it should "throw AssertionError naming the Labels column when only xVal and yVal are set" in { + opDesc.xVal = "coord_a" + opDesc.yVal = "coord_b" + val ex = intercept[AssertionError](opDesc.generatePythonCode()) + ex.getMessage should not be null + ex.getMessage should include("cannot be empty") + fieldPart(ex.getMessage) should include("label") + } + + it should "generate python code carrying all three configured columns" in { + opDesc.xVal = "coord_a" + opDesc.yVal = "coord_b" + opDesc.labels = "label_col" + val code = opDesc.generatePythonCode() + assert(carries(code, "coord_a")) + assert(carries(code, "coord_b")) + assert(carries(code, "label_col")) + code should include("create_dendrogram") + // empty threshold falls back to color_threshold=None + code should include("color_threshold=None") + } + + 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 = Some(42.5) + val code = opDesc.generatePythonCode() + // 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) + } +}
