(texera) 01/01: feat(operator): reject Count Vectorizer on the Gaussian Naive Bayes operators (#7599)

[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-7599-58112d771762980595dbaac28f24bd8e9239e307
in repository https://gitbox.apache.org/repos/asf/texera.git

commit ab1a8baeb67fd31b44207cf5d0575a529e1e0947
Author: Kary Zheng <15...@users.noreply.github.com>
AuthorDate: Tue Aug 25 00:51:25 2026 +0000

    feat(operator): reject Count Vectorizer on the Gaussian Naive Bayes operators (#7599)
    
    ### What changes were proposed in this PR?
    
    `Gaussian Naive Bayes` and `Training: Gaussian Naive Bayes` offered a
    `Count Vectorizer` switch with no working setting. Turning it on ended
    the execution from inside scikit-learn: `CountVectorizer` emits a sparse
    matrix, and `GaussianNB` validates its input without `accept_sparse`
    because it estimates a mean and a variance per feature, which reads the
    zeros too. The refusal is deliberate on scikit-learn's side, since
    densifying a text matrix with a large vocabulary is what would exhaust
    memory.
    
    It is specific to this estimator, not to the switch. The switch is
    declared once on `SklearnModelOpDesc` and both families splice it into
    the pipeline the same way, so every estimator in both groups inherits it
    whether or not its own can use it.
    
    An estimator that cannot take the sparse matrix now names what to reach
    for instead, and the shared `getOutputSchemas` turns that into a
    compile-time failure naming both. Nothing fires while the switch is off,
    which is its default, so a freshly dropped operator is not reported
    invalid before it is configured.
    
    Hiding the switch would have been the closer match to how the family
    already handles an impossible pairing, since `Tfidf Transformer` and
    `Text Attribute` are hidden when `Count Vectorizer` is off. It is not
    available here: a hide keys on a sibling field's value and is evaluated
    in the frontend, so it cannot be made per estimator on a field the base
    declares once. It would also leave a workflow already saved with the
    switch on still failing at run time, which the compile-time check
    catches.
    
    ### Any related issues, documentation, discussions?
    
    Fixes #7597.
    
    ### How was this PR tested?
    
    - The premise checked against scikit-learn: fitting
    `make_pipeline(CountVectorizer(), Estimator())` on the same documents,
    `GaussianNB` raises the TypeError in the issue while `MultinomialNB`,
    `BernoulliNB` and `ComplementNB` all fit, which is what the message
    points at.
    - Both operators' specs assert the rejection carries the estimator's own
    name and the alternatives.
    - The base's spec covers the three states that matter: an estimator
    naming no alternative still accepts the switch, one naming an
    alternative rejects it, and neither fires while the switch is off.
    - `WorkflowOperator/testOnly *Sklearn*`: 62 suites, 421 tests, none
    failed. `scalafmtCheckAll` and both scalafix checks clean.
    
    ### 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>
---
 .../sklearn/SklearnGaussianNaiveBayesOpDesc.scala  |  6 +++++
 .../operator/sklearn/SklearnModelOpDesc.scala      | 18 ++++++++++++++
 .../SklearnTrainingGaussianNaiveBayesOpDesc.scala  |  5 ++++
 .../SklearnGaussianNaiveBayesOpDescSpec.scala      | 15 ++++++++++++
 .../operator/sklearn/SklearnModelOpDescSpec.scala  | 28 ++++++++++++++++++++++
 ...learnTrainingGaussianNaiveBayesOpDescSpec.scala | 11 +++++++++
 6 files changed, 83 insertions(+)

diff --git a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/sklearn/SklearnGaussianNaiveBayesOpDesc.scala b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/sklearn/SklearnGaussianNaiveBayesOpDesc.scala
index 0b9c9efece..4f2a9b5525 100644
--- a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/sklearn/SklearnGaussianNaiveBayesOpDesc.scala
+++ b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/sklearn/SklearnGaussianNaiveBayesOpDesc.scala
@@ -22,4 +22,10 @@ package org.apache.texera.amber.operator.sklearn
 class SklearnGaussianNaiveBayesOpDesc extends SklearnClassifierOpDesc {
   override def getImportStatements = "from sklearn.naive_bayes import GaussianNB"
   override def getUserFriendlyModelName = "Gaussian Naive Bayes"
+
+  // GaussianNB estimates a mean and a variance for every feature, which reads the
+  // zeros as well, so scikit-learn validates its input without accept_sparse. The
+  // refusal is deliberate: densifying a text matrix is what would exhaust memory.
+  override protected def countVectorizerAlternatives: Option[String] =
+    Some("Multinomial, Bernoulli or Complement Naive Bayes")
 }
diff --git a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/sklearn/SklearnModelOpDesc.scala b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/sklearn/SklearnModelOpDesc.scala
index f7506ffa82..fc834659e4 100644
--- a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/sklearn/SklearnModelOpDesc.scala
+++ b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/sklearn/SklearnModelOpDesc.scala
@@ -117,9 +117,27 @@ abstract class SklearnModelOpDesc extends PythonOperatorDescriptor {
   @JsonIgnore
   def getUserFriendlyModelName: String
 
+  /** An estimator that cannot be fitted on the sparse matrix `CountVectorizer`
+    * produces names here what to reach for instead, and turning the switch on
+    * stops the workflow at compile time rather than inside scikit-learn.
+    *
+    * The switch is declared on this base, so every estimator in both families
+    * inherits it whether or not its own can use it.
+    */
+  @JsonIgnore
+  protected def countVectorizerAlternatives: Option[String] = None
+
   override def getOutputSchemas(
       inputSchemas: Map[PortIdentity, Schema]
   ): Map[PortIdentity, Schema] = {
+    if (countVectorizer) {
+      countVectorizerAlternatives.foreach { alternatives =>
+        throw new RuntimeException(
+          s"$getUserFriendlyModelName cannot be trained on the sparse matrix Count Vectorizer" +
+            s" produces. Turn Count Vectorizer off, or use $alternatives."
+        )
+      }
+    }
     Map(
       operatorInfo.outputPorts.head.id -> Schema()
         .add("model_name", AttributeType.STRING)
diff --git a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/sklearn/training/SklearnTrainingGaussianNaiveBayesOpDesc.scala b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/sklearn/training/SklearnTrainingGaussianNaiveBayesOpDesc.scala
index 8d4101b268..f0326716e4 100644
--- a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/sklearn/training/SklearnTrainingGaussianNaiveBayesOpDesc.scala
+++ b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/sklearn/training/SklearnTrainingGaussianNaiveBayesOpDesc.scala
@@ -22,4 +22,9 @@ package org.apache.texera.amber.operator.sklearn.training
 class SklearnTrainingGaussianNaiveBayesOpDesc extends SklearnTrainingOpDesc {
   override def getImportStatements = "from sklearn.naive_bayes import GaussianNB"
   override def getUserFriendlyModelName = "Training: Gaussian Naive Bayes"
+
+  // The same estimator as the classifier family's, with the same limitation: a mean
+  // and a variance per feature cannot be read off a sparse matrix without densifying.
+  override protected def countVectorizerAlternatives: Option[String] =
+    Some("Multinomial, Bernoulli or Complement Naive Bayes")
 }
diff --git a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/sklearn/SklearnGaussianNaiveBayesOpDescSpec.scala b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/sklearn/SklearnGaussianNaiveBayesOpDescSpec.scala
index f4cd969c2e..f6020e0a50 100644
--- a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/sklearn/SklearnGaussianNaiveBayesOpDescSpec.scala
+++ b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/sklearn/SklearnGaussianNaiveBayesOpDescSpec.scala
@@ -78,4 +78,19 @@ class SklearnGaussianNaiveBayesOpDescSpec extends AnyFlatSpec with Matchers {
     r.target shouldBe "label"
     r.countVectorizer shouldBe true
   }
+
+  "SklearnGaussianNaiveBayesOpDesc.getOutputSchemas" should
+    "reject Count Vectorizer, naming the estimator and what accepts it instead" in {
+    // GaussianNB is fitted on a mean and a variance per feature, which a sparse
+    // matrix cannot supply without being densified, so this used to end the run
+    // from inside scikit-learn. It stops at compile time now.
+    val d = new SklearnGaussianNaiveBayesOpDesc
+    d.target = "y"
+    d.text = List("note")
+    d.countVectorizer = true
+    val thrown = intercept[RuntimeException](d.getOutputSchemas(Map.empty))
+    thrown.getMessage should include("Gaussian Naive Bayes")
+    thrown.getMessage should include("Count Vectorizer")
+    thrown.getMessage should include("Multinomial, Bernoulli or Complement Naive Bayes")
+  }
 }
diff --git a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/sklearn/SklearnModelOpDescSpec.scala b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/sklearn/SklearnModelOpDescSpec.scala
index d94ae243e7..1bf1ca4c4f 100644
--- a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/sklearn/SklearnModelOpDescSpec.scala
+++ b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/sklearn/SklearnModelOpDescSpec.scala
@@ -75,4 +75,32 @@ class SklearnModelOpDescSpec extends AnyFlatSpec with Matchers {
     )
     d.getOutputSchemas(arbitraryInput) shouldBe fromEmpty
   }
+
+  it should "let Count Vectorizer through for an estimator that named no alternative" in {
+    // The default, and what every estimator but GaussianNB relies on: the sparse
+    // matrix is what the others accept.
+    val d = new TestSklearnModelOpDesc
+    d.countVectorizer = true
+    d.getOutputSchemas(Map.empty).keySet shouldBe Set(d.operatorInfo.outputPorts.head.id)
+  }
+
+  it should "reject Count Vectorizer for an estimator that named one" in {
+    val d = new TestSklearnModelOpDesc {
+      override protected def countVectorizerAlternatives: Option[String] = Some("Some Other Model")
+    }
+    d.countVectorizer = true
+    val thrown = intercept[RuntimeException](d.getOutputSchemas(Map.empty))
+    thrown.getMessage should include("Test Model")
+    thrown.getMessage should include("Count Vectorizer")
+    thrown.getMessage should include("Some Other Model")
+  }
+
+  it should "stay silent while Count Vectorizer is off, whatever the estimator" in {
+    // The switch defaults to off, so a freshly dropped operator must not be
+    // reported invalid before anyone has configured it.
+    val d = new TestSklearnModelOpDesc {
+      override protected def countVectorizerAlternatives: Option[String] = Some("Some Other Model")
+    }
+    d.getOutputSchemas(Map.empty).keySet shouldBe Set(d.operatorInfo.outputPorts.head.id)
+  }
 }
diff --git a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/sklearn/training/SklearnTrainingGaussianNaiveBayesOpDescSpec.scala b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/sklearn/training/SklearnTrainingGaussianNaiveBayesOpDescSpec.scala
index 045ae4d979..101f880387 100644
--- a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/sklearn/training/SklearnTrainingGaussianNaiveBayesOpDescSpec.scala
+++ b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/sklearn/training/SklearnTrainingGaussianNaiveBayesOpDescSpec.scala
@@ -76,4 +76,15 @@ class SklearnTrainingGaussianNaiveBayesOpDescSpec extends AnyFlatSpec with Match
     r.target shouldBe "label"
     r.countVectorizer shouldBe true
   }
+
+  "SklearnTrainingGaussianNaiveBayesOpDesc.getOutputSchemas" should
+    "reject Count Vectorizer the same way its classifier counterpart does" in {
+    val d = new SklearnTrainingGaussianNaiveBayesOpDesc
+    d.target = "y"
+    d.text = List("note")
+    d.countVectorizer = true
+    val thrown = intercept[RuntimeException](d.getOutputSchemas(Map.empty))
+    thrown.getMessage should include("Training: Gaussian Naive Bayes")
+    thrown.getMessage should include("Multinomial, Bernoulli or Complement Naive Bayes")
+  }
 }