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-7259-ae17c8fafbb213897579a574519004bf0d66ad20 in repository https://gitbox.apache.org/repos/asf/texera.git
commit 6daa84641094185690ec1d721f91de29635ebd93 Author: Kary Zheng <[email protected]> AuthorDate: Fri Aug 7 12:08:51 2026 -0700 feat(histogram): declare Distribution Type's values as a schema enum (#7259) ### What changes were proposed in this PR? `marginal` — the **Distribution Type** field — was a plain string whose legal values existed only in its description, so the form rendered a free-text box. It now declares them as a schema `enum`: the empty string, which is the value saved Histograms already hold and which code generation reads as "no marginal plot", plus `rug`, `box`, `violin` and `histogram`. The property editor validates a stored property against this enum, so the empty value has to stay in the list rather than be replaced by a `none` sentinel. The enum exposes `histogram` too. `px.histogram` accepts it and the operator always could, but the description named only three, so it was unreachable in practice. ### Why are the changes needed? The value is interpolated straight into `px.histogram(..., marginal=<value>)`, so a typo reaches plotly and fails inside it with `AttributeError: 'NoneType' object has no attribute 'constructor'` — an error that mentions nothing the user typed. ### Any related issues, documentation, discussions? Closes #7209 ### How was this PR tested? `HistogramChartOpDescSpec`, 7 tests. Two are new: one asserts the generated schema's `marginal` enum is exactly the five values, empty included, since that is what the property editor validates a stored value against; the other deserializes a Histogram saved with `marginal: ""` and asserts the rendered figure carries no `marginal=` argument. `WorkflowOperator/compile`, `scalafmtCheckAll` and `scalafixAll --check` are clean. ### Does this PR introduce any user-facing change? Yes. Distribution Type becomes a dropdown. Saved workflows are unaffected: the stored value, empty or one of `rug`, `box` and `violin`, stays legal and keeps its meaning. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 5) --------- Co-authored-by: Claude Opus 5 (1M context) <[email protected]> --- .../histogram/HistogramChartOpDesc.scala | 10 ++++-- .../histogram/HistogramChartOpDescSpec.scala | 38 ++++++++++++++++++++++ 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/histogram/HistogramChartOpDesc.scala b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/histogram/HistogramChartOpDesc.scala index 5c52925fa0..9c16e43658 100644 --- a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/histogram/HistogramChartOpDesc.scala +++ b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/histogram/HistogramChartOpDesc.scala @@ -20,7 +20,7 @@ package org.apache.texera.amber.operator.visualization.histogram import com.fasterxml.jackson.annotation.{JsonProperty, JsonPropertyDescription} -import com.kjetland.jackson.jsonSchema.annotations.JsonSchemaTitle +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 @@ -51,9 +51,15 @@ class HistogramChartOpDesc extends PythonOperatorDescriptor { @AutofillAttributeName var separateBy: EncodableString = "" + // Empty stays the "no marginal plot" value instead of a `none` sentinel: saved + // workflows already hold it, and a stored value outside the enum would fail the + // property editor's validation. @JsonProperty(required = false, defaultValue = "") @JsonSchemaTitle("Distribution Type") - @JsonPropertyDescription("Distribution type (rug, box, violin).") + @JsonPropertyDescription("Optional marginal plot to display alongside the histogram.") + @JsonSchemaInject( + json = """{ "enum": ["", "rug", "box", "violin", "histogram"], "default": "" }""" + ) var marginal: EncodableString = "" @JsonProperty(required = false) diff --git a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/histogram/HistogramChartOpDescSpec.scala b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/histogram/HistogramChartOpDescSpec.scala index d67937815b..2c34e30d43 100644 --- a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/histogram/HistogramChartOpDescSpec.scala +++ b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/histogram/HistogramChartOpDescSpec.scala @@ -19,12 +19,16 @@ package org.apache.texera.amber.operator.visualization.histogram +import org.apache.texera.amber.operator.LogicalOp +import org.apache.texera.amber.operator.metadata.OperatorMetadataGenerator +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 +import scala.jdk.CollectionConverters._ class HistogramChartOpDescSpec extends AnyFlatSpec with BeforeAndAfter with Matchers { @@ -78,4 +82,38 @@ class HistogramChartOpDescSpec extends AnyFlatSpec with BeforeAndAfter with Matc assert(carries(plain, "hist_facet_col")) assert(carries(plain, "hist_pattern_col")) } + + private def marginalEnum: Seq[String] = + OperatorMetadataGenerator + .generateOperatorJsonSchema(classOf[HistogramChartOpDesc]) + .path("properties") + .path("marginal") + .path("enum") + .elements() + .asScala + .map(_.asText()) + .toSeq + + // The property editor validates a stored value against this enum, so leaving the + // empty value out would flag every workflow saved before the enum existed. + "HistogramChartOpDesc.marginal" should "declare its values, empty included, as a schema enum" in { + marginalEnum should contain theSameElementsAs Seq("", "rug", "box", "violin", "histogram") + } + + it should "read a workflow saved with an empty Distribution Type and omit the argument" in { + val saved = objectMapper + .readValue( + """{"operatorType":"Histogram","value":"hist_value_col","marginal":""}""", + classOf[LogicalOp] + ) + .asInstanceOf[HistogramChartOpDesc] + saved.marginal shouldBe "" + saved.createPlotlyFigure().plain should not include "marginal=" + } + + it should "default to no marginal plot on a freshly dropped operator" in { + opDesc.marginal shouldBe "" + opDesc.value = "hist_value_col" + opDesc.createPlotlyFigure().plain should not include "marginal=" + } }
