You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@texera.apache.org by gi...@apache.org on 2026/08/07 19:09:14 UTC
(texera) branch gh-readonly-queue/main/pr-7259-ae17c8fafbb213897579a574519004bf0d66ad20 created (now 6daa846410)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a change to branch gh-readonly-queue/main/pr-7259-ae17c8fafbb213897579a574519004bf0d66ad20
in repository https://gitbox.apache.org/repos/asf/texera.git
at 6daa846410 feat(histogram): declare Distribution Type's values as a schema enum (#7259)
This branch includes the following new commits:
new 6daa846410 feat(histogram): declare Distribution Type's values as a schema enum (#7259)
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(texera) 01/01: feat(histogram): declare Distribution Type's values as a schema enum (#7259)
Posted by gi...@apache.org.
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 <15...@users.noreply.github.com>
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) <no...@anthropic.com>
---
.../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="
+ }
}