diff --git a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/volcanoPlot/VolcanoPlotOpDesc.scala b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/volcanoPlot/VolcanoPlotOpDesc.scala index 9ceac44472a..3f8014d84e5 100644 --- a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/volcanoPlot/VolcanoPlotOpDesc.scala +++ b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/volcanoPlot/VolcanoPlotOpDesc.scala @@ -29,6 +29,7 @@ import org.apache.texera.amber.operator.PythonOperatorDescriptor import org.apache.texera.amber.operator.visualization.PlotlyStandaloneCode import org.apache.texera.amber.operator.metadata.annotations.{AutofillAttributeName, SampleColumn} import org.apache.texera.amber.operator.metadata.{OperatorGroupConstants, OperatorInfo} +import org.apache.texera.amber.pybuilder.PythonTemplateBuilder import org.apache.texera.amber.pybuilder.PythonTemplateBuilder.pyStringLiteral import javax.validation.constraints.NotNull @@ -69,6 +70,14 @@ class VolcanoPlotOpDesc extends PythonOperatorDescriptor with PlotlyStandaloneCo @NotNull(message = "P-Value Column cannot be empty") var pvalueColumn: EncodableString = "" + @JsonProperty(required = false) + @JsonSchemaTitle("Hover column") + @JsonPropertyDescription( + "Column value to display when a dot is hovered over; leave empty for the first column" + ) + @AutofillAttributeName + var hoverName: EncodableString = "" + override def operatorInfo: OperatorInfo = OperatorInfo.forVisualization( userFriendlyName = "Volcano Plot", @@ -84,6 +93,39 @@ class VolcanoPlotOpDesc extends PythonOperatorDescriptor with PlotlyStandaloneCo Map(operatorInfo.outputPorts.head.id -> outputSchema) } + /** An empty or blank hover column keeps the first column, which is what saved + * workflows show. One rule for both code paths, so they cannot disagree. + */ + private def hoverColumnSet: Boolean = hoverName.trim.nonEmpty + + private def hoverColumn: PythonTemplateBuilder = + if (hoverColumnSet) pyb"$hoverName" else pyb"table.columns[0]" + + /** A set hover column that the input lacks gets the operator's error page, like the + * two required columns, instead of the ValueError plotly would raise. + */ + private def hoverColumnCheck: PythonTemplateBuilder = + if (hoverColumnSet) + pyb"""if $hoverName not in table.columns: + | yield {"html-content": self.render_error("Missing hover column in table.")} + | return + |""" + else pyb"" + + // The exported script's twins of the two helpers above, with the column name as a + // Python literal rather than decoded. The check is one more `elif` in the script's + // chain, so it ends with its own newline and the template puts `else:` right after it. + private def standaloneHoverColumn: String = + if (hoverColumnSet) pyStringLiteral(hoverName) else "table.columns[0]" + + private def standaloneHoverColumnCheck: String = + if (hoverColumnSet) + s"""elif ${pyStringLiteral(hoverName)} not in in1df.columns: + | with open(outputHtml, "w", encoding="utf-8") as output: + | output.write(render_error("Missing hover column in table.")) + |""".stripMargin + else "" + override def generatePythonCode(): String = { pyb""" |from pytexera import * @@ -105,7 +147,7 @@ class VolcanoPlotOpDesc extends PythonOperatorDescriptor with PlotlyStandaloneCo | if $pvalueColumn not in table.columns or $effectColumn not in table.columns: | yield {"html-content": self.render_error("Missing required columns in table.")} | return - | + | $hoverColumnCheck | # Filter out non-positive p-values to avoid math errors | table = table[table[$pvalueColumn] > 0] | if table.empty: @@ -118,7 +160,7 @@ class VolcanoPlotOpDesc extends PythonOperatorDescriptor with PlotlyStandaloneCo | table, | x=$effectColumn, | y="-log10(pvalue)", - | hover_name=table.columns[0], + | hover_name=$hoverColumn, | color=$effectColumn, | color_continuous_scale="RdBu", | title="Volcano Plot" @@ -145,7 +187,7 @@ class VolcanoPlotOpDesc extends PythonOperatorDescriptor with PlotlyStandaloneCo |elif $pvalueLit not in in1df.columns or $effectLit not in in1df.columns: | with open(outputHtml, "w", encoding="utf-8") as output: | output.write(render_error("Missing required columns in table.")) - |else: + |${standaloneHoverColumnCheck}else: | table = in1df[in1df[$pvalueLit] > 0].copy() | if table.empty: | with open(outputHtml, "w", encoding="utf-8") as output: @@ -156,7 +198,7 @@ class VolcanoPlotOpDesc extends PythonOperatorDescriptor with PlotlyStandaloneCo | table, | x=$effectLit, | y="-log10(pvalue)", - | hover_name=table.columns[0], + | hover_name=$standaloneHoverColumn, | color=$effectLit, | color_continuous_scale="RdBu", | title="Volcano Plot" diff --git a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/volcanoPlot/VolcanoPlotOpDescSpec.scala b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/volcanoPlot/VolcanoPlotOpDescSpec.scala index e85ed589efa..b2722899037 100644 --- a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/volcanoPlot/VolcanoPlotOpDescSpec.scala +++ b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/volcanoPlot/VolcanoPlotOpDescSpec.scala @@ -20,10 +20,15 @@ package org.apache.texera.amber.operator.visualization.volcanoPlot import org.apache.texera.amber.core.tuple.AttributeType +import org.apache.texera.amber.operator.LogicalOp import org.apache.texera.amber.operator.metadata.OperatorGroupConstants +import org.apache.texera.amber.util.JSONUtils.objectMapper import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers +import java.nio.charset.StandardCharsets +import java.util.Base64 + class VolcanoPlotOpDescSpec extends AnyFlatSpec with Matchers { private def configured: VolcanoPlotOpDesc = { @@ -33,6 +38,14 @@ class VolcanoPlotOpDescSpec extends AnyFlatSpec with Matchers { op } + private def b64(s: String): String = + Base64.getEncoder.encodeToString(s.getBytes(StandardCharsets.UTF_8)) + + // Every UI-provided string reaches the generated code as this runtime + // decode expression, never as a literal. + private def decoded(s: String): String = + s"self.decode_python_template('${b64(s)}')" + "VolcanoPlotOpDesc.operatorInfo" should "advertise the user-friendly name and Scientific group" in { val info = (new VolcanoPlotOpDesc).operatorInfo info.userFriendlyName shouldBe "Volcano Plot" @@ -68,6 +81,124 @@ class VolcanoPlotOpDescSpec extends AnyFlatSpec with Matchers { decodeOccurrences should be >= 2 } + private def decodeSites(code: String): Int = + "decode_python_template".r.findAllIn(code).length + + it should "keep the first column as the hover label when no hover column is set" in { + val code = configured.generatePythonCode() + code should include("hover_name=table.columns[0],") + // An empty field adds no decode site of its own, and nothing to check for. + code should not include "hover_name=self.decode_python_template(" + code should not include "Missing hover column" + } + + it should "answer a set hover column that the table lacks with the operator's error page" in { + // Like the two required columns, and unlike plotly's uncaught ValueError. + val op = configured + op.hoverName = "gene" + val code = op.generatePythonCode() + code should include(s"if ${decoded("gene")} not in table.columns:") + code should include("""self.render_error("Missing hover column in table.")""") + } + + it should "treat a whitespace-only hover column as empty" in { + Seq(" ", "\t", " \n ").foreach { blank => + val op = configured + op.hoverName = blank + val code = op.generatePythonCode() + code should include("hover_name=table.columns[0],") + code should not include "Missing hover column" + code should not include decoded(blank) + } + } + + it should "use the configured hover column instead of the first column" in { + val op = configured + op.hoverName = "gene" + val code = op.generatePythonCode() + code should include(s"hover_name=${decoded("gene")},") + code should not include "hover_name=table.columns[0]" + // Exactly two decode sites more than the default: the presence check and the + // hover_name argument, both naming the hover column. + decodeSites(code) shouldBe decodeSites(configured.generatePythonCode()) + 2 + } + + it should "route a hover column with quotes and non-ASCII text through the decoder" in { + val op = configured + op.hoverName = "HOVER_SENT \"gene\" 基因" + val code = op.generatePythonCode() + code should include(s"hover_name=${decoded(op.hoverName)},") + code should not include "HOVER_SENT" + } + + it should "render a workflow saved before this change exactly as a default-constructed operator" in { + val saved = objectMapper + .readValue( + """{"operatorType":"VolcanoPlot","effectColumn":"log2fc","pvalueColumn":"pvalue"}""", + classOf[LogicalOp] + ) + .asInstanceOf[VolcanoPlotOpDesc] + val code = saved.generatePythonCode() + code shouldBe configured.generatePythonCode() + code should include("hover_name=table.columns[0],") + code should not include "Missing hover column" + } + + "VolcanoPlotOpDesc.generateStandaloneCode" should "keep the first column as the hover label when no hover column is set" in { + val code = configured.generateStandaloneCode() + code should include("hover_name=table.columns[0],") + code should not include "Missing hover column" + } + + it should "use the configured hover column as a literal and check that the table has it" in { + val op = configured + op.hoverName = "gene" + val code = op.generateStandaloneCode() + code should include("""hover_name="gene",""") + code should not include "hover_name=table.columns[0]" + // One more branch of the script's column checks, before the row filter and + // right before the drawing branch, so a missing hover column gets the error + // page and not plotly's ValueError. + code should include( + """ output.write(render_error("Missing required columns in table.")) + |elif "gene" not in in1df.columns: + | with open(outputHtml, "w", encoding="utf-8") as output: + | output.write(render_error("Missing hover column in table.")) + |else:""".stripMargin + ) + } + + it should "escape a hover column with quotes or a backslash rather than decode it" in { + // The export has no decoder, so the name itself is in the script: a quote or a + // backslash must come out escaped, or the literal would end early. + val op = configured + op.hoverName = "HOVER_SENT \"gene\" 基因" + val code = op.generateStandaloneCode() + code should include("hover_name=\"HOVER_SENT \\\"gene\\\" 基因\",") + code should include("elif \"HOVER_SENT \\\"gene\\\" 基因\" not in in1df.columns:") + code should not include "decode_python_template" + op.hoverName = "gene\\id" + op.generateStandaloneCode() should include("hover_name=\"gene\\\\id\",") + } + + it should "treat a whitespace-only hover column as empty" in { + Seq(" ", "\t", " \n ").foreach { blank => + val op = configured + op.hoverName = blank + op.generateStandaloneCode() shouldBe configured.generateStandaloneCode() + } + } + + it should "render a workflow saved before this change exactly as a default-constructed operator" in { + val saved = objectMapper + .readValue( + """{"operatorType":"VolcanoPlot","effectColumn":"log2fc","pvalueColumn":"pvalue"}""", + classOf[LogicalOp] + ) + .asInstanceOf[VolcanoPlotOpDesc] + saved.generateStandaloneCode() shouldBe configured.generateStandaloneCode() + } + it should "currently render code even when required fields are empty (no assert guard)" in { // Documents the present behavior: VolcanoPlotOpDesc does not assert on // its required fields inside `generatePythonCode`. An empty