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..262d97b7213 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 @@ -20,15 +20,17 @@ package org.apache.texera.amber.operator.visualization.volcanoPlot import com.fasterxml.jackson.annotation.{JsonProperty, JsonPropertyDescription} +import com.fasterxml.jackson.databind.annotation.JsonDeserialize import com.kjetland.jackson.jsonSchema.annotations.{JsonSchemaInject, 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.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 +71,34 @@ class VolcanoPlotOpDesc extends PythonOperatorDescriptor with PlotlyStandaloneCo @NotNull(message = "P-Value Column cannot be empty") var pvalueColumn: EncodableString = "" + // Numeric: both are compared against the data and the first is log10'd. contentAs + // names the boxed class — Option erases its element type, and a blank must not read as 0. + @JsonProperty(required = false) + @JsonSchemaTitle("Significance threshold") + @JsonPropertyDescription( + "P-value cut-off such as 0.05; draws a dashed horizontal line at -log10 of it" + ) + @JsonSchemaInject(json = """{"exclusiveMinimum": 0, "maximum": 1}""") + @JsonDeserialize(contentAs = classOf[java.lang.Double]) + var significanceThreshold: Option[Double] = None + + @JsonProperty(required = false) + @JsonSchemaTitle("Effect-size threshold") + @JsonPropertyDescription( + "Effect-size cut-off such as 1; draws dashed vertical lines at plus and minus it" + ) + @JsonSchemaInject(json = """{"exclusiveMinimum": 0}""") + @JsonDeserialize(contentAs = classOf[java.lang.Double]) + var effectSizeThreshold: Option[Double] = None + + @JsonProperty(required = false, defaultValue = "false") + @JsonSchemaTitle("Color by significance") + @JsonPropertyDescription( + "Color points Up, Down or Not significant by both thresholds instead of by effect size; " + + "needs both thresholds, otherwise the effect-size coloring is kept" + ) + var colorBySignificance: Boolean = false + override def operatorInfo: OperatorInfo = OperatorInfo.forVisualization( userFriendlyName = "Volcano Plot", @@ -84,6 +114,88 @@ class VolcanoPlotOpDesc extends PythonOperatorDescriptor with PlotlyStandaloneCo Map(operatorInfo.outputPorts.head.id -> outputSchema) } + /** A cut-off is used only when it can be drawn: finite, above zero and, for the + * p-value, at most one. Anything else counts as unset. The schema bounds keep such + * values out of the property panel; this keeps a hand-edited workflow from failing. + */ + private def isUsable(cutoff: Double): Boolean = + !cutoff.isNaN && !cutoff.isInfinite && cutoff > 0 + + private def significanceCutoff: Option[Double] = + significanceThreshold.filter(sig => isUsable(sig) && sig <= 1) + + private def effectCutoff: Option[Double] = effectSizeThreshold.filter(isUsable) + + /** Both cut-offs are needed to call a point Up or Down. */ + private def colorByCategory: Boolean = + colorBySignificance && significanceCutoff.isDefined && effectCutoff.isDefined + + /** Strict comparisons: a point exactly on a cut-off is not significant. */ + private def significanceColumn: PythonTemplateBuilder = + if (colorByCategory) { + val sig = significanceCutoff.get + val eff = effectCutoff.get + pyb"""significant = table[$pvalueColumn] < $sig + | table["Significance"] = np.select( + | [significant & (table[$effectColumn] > $eff), significant & (table[$effectColumn] < -$eff)], + | ["Up", "Down"], + | default="Not significant", + | ) + |""" + } else pyb"" + + // ColorBrewer RdBu end points plus a neutral grey, so the figure reads like other + // volcano-plot tooling; the continuous RdBu scale stays when not coloring by category. + // Shared by both code paths so the exported figure cannot drift from the engine's. + private def categoryColorArgs: PythonLiteral = + """color="Significance", + | color_discrete_map={"Up": "#B2182B", "Down": "#2166AC", "Not significant": "#9E9E9E"}, + | category_orders={"Significance": ["Up", "Down", "Not significant"]},""".stripMargin + + private def colorArgs: PythonTemplateBuilder = + if (colorByCategory) pyb"$categoryColorArgs" + else + pyb"""color=$effectColumn, + | color_continuous_scale="RdBu",""" + + // The exported script's twins of the two helpers above: the same Python with the + // column names as literals rather than decoded, and the very same threshold literals. + private def standaloneSignificanceColumn: String = + if (colorByCategory) { + val sig = significanceCutoff.get + val eff = effectCutoff.get + val pvalueLit = pyStringLiteral(pvalueColumn) + val effectLit = pyStringLiteral(effectColumn) + s"""significant = table[$pvalueLit] < $sig + | table["Significance"] = np.select( + | [significant & (table[$effectLit] > $eff), significant & (table[$effectLit] < -$eff)], + | ["Up", "Down"], + | default="Not significant", + | ) + |""".stripMargin + } else "" + + private def standaloneColorArgs: String = + if (colorByCategory) categoryColorArgs + else + s"""color=${pyStringLiteral(effectColumn)}, + | color_continuous_scale="RdBu",""".stripMargin + + /** One dashed line per cut-off that is set: horizontal at -log10 of the p-value + * cut-off, vertical at plus and minus the effect-size cut-off. + */ + private def thresholdLines: PythonLiteral = { + val lines = scala.collection.mutable.ArrayBuffer[PythonTemplateBuilder]() + significanceCutoff.foreach { sig => + lines += pyb"""fig.add_hline(y=-np.log10($sig), line_dash="dash", line_color="gray")""" + } + effectCutoff.foreach { eff => + lines += pyb"""fig.add_vline(x=$eff, line_dash="dash", line_color="gray")""" + lines += pyb"""fig.add_vline(x=-$eff, line_dash="dash", line_color="gray")""" + } + lines.mkString("\n ") + } + override def generatePythonCode(): String = { pyb""" |from pytexera import * @@ -113,16 +225,16 @@ class VolcanoPlotOpDesc extends PythonOperatorDescriptor with PlotlyStandaloneCo | return | | table["-log10(pvalue)"] = -np.log10(table[$pvalueColumn]) - | + | $significanceColumn | fig = px.scatter( | table, | x=$effectColumn, | y="-log10(pvalue)", | hover_name=table.columns[0], - | color=$effectColumn, - | color_continuous_scale="RdBu", + | $colorArgs | title="Volcano Plot" | ) + | $thresholdLines | | html = plotly.io.to_html(fig, include_plotlyjs='cdn', auto_play=False) | yield {"html-content": html} @@ -152,15 +264,16 @@ class VolcanoPlotOpDesc extends PythonOperatorDescriptor with PlotlyStandaloneCo | output.write(render_error("No rows with valid p-values.")) | else: | table["-log10(pvalue)"] = -np.log10(table[$pvalueLit]) + | $standaloneSignificanceColumn | fig = px.scatter( | table, | x=$effectLit, | y="-log10(pvalue)", | hover_name=table.columns[0], - | color=$effectLit, - | color_continuous_scale="RdBu", + | $standaloneColorArgs | title="Volcano Plot" | ) + | $thresholdLines | fig.write_json(outputJson) | fig.write_html(outputHtml) | print("Volcano plot saved to " + outputHtml)""".stripMargin 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..69f6b3f68e7 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,7 +20,9 @@ package org.apache.texera.amber.operator.visualization.volcanoPlot import org.apache.texera.amber.core.tuple.AttributeType -import org.apache.texera.amber.operator.metadata.OperatorGroupConstants +import org.apache.texera.amber.operator.LogicalOp +import org.apache.texera.amber.operator.metadata.{OperatorGroupConstants, OperatorMetadataGenerator} +import org.apache.texera.amber.util.JSONUtils.objectMapper import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers @@ -33,6 +35,20 @@ class VolcanoPlotOpDescSpec extends AnyFlatSpec with Matchers { op } + private val hline = """fig.add_hline(y=-np.log10(0.05), line_dash="dash", line_color="gray")""" + private val vlines = + """fig.add_vline(x=1.0, line_dash="dash", line_color="gray") + | fig.add_vline(x=-1.0, line_dash="dash", line_color="gray")""".stripMargin + private val discreteMap = + """color_discrete_map={"Up": "#B2182B", "Down": "#2166AC", "Not significant": "#9E9E9E"}""" + + private def assertContinuousColoring(code: String): Unit = { + code should include("""color_continuous_scale="RdBu"""") + code should not include "Significance" + code should not include discreteMap + code should not include "category_orders" + } + "VolcanoPlotOpDesc.operatorInfo" should "advertise the user-friendly name and Scientific group" in { val info = (new VolcanoPlotOpDesc).operatorInfo info.userFriendlyName shouldBe "Volcano Plot" @@ -68,6 +84,335 @@ class VolcanoPlotOpDescSpec extends AnyFlatSpec with Matchers { decodeOccurrences should be >= 2 } + it should "draw no cut-off line and keep the continuous effect-size coloring by default" in { + val code = configured.generatePythonCode() + code should not include "add_hline" + code should not include "add_vline" + assertContinuousColoring(code) + } + + it should "draw a dashed horizontal line at -log10 of the significance threshold" in { + val op = configured + op.significanceThreshold = Some(0.05) + val code = op.generatePythonCode() + code should include(hline) + code should not include "add_vline" + assertContinuousColoring(code) + } + + it should "draw dashed vertical lines at plus and minus the effect-size threshold" in { + val op = configured + op.effectSizeThreshold = Some(1.0) + val code = op.generatePythonCode() + code should include(vlines) + code should not include "add_hline" + assertContinuousColoring(code) + } + + it should "draw both cut-offs while keeping the continuous coloring when the toggle is off" in { + val op = configured + op.significanceThreshold = Some(0.05) + op.effectSizeThreshold = Some(1.0) + val code = op.generatePythonCode() + code should include(hline) + code should include(vlines) + assertContinuousColoring(code) + } + + it should "skip a significance threshold that has no -log10 (zero or negative)" in { + Seq(0.0, -0.01).foreach { bad => + val op = configured + op.significanceThreshold = Some(bad) + op.effectSizeThreshold = Some(1.0) + op.colorBySignificance = true + val code = op.generatePythonCode() + code should not include "add_hline" + code should not include "np.log10(0.0)" + code should not include "np.log10(-0.01)" + // The effect-size lines still draw, and without a usable p-value cut-off + // no point can be called Up or Down, so the coloring stays continuous. + code should include(vlines) + assertContinuousColoring(code) + } + } + + it should "skip an effect-size threshold that is zero or negative" in { + // A negative cut-off would flip the Up and Down comparisons and call small + // effects significant; zero is the same boundary and is ruled out too. + Seq(0.0, -1.0).foreach { bad => + val op = configured + op.significanceThreshold = Some(0.05) + op.effectSizeThreshold = Some(bad) + op.colorBySignificance = true + val code = op.generatePythonCode() + code should not include "add_vline" + code should include(hline) + assertContinuousColoring(code) + } + } + + it should "skip a significance threshold above one, whose line would sit below the axis" in { + val op = configured + op.significanceThreshold = Some(2.0) + op.effectSizeThreshold = Some(1.0) + op.colorBySignificance = true + val code = op.generatePythonCode() + code should not include "add_hline" + code should include(vlines) + assertContinuousColoring(code) + } + + it should "color Up, Down and Not significant when both thresholds are set and the toggle is on" in { + val op = configured + op.significanceThreshold = Some(0.05) + op.effectSizeThreshold = Some(1.0) + op.colorBySignificance = true + val code = op.generatePythonCode() + // Strict comparisons on the selected p-value column and the effect column: + // a point exactly on a cut-off is not significant. + code should include("significant = table[self.decode_python_template(") + code should include("] < 0.05") + code should include("] > 1.0)") + code should include("] < -1.0)") + code should include("""table["Significance"] = np.select(""") + code should include("""["Up", "Down"],""") + code should include("""default="Not significant",""") + code should include("""color="Significance",""") + code should include(discreteMap) + code should include("""category_orders={"Significance": ["Up", "Down", "Not significant"]}""") + code should not include "color_continuous_scale" + code should include(hline) + code should include(vlines) + } + + it should "accept a significance threshold of exactly one, the schema maximum, and draw it at y = 0" in { + val op = configured + op.significanceThreshold = Some(1.0) + op.effectSizeThreshold = Some(1.0) + op.colorBySignificance = true + val code = op.generatePythonCode() + code should include("""fig.add_hline(y=-np.log10(1.0), line_dash="dash", line_color="gray")""") + code should include(vlines) + code should include("""color="Significance",""") + code should include("] < 1.0") + } + + it should "emit every threshold as a literal Python float, exponent forms included" in { + val pyFloat = """^-?\d+(\.\d+)?([eE][-+]?\d+)?$""".r + // The literal after np.log10( or add_vline(x= ; the y column's np.log10(table[...]) does + // not match because its argument does not start with a digit. + val emittedLiterals = """np\.log10\(([0-9.Ee+-]+)\)|add_vline\(x=-?([0-9.Ee+-]+),""".r + Seq( + (1e-300, 1e21, "1.0E-300", "1.0E21"), + (java.lang.Double.MIN_VALUE, 1.0, "4.9E-324", "1.0") + ).foreach { + case (sig, eff, sigLit, effLit) => + val op = configured + op.significanceThreshold = Some(sig) + op.effectSizeThreshold = Some(eff) + op.colorBySignificance = true + val code = op.generatePythonCode() + val literals = emittedLiterals + .findAllMatchIn(code) + .map(m => Option(m.group(1)).getOrElse(m.group(2))) + .toSeq + literals should contain theSameElementsAs Seq(sigLit, effLit, effLit) + literals.foreach(lit => pyFloat.findFirstIn(lit) shouldBe Some(lit)) + // The category comparisons use the very same literals. + code should include(s"] < $sigLit") + code should include(s"] > $effLit)") + code should include(s"] < -$effLit)") + } + } + + it should "fall back to the continuous coloring when the toggle is on but a threshold is missing" in { + val onlySignificance = configured + onlySignificance.significanceThreshold = Some(0.05) + onlySignificance.colorBySignificance = true + val sigCode = onlySignificance.generatePythonCode() + sigCode should include(hline) + sigCode should not include "add_vline" + assertContinuousColoring(sigCode) + + val onlyEffect = configured + onlyEffect.effectSizeThreshold = Some(1.0) + onlyEffect.colorBySignificance = true + val effCode = onlyEffect.generatePythonCode() + effCode should include(vlines) + effCode should not include "add_hline" + assertContinuousColoring(effCode) + + val neither = configured + neither.colorBySignificance = true + assertContinuousColoring(neither.generatePythonCode()) + } + + 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 not include "add_hline" + code should not include "add_vline" + assertContinuousColoring(code) + } + + "VolcanoPlotOpDesc.generateStandaloneCode" should "draw no cut-off line and keep the continuous effect-size coloring by default" in { + val code = configured.generateStandaloneCode() + code should not include "add_hline" + code should not include "add_vline" + assertContinuousColoring(code) + code should include("""color="log2fc",""") + } + + it should "draw the dashed cut-off lines while keeping the continuous coloring when the toggle is off" in { + val op = configured + op.significanceThreshold = Some(0.05) + op.effectSizeThreshold = Some(1.0) + val code = op.generateStandaloneCode() + code should include(hline) + code should include(vlines) + assertContinuousColoring(code) + } + + it should "color Up, Down and Not significant with the column names as literals" in { + val op = configured + op.significanceThreshold = Some(0.05) + op.effectSizeThreshold = Some(1.0) + op.colorBySignificance = true + val code = op.generateStandaloneCode() + code should include("""significant = table["pvalue"] < 0.05""") + code should include( + """[significant & (table["log2fc"] > 1.0), significant & (table["log2fc"] < -1.0)]""" + ) + code should include("""color="Significance",""") + code should include(discreteMap) + code should include("""category_orders={"Significance": ["Up", "Down", "Not significant"]},""") + code should not include """color_continuous_scale="RdBu"""" + code should include(hline) + code should include(vlines) + code should not include "decode_python_template" + } + + it should "make the same cut-off and coloring decisions as the engine" in { + // The export is drawn from the same figure, so for every shape a stored + // threshold can take the two paths must agree on which lines to draw and on + // whether to color by category; a rule applied on one side only would make + // the exported figure differ from the one shown in the workflow. + val cutOffLine = """fig\.add_[hv]line\([^\n]*""".r + Seq( + (None, None, false), + (Some(0.05), Some(1.0), false), + (Some(0.05), Some(1.0), true), + (Some(0.05), None, true), + (Some(0.0), Some(1.0), true), + (Some(1.5), Some(-1.0), true), + (Some(Double.NaN), Some(Double.PositiveInfinity), true), + (Some(1e-300), Some(1e21), true) + ).foreach { + case (sig, eff, toggle) => + val op = configured + op.significanceThreshold = sig + op.effectSizeThreshold = eff + op.colorBySignificance = toggle + val engine = op.generatePythonCode() + val export = op.generateStandaloneCode() + cutOffLine.findAllIn(export).toSeq shouldBe cutOffLine.findAllIn(engine).toSeq + export.contains("Significance") shouldBe engine.contains("Significance") + export.contains(discreteMap) shouldBe engine.contains(discreteMap) + } + } + + 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.generateStandaloneCode() + code shouldBe configured.generateStandaloneCode() + code should not include "add_hline" + code should not include "add_vline" + assertContinuousColoring(code) + } + + /** 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 readOp(json: String): VolcanoPlotOpDesc = + objectMapper + .readValue(s"""{"operatorType":"VolcanoPlot"$json}""", classOf[LogicalOp]) + .asInstanceOf[VolcanoPlotOpDesc] + + "VolcanoPlotOpDesc thresholds" should "deserialize JSON numbers" in { + val op = readOp(""","significanceThreshold":0.05,"effectSizeThreshold":1""") + op.significanceThreshold shouldBe Some(0.05) + op.effectSizeThreshold shouldBe Some(1.0) + } + + it should "deserialize numeric strings" in { + val op = readOp(""","significanceThreshold":"0.05","effectSizeThreshold":"1.5"""") + op.significanceThreshold shouldBe Some(0.05) + op.effectSizeThreshold shouldBe Some(1.5) + } + + it should "read an absent, null or blank value as unset rather than as zero" in { + readOp("").significanceThreshold shouldBe None + readOp("").effectSizeThreshold shouldBe None + val nulls = readOp(""","significanceThreshold":null,"effectSizeThreshold":null""") + nulls.significanceThreshold shouldBe None + nulls.effectSizeThreshold shouldBe None + val blanks = readOp(""","significanceThreshold":"","effectSizeThreshold":""""") + blanks.significanceThreshold shouldBe None + blanks.effectSizeThreshold shouldBe None + } + + it should "hold Doubles, not the raw JSON values" in { + val op = readOp(""","significanceThreshold":"0.05","effectSizeThreshold":"1.5"""") + op.significanceThreshold.map(_ * 2) shouldBe Some(0.1) + op.effectSizeThreshold.map(_ * 2) shouldBe Some(3.0) + } + + it should "treat a non-finite value read from JSON as unset so no Infinity or NaN reaches the Python" in { + Seq(""""Infinity"""", """"-Infinity"""", """"NaN"""").foreach { raw => + val op = readOp( + s""","significanceThreshold":$raw,"effectSizeThreshold":$raw,"colorBySignificance":true""" + ) + // Jackson does read the text as a Double; the operator must be the one to drop it. + op.significanceThreshold.exists(d => d.isNaN || d.isInfinite) shouldBe true + op.effectSizeThreshold.exists(d => d.isNaN || d.isInfinite) shouldBe true + op.effectColumn = "log2fc" + op.pvalueColumn = "pvalue" + val code = op.generatePythonCode() + code should not include "add_hline" + code should not include "add_vline" + code should not include "Infinity" + code should not include "NaN" + assertContinuousColoring(code) + } + } + + "VolcanoPlotOpDesc schema" should "bound both thresholds so the property panel rejects what the operator ignores" in { + val props = OperatorMetadataGenerator + .generateOperatorJsonSchema(classOf[VolcanoPlotOpDesc]) + .path("properties") + props.path("significanceThreshold").path("exclusiveMinimum").asDouble() shouldBe 0.0 + props.path("significanceThreshold").path("maximum").asDouble() shouldBe 1.0 + props.path("effectSizeThreshold").path("exclusiveMinimum").asDouble() shouldBe 0.0 + props.path("effectSizeThreshold").has("maximum") shouldBe false + } + + "VolcanoPlotOpDesc.colorBySignificance" should "default to false and read a saved value" in { + readOp("").colorBySignificance shouldBe false + readOp(""","colorBySignificance":true""").colorBySignificance shouldBe true + readOp(""","colorBySignificance":false""").colorBySignificance shouldBe false + } + 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