You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@texera.apache.org by "aglinxinyuan (via GitHub)" <gi...@apache.org> on 2026/06/23 01:27:38 UTC

[PR] test(workflow-operator): add unit test coverage for Hugging Face operator descriptors [texera]

aglinxinyuan opened a new pull request, #5898:
URL: https://github.com/apache/texera/pull/5898

   ### What changes were proposed in this PR?
   
   Pin behavior of four previously-untested Hugging Face operator descriptors in `common/workflow-operator`. No production-code changes.
   
   | Spec | Source class | Tests |
   | --- | --- | --- |
   | `HuggingFaceSentimentAnalysisOpDescSpec` | `HuggingFaceSentimentAnalysisOpDesc` | 7 |
   | `HuggingFaceSpamSMSDetectionOpDescSpec` | `HuggingFaceSpamSMSDetectionOpDesc` | 6 |
   | `HuggingFaceTextSummarizationOpDescSpec` | `HuggingFaceTextSummarizationOpDesc` | 7 |
   | `HuggingFaceIrisLogisticRegressionOpDescSpec` | `HuggingFaceIrisLogisticRegressionOpDesc` | 6 |
   
   **Behavior pinned**
   
   | Surface | Contract |
   | --- | --- |
   | `operatorInfo` | exact name + description; Hugging Face group; 1-in/1-out (Sentiment also `supportReconfiguration`) |
   | field defaults | all column fields default to `null` |
   | `getOutputSchemas` | appended result-column types (Sentiment 3×DOUBLE; Spam BOOLEAN+DOUBLE; Summarization STRING; Iris STRING+DOUBLE) keyed by the declared output port, plus the null/blank result-name guards |
   | `generatePythonCode` | emits the model id + structural fragments and carries the configured columns |
   | `getPhysicalOp` | wires `OpExecWithCode` tagged `"python"` and carries the operator's port identities |
   | Round-trip | config fields preserved through the polymorphic `LogicalOp` base |
   
   Note: column fields are `EncodableString`, so in the emitted (encoded) code they appear as `self.decode_python_template('<base64>')` — assertions use a `carries` helper that accepts the raw name or its base64 form.
   
   ### Any related issues, documentation, discussions?
   
   Part of the ongoing `workflow-operator` unit-test coverage effort (follow-up to #5843, #5844).
   
   ### How was this PR tested?
   
   - `sbt "WorkflowOperator/testOnly *HuggingFaceSentimentAnalysisOpDescSpec *HuggingFaceSpamSMSDetectionOpDescSpec *HuggingFaceTextSummarizationOpDescSpec *HuggingFaceIrisLogisticRegressionOpDescSpec"` — 26 tests, all green
   - `sbt "WorkflowOperator/Test/scalafmtCheck"` and `sbt "WorkflowOperator/scalafixAll --check"` — clean
   - CI to confirm
   
   ### Was this PR authored or co-authored using generative AI tooling?
   
   Generated-by: Claude Code (Opus 4.8 [1M context])
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] test(workflow-operator): add unit test coverage for Hugging Face operator descriptors [texera]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #5898:
URL: https://github.com/apache/texera/pull/5898#discussion_r3456440565


##########
common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/huggingFace/HuggingFaceSpamSMSDetectionOpDescSpec.scala:
##########
@@ -0,0 +1,117 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.texera.amber.operator.huggingFace
+
+import org.apache.texera.amber.core.executor.OpExecWithCode
+import org.apache.texera.amber.core.tuple.{AttributeType, Schema}
+import org.apache.texera.amber.core.virtualidentity.{ExecutionIdentity, WorkflowIdentity}
+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 HuggingFaceSpamSMSDetectionOpDescSpec extends AnyFlatSpec with Matchers {
+
+  private val workflowId = WorkflowIdentity(1L)
+  private val executionId = ExecutionIdentity(1L)
+
+  private def b64(s: String): String =
+    Base64.getEncoder.encodeToString(s.getBytes(StandardCharsets.UTF_8))
+
+  private def carries(output: String, name: String): Boolean =
+    output.contains(name) || output.contains(b64(name))
+

Review Comment:
   `carries` currently accepts `output.contains(name)`, but `text` is already present in the generated Python via the literal task string `"text-classification"`, and `score` is present via `result["score"]`. This means the assertions can pass even if the EncodableString fields were not carried through codegen. Since these fields are `EncodableString` and `.encode` mode base64-wraps them, make `carries` check only the base64 form (or the decode call) to avoid false positives.



##########
common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/huggingFace/HuggingFaceSentimentAnalysisOpDescSpec.scala:
##########
@@ -0,0 +1,130 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.texera.amber.operator.huggingFace
+
+import org.apache.texera.amber.core.executor.OpExecWithCode
+import org.apache.texera.amber.core.tuple.{AttributeType, Schema}
+import org.apache.texera.amber.core.virtualidentity.{ExecutionIdentity, WorkflowIdentity}
+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 HuggingFaceSentimentAnalysisOpDescSpec extends AnyFlatSpec with Matchers {
+
+  private val workflowId = WorkflowIdentity(1L)
+  private val executionId = ExecutionIdentity(1L)
+
+  private def b64(s: String): String =
+    Base64.getEncoder.encodeToString(s.getBytes(StandardCharsets.UTF_8))
+
+  // EncodableString fields render as self.decode_python_template('<base64>') in encode mode.
+  private def carries(output: String, name: String): Boolean =
+    output.contains(name) || output.contains(b64(name))
+

Review Comment:
   `carries` currently accepts `output.contains(name)`, which makes the codegen assertions trivially pass for `pos`/`neu`/`neg` because those substrings already appear in the generated Python (`"positive"`, `"neutral"`, `"negative"`) even if the EncodableString fields were not spliced correctly. Since these fields are `EncodableString` and `.encode` mode always base64-wraps them, the test should assert on the base64 form (or the decode call) only to avoid false positives.



##########
common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/huggingFace/HuggingFaceTextSummarizationOpDescSpec.scala:
##########
@@ -0,0 +1,121 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.texera.amber.operator.huggingFace
+
+import org.apache.texera.amber.core.executor.OpExecWithCode
+import org.apache.texera.amber.core.tuple.{AttributeType, Schema}
+import org.apache.texera.amber.core.virtualidentity.{ExecutionIdentity, WorkflowIdentity}
+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 HuggingFaceTextSummarizationOpDescSpec extends AnyFlatSpec with Matchers {
+
+  private val workflowId = WorkflowIdentity(1L)
+  private val executionId = ExecutionIdentity(1L)
+
+  private def b64(s: String): String =
+    Base64.getEncoder.encodeToString(s.getBytes(StandardCharsets.UTF_8))
+
+  private def carries(output: String, name: String): Boolean =
+    output.contains(name) || output.contains(b64(name))
+

Review Comment:
   `carries` currently accepts `output.contains(name)`, but both `text` and `summary` already appear as local variable names in the generated Python (e.g., `text = ...`, `summary = ...`). That makes the codegen assertions pass even if the EncodableString fields were not spliced correctly. Since these fields are `EncodableString` and `.encode` mode base64-wraps them, make `carries` check only the base64 form (or the decode call) to avoid false positives.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] test(workflow-operator): add unit test coverage for Hugging Face operator descriptors [texera]

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #5898:
URL: https://github.com/apache/texera/pull/5898#issuecomment-4774783575

   ## [Codecov](https://app.codecov.io/gh/apache/texera/pull/5898?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :white_check_mark: All modified and coverable lines are covered by tests.
   :white_check_mark: Project coverage is 54.09%. Comparing base ([`439ea72`](https://app.codecov.io/gh/apache/texera/commit/439ea72e46b78aec7f71e8889225f2c90942a2c2?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`12a13d3`](https://app.codecov.io/gh/apache/texera/commit/12a13d30bddf70a2dae779b07533324ee4208b7d?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@            Coverage Diff            @@
   ##               main    #5898   +/-   ##
   =========================================
     Coverage     54.09%   54.09%           
     Complexity     2817     2817           
   =========================================
     Files          1103     1103           
     Lines         42650    42650           
     Branches       4588     4588           
   =========================================
     Hits          23070    23070           
     Misses        18236    18236           
     Partials       1344     1344           
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/texera/pull/5898/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | | *Carryforward flag |
   |---|---|---|---|
   | [access-control-service](https://app.codecov.io/gh/apache/texera/pull/5898/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `70.44% <ø> (ø)` | | Carriedforward from [439ea72](https://app.codecov.io/gh/apache/texera/commit/439ea72e46b78aec7f71e8889225f2c90942a2c2?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [agent-service](https://app.codecov.io/gh/apache/texera/pull/5898/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `34.36% <ø> (ø)` | | Carriedforward from [439ea72](https://app.codecov.io/gh/apache/texera/commit/439ea72e46b78aec7f71e8889225f2c90942a2c2?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [amber](https://app.codecov.io/gh/apache/texera/pull/5898/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.61% <ø> (ø)` | | Carriedforward from [439ea72](https://app.codecov.io/gh/apache/texera/commit/439ea72e46b78aec7f71e8889225f2c90942a2c2?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [computing-unit-managing-service](https://app.codecov.io/gh/apache/texera/pull/5898/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `1.65% <ø> (ø)` | | Carriedforward from [439ea72](https://app.codecov.io/gh/apache/texera/commit/439ea72e46b78aec7f71e8889225f2c90942a2c2?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [config-service](https://app.codecov.io/gh/apache/texera/pull/5898/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.35% <ø> (ø)` | | |
   | [file-service](https://app.codecov.io/gh/apache/texera/pull/5898/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `58.59% <ø> (ø)` | | Carriedforward from [439ea72](https://app.codecov.io/gh/apache/texera/commit/439ea72e46b78aec7f71e8889225f2c90942a2c2?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [frontend](https://app.codecov.io/gh/apache/texera/pull/5898/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `48.09% <ø> (ø)` | | Carriedforward from [439ea72](https://app.codecov.io/gh/apache/texera/commit/439ea72e46b78aec7f71e8889225f2c90942a2c2?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [pyamber](https://app.codecov.io/gh/apache/texera/pull/5898/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `90.20% <ø> (ø)` | | Carriedforward from [439ea72](https://app.codecov.io/gh/apache/texera/commit/439ea72e46b78aec7f71e8889225f2c90942a2c2?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [python](https://app.codecov.io/gh/apache/texera/pull/5898/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `90.76% <ø> (ø)` | | Carriedforward from [439ea72](https://app.codecov.io/gh/apache/texera/commit/439ea72e46b78aec7f71e8889225f2c90942a2c2?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [workflow-compiling-service](https://app.codecov.io/gh/apache/texera/pull/5898/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `58.69% <ø> (ø)` | | Carriedforward from [439ea72](https://app.codecov.io/gh/apache/texera/commit/439ea72e46b78aec7f71e8889225f2c90942a2c2?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   
   *This pull request uses carry forward flags. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to find out more.
   </details>
   
   [:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/texera/pull/5898?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] test(workflow-operator): add unit test coverage for Hugging Face operator descriptors [texera]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5898:
URL: https://github.com/apache/texera/pull/5898#issuecomment-4774795071

   <!-- texera-benchmarks-comment -->
   ## ⚠️ Benchmark changes need a look
   
   🟢 0 better · 🔴 3 worse · ⚪ 12 noise (<±5%) · 0 without baseline
   
   > Compared against main `439ea72` benchmarked on **this same runner**, so the delta is largely free of cross-runner hardware noise. The "7d avg" column still reflects the gh-pages dashboard. Treat <±5% as noise unless repeated.
   
   [Dashboard](https://apache.github.io/texera/dev/bench/) · [Run](https://github.com/apache/texera/actions/runs/27995789682)
   
   | | config | throughput | MB/s | latency | max Δ latest / 7d |
   |---|---|---:|---:|---:|---:|
   | 🔴 | bs=10 sw=10 sl=64 | 423 | 0.258 | 22,467/34,452/34,452 us | 🔴 +7.0% / 🟢 -5.5% |
   | ⚪ | bs=100 sw=10 sl=64 | 954 | 0.582 | 104,679/124,582/124,582 us | ⚪ within ±5% / 🟢 -10.9% |
   | ⚪ | bs=1000 sw=10 sl=64 | 1,096 | 0.669 | 916,155/961,662/961,662 us | ⚪ within ±5% / 🟢 -6.0% |
   
   <details><summary>Baseline details</summary>
   
   Latest main `439ea72` from same runner
   
   | config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
   |---|---|---:|---:|---:|---:|---:|
   | bs=10 sw=10 sl=64 | throughput | 423 tuples/sec | 442 tuples/sec | 410.82 tuples/sec | -4.3% | +3.0% |
   | bs=10 sw=10 sl=64 | MB/s | 0.258 MB/s | 0.269 MB/s | 0.251 MB/s | -4.1% | +2.9% |
   | bs=10 sw=10 sl=64 | p50 | 22,467 us | 20,999 us | 23,785 us | +7.0% | -5.5% |
   | bs=10 sw=10 sl=64 | p95 | 34,452 us | 32,221 us | 34,980 us | +6.9% | -1.5% |
   | bs=10 sw=10 sl=64 | p99 | 34,452 us | 32,221 us | 34,980 us | +6.9% | -1.5% |
   | bs=100 sw=10 sl=64 | throughput | 954 tuples/sec | 932 tuples/sec | 891.94 tuples/sec | +2.4% | +7.0% |
   | bs=100 sw=10 sl=64 | MB/s | 0.582 MB/s | 0.569 MB/s | 0.544 MB/s | +2.3% | +6.9% |
   | bs=100 sw=10 sl=64 | p50 | 104,679 us | 107,166 us | 112,277 us | -2.3% | -6.8% |
   | bs=100 sw=10 sl=64 | p95 | 124,582 us | 129,039 us | 139,802 us | -3.5% | -10.9% |
   | bs=100 sw=10 sl=64 | p99 | 124,582 us | 129,039 us | 139,802 us | -3.5% | -10.9% |
   | bs=1000 sw=10 sl=64 | throughput | 1,096 tuples/sec | 1,096 tuples/sec | 1,041 tuples/sec | 0.0% | +5.3% |
   | bs=1000 sw=10 sl=64 | MB/s | 0.669 MB/s | 0.669 MB/s | 0.635 MB/s | 0.0% | +5.3% |
   | bs=1000 sw=10 sl=64 | p50 | 916,155 us | 915,885 us | 972,714 us | +0.0% | -5.8% |
   | bs=1000 sw=10 sl=64 | p95 | 961,662 us | 946,357 us | 1,023,057 us | +1.6% | -6.0% |
   | bs=1000 sw=10 sl=64 | p99 | 961,662 us | 946,357 us | 1,023,057 us | +1.6% | -6.0% |
   
   </details>
   
   <details><summary>Raw CSV</summary>
   
   ```csv
   config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
   0,10,10,64,20,473.06,200,128000,423,0.258,22466.95,34451.73,34451.73
   1,100,10,64,20,2096.91,2000,1280000,954,0.582,104678.57,124582.13,124582.13
   2,1000,10,64,20,18247.73,20000,12800000,1096,0.669,916154.62,961661.55,961661.55
   ```
   
   </details>
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] test(workflow-operator): add unit test coverage for Hugging Face operator descriptors [texera]

Posted by "aglinxinyuan (via GitHub)" <gi...@apache.org>.
aglinxinyuan commented on code in PR #5898:
URL: https://github.com/apache/texera/pull/5898#discussion_r3458069030


##########
common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/huggingFace/HuggingFaceIrisLogisticRegressionOpDescSpec.scala:
##########
@@ -0,0 +1,114 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.texera.amber.operator.huggingFace
+
+import org.apache.texera.amber.core.tuple.{AttributeType, Schema}
+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 HuggingFaceIrisLogisticRegressionOpDescSpec extends AnyFlatSpec with Matchers {
+
+  private def b64(s: String): String =
+    Base64.getEncoder.encodeToString(s.getBytes(StandardCharsets.UTF_8))
+
+  // EncodableString fields are always base64-wrapped in .encode mode
+  // (self.decode_python_template('<base64>')), so assert on the base64 form only rather than
+  // the raw column name, which could appear in the generated Python for unrelated reasons.
+  private def carries(output: String, name: String): Boolean =
+    output.contains(b64(name))
+
+  private def configured(): HuggingFaceIrisLogisticRegressionOpDesc = {
+    val d = new HuggingFaceIrisLogisticRegressionOpDesc
+    d.petalLengthCmAttribute = "petalLength"
+    d.petalWidthCmAttribute = "petalWidth"
+    d.predictionClassName = "species"
+    d.predictionProbabilityName = "probability"
+    d
+  }
+
+  "HuggingFaceIrisLogisticRegressionOpDesc.operatorInfo" should
+    "advertise the name, Hugging Face group, and a 1-in/1-out shape" in {
+    val info = (new HuggingFaceIrisLogisticRegressionOpDesc).operatorInfo
+    info.userFriendlyName shouldBe "Hugging Face Iris Logistic Regression"
+    info.operatorDescription shouldBe
+      "Predict whether an iris is an Iris-setosa using a pre-trained logistic regression model"
+    info.operatorGroupName shouldBe OperatorGroupConstants.HUGGINGFACE_GROUP
+    info.inputPorts should have length 1
+    info.outputPorts should have length 1
+  }
+
+  "HuggingFaceIrisLogisticRegressionOpDesc" should "default all column fields to null" in {
+    val d = new HuggingFaceIrisLogisticRegressionOpDesc
+    d.petalLengthCmAttribute shouldBe null
+    d.petalWidthCmAttribute shouldBe null
+    d.predictionClassName shouldBe null
+    d.predictionProbabilityName shouldBe null
+  }
+
+  "HuggingFaceIrisLogisticRegressionOpDesc.getOutputSchemas" should
+    "reject a blank prediction result name" in {
+    val d = new HuggingFaceIrisLogisticRegressionOpDesc
+    val in = Schema().add("sepal", AttributeType.STRING)
+    val ex = intercept[RuntimeException] {
+      d.getOutputSchemas(Map(d.operatorInfo.inputPorts.head.id -> in))
+    }
+    ex.getMessage shouldBe "Result attribute name should not be empty"
+  }
+
+  it should "append a STRING class column and a DOUBLE probability column, keyed by the output port" in {
+    val d = configured()
+    val in = Schema().add("sepal", AttributeType.STRING)
+    val out = d.getOutputSchemas(Map(d.operatorInfo.inputPorts.head.id -> in))
+    val schema = out(d.operatorInfo.outputPorts.head.id)
+    schema.getAttribute("sepal").getType shouldBe AttributeType.STRING
+    schema.getAttribute("species").getType shouldBe AttributeType.STRING
+    schema.getAttribute("probability").getType shouldBe AttributeType.DOUBLE
+  }
+
+  "HuggingFaceIrisLogisticRegressionOpDesc.generatePythonCode" should
+    "emit the logistic-regression operator carrying the configured columns (encoded)" in {
+    val d = configured()
+    val code = d.generatePythonCode()
+    code should include("class ProcessTupleOperator(UDFOperatorV2)")
+    code should include("LinearModel.from_pretrained")
+    code should include("sadhaklal/logistic-regression-iris")
+    code should include("self.decode_python_template(")
+    carries(code, "petalLength") shouldBe true
+    carries(code, "species") shouldBe true
+  }
+
+  "HuggingFaceIrisLogisticRegressionOpDesc" should
+    "round-trip its config fields through the polymorphic base" in {
+    val d = configured()
+    val restored = objectMapper.readValue(objectMapper.writeValueAsString(d), classOf[LogicalOp])
+    restored shouldBe a[HuggingFaceIrisLogisticRegressionOpDesc]
+    val h = restored.asInstanceOf[HuggingFaceIrisLogisticRegressionOpDesc]
+    h.petalLengthCmAttribute shouldBe "petalLength"
+    h.petalWidthCmAttribute shouldBe "petalWidth"
+    h.predictionClassName shouldBe "species"
+    h.predictionProbabilityName shouldBe "probability"
+  }
+}

Review Comment:
   Good call — added the `getPhysicalOp` test to the Iris spec, mirroring the siblings: asserts `OpExecWithCode(_, "python")` plus input/output port-identity carry-forward. Spec is now at parity (7 tests).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] test(workflow-operator): add unit test coverage for Hugging Face operator descriptors [texera]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5898:
URL: https://github.com/apache/texera/pull/5898#issuecomment-4774762467

   <!-- texera-reviewer-suggestion -->
   ### Automated Reviewer Suggestions
   
   Based on the `git blame` history of the changed files, we recommend the following reviewers:
   
   - No candidates found from `git blame` history.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] test(workflow-operator): add unit test coverage for Hugging Face operator descriptors [texera]

Posted by "aglinxinyuan (via GitHub)" <gi...@apache.org>.
aglinxinyuan commented on code in PR #5898:
URL: https://github.com/apache/texera/pull/5898#discussion_r3456985545


##########
common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/huggingFace/HuggingFaceSentimentAnalysisOpDescSpec.scala:
##########
@@ -0,0 +1,130 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.texera.amber.operator.huggingFace
+
+import org.apache.texera.amber.core.executor.OpExecWithCode
+import org.apache.texera.amber.core.tuple.{AttributeType, Schema}
+import org.apache.texera.amber.core.virtualidentity.{ExecutionIdentity, WorkflowIdentity}
+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 HuggingFaceSentimentAnalysisOpDescSpec extends AnyFlatSpec with Matchers {
+
+  private val workflowId = WorkflowIdentity(1L)
+  private val executionId = ExecutionIdentity(1L)
+
+  private def b64(s: String): String =
+    Base64.getEncoder.encodeToString(s.getBytes(StandardCharsets.UTF_8))
+
+  // EncodableString fields render as self.decode_python_template('<base64>') in encode mode.
+  private def carries(output: String, name: String): Boolean =
+    output.contains(name) || output.contains(b64(name))
+

Review Comment:
   Good catch — `carries` now asserts on the base64 form only, so the raw "positive"/"neutral"/"negative" label keys can no longer mask a missing splice.



##########
common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/huggingFace/HuggingFaceSpamSMSDetectionOpDescSpec.scala:
##########
@@ -0,0 +1,117 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.texera.amber.operator.huggingFace
+
+import org.apache.texera.amber.core.executor.OpExecWithCode
+import org.apache.texera.amber.core.tuple.{AttributeType, Schema}
+import org.apache.texera.amber.core.virtualidentity.{ExecutionIdentity, WorkflowIdentity}
+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 HuggingFaceSpamSMSDetectionOpDescSpec extends AnyFlatSpec with Matchers {
+
+  private val workflowId = WorkflowIdentity(1L)
+  private val executionId = ExecutionIdentity(1L)
+
+  private def b64(s: String): String =
+    Base64.getEncoder.encodeToString(s.getBytes(StandardCharsets.UTF_8))
+
+  private def carries(output: String, name: String): Boolean =
+    output.contains(name) || output.contains(b64(name))
+

Review Comment:
   Done — `carries` is base64-only now, so "text" in "text-classification" and "score" in result["score"] can't cause a false positive.



##########
common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/huggingFace/HuggingFaceTextSummarizationOpDescSpec.scala:
##########
@@ -0,0 +1,121 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.texera.amber.operator.huggingFace
+
+import org.apache.texera.amber.core.executor.OpExecWithCode
+import org.apache.texera.amber.core.tuple.{AttributeType, Schema}
+import org.apache.texera.amber.core.virtualidentity.{ExecutionIdentity, WorkflowIdentity}
+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 HuggingFaceTextSummarizationOpDescSpec extends AnyFlatSpec with Matchers {
+
+  private val workflowId = WorkflowIdentity(1L)
+  private val executionId = ExecutionIdentity(1L)
+
+  private def b64(s: String): String =
+    Base64.getEncoder.encodeToString(s.getBytes(StandardCharsets.UTF_8))
+
+  private def carries(output: String, name: String): Boolean =
+    output.contains(name) || output.contains(b64(name))
+

Review Comment:
   Done — `carries` asserts the base64 form only, so the `text`/`summary` local variable names no longer mask a missing splice. Applied the same tightening to the Iris spec for consistency.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] test(workflow-operator): add unit test coverage for Hugging Face operator descriptors [texera]

Posted by "mengw15 (via GitHub)" <gi...@apache.org>.
mengw15 commented on code in PR #5898:
URL: https://github.com/apache/texera/pull/5898#discussion_r3457956314


##########
common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/huggingFace/HuggingFaceIrisLogisticRegressionOpDescSpec.scala:
##########
@@ -0,0 +1,114 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.texera.amber.operator.huggingFace
+
+import org.apache.texera.amber.core.tuple.{AttributeType, Schema}
+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 HuggingFaceIrisLogisticRegressionOpDescSpec extends AnyFlatSpec with Matchers {
+
+  private def b64(s: String): String =
+    Base64.getEncoder.encodeToString(s.getBytes(StandardCharsets.UTF_8))
+
+  // EncodableString fields are always base64-wrapped in .encode mode
+  // (self.decode_python_template('<base64>')), so assert on the base64 form only rather than
+  // the raw column name, which could appear in the generated Python for unrelated reasons.
+  private def carries(output: String, name: String): Boolean =
+    output.contains(b64(name))
+
+  private def configured(): HuggingFaceIrisLogisticRegressionOpDesc = {
+    val d = new HuggingFaceIrisLogisticRegressionOpDesc
+    d.petalLengthCmAttribute = "petalLength"
+    d.petalWidthCmAttribute = "petalWidth"
+    d.predictionClassName = "species"
+    d.predictionProbabilityName = "probability"
+    d
+  }
+
+  "HuggingFaceIrisLogisticRegressionOpDesc.operatorInfo" should
+    "advertise the name, Hugging Face group, and a 1-in/1-out shape" in {
+    val info = (new HuggingFaceIrisLogisticRegressionOpDesc).operatorInfo
+    info.userFriendlyName shouldBe "Hugging Face Iris Logistic Regression"
+    info.operatorDescription shouldBe
+      "Predict whether an iris is an Iris-setosa using a pre-trained logistic regression model"
+    info.operatorGroupName shouldBe OperatorGroupConstants.HUGGINGFACE_GROUP
+    info.inputPorts should have length 1
+    info.outputPorts should have length 1
+  }
+
+  "HuggingFaceIrisLogisticRegressionOpDesc" should "default all column fields to null" in {
+    val d = new HuggingFaceIrisLogisticRegressionOpDesc
+    d.petalLengthCmAttribute shouldBe null
+    d.petalWidthCmAttribute shouldBe null
+    d.predictionClassName shouldBe null
+    d.predictionProbabilityName shouldBe null
+  }
+
+  "HuggingFaceIrisLogisticRegressionOpDesc.getOutputSchemas" should
+    "reject a blank prediction result name" in {
+    val d = new HuggingFaceIrisLogisticRegressionOpDesc
+    val in = Schema().add("sepal", AttributeType.STRING)
+    val ex = intercept[RuntimeException] {
+      d.getOutputSchemas(Map(d.operatorInfo.inputPorts.head.id -> in))
+    }
+    ex.getMessage shouldBe "Result attribute name should not be empty"
+  }
+
+  it should "append a STRING class column and a DOUBLE probability column, keyed by the output port" in {
+    val d = configured()
+    val in = Schema().add("sepal", AttributeType.STRING)
+    val out = d.getOutputSchemas(Map(d.operatorInfo.inputPorts.head.id -> in))
+    val schema = out(d.operatorInfo.outputPorts.head.id)
+    schema.getAttribute("sepal").getType shouldBe AttributeType.STRING
+    schema.getAttribute("species").getType shouldBe AttributeType.STRING
+    schema.getAttribute("probability").getType shouldBe AttributeType.DOUBLE
+  }
+
+  "HuggingFaceIrisLogisticRegressionOpDesc.generatePythonCode" should
+    "emit the logistic-regression operator carrying the configured columns (encoded)" in {
+    val d = configured()
+    val code = d.generatePythonCode()
+    code should include("class ProcessTupleOperator(UDFOperatorV2)")
+    code should include("LinearModel.from_pretrained")
+    code should include("sadhaklal/logistic-regression-iris")
+    code should include("self.decode_python_template(")
+    carries(code, "petalLength") shouldBe true
+    carries(code, "species") shouldBe true
+  }
+
+  "HuggingFaceIrisLogisticRegressionOpDesc" should
+    "round-trip its config fields through the polymorphic base" in {
+    val d = configured()
+    val restored = objectMapper.readValue(objectMapper.writeValueAsString(d), classOf[LogicalOp])
+    restored shouldBe a[HuggingFaceIrisLogisticRegressionOpDesc]
+    val h = restored.asInstanceOf[HuggingFaceIrisLogisticRegressionOpDesc]
+    h.petalLengthCmAttribute shouldBe "petalLength"
+    h.petalWidthCmAttribute shouldBe "petalWidth"
+    h.predictionClassName shouldBe "species"
+    h.predictionProbabilityName shouldBe "probability"
+  }
+}

Review Comment:
   Within-PR consistency: the other three Hugging Face specs in this PR each include a `getPhysicalOp` test that asserts `OpExecWithCode(..., "python")` plus port-identity carry-forward (`physical.inputPorts.keySet shouldBe d.operatorInfo.inputPorts.map(_.id).toSet`). The Iris spec doesn't — it never imports `OpExecWithCode` / `WorkflowIdentity` / `ExecutionIdentity` and skips the test entirely.
   
   Iris extends the same `PythonOperatorDescriptor` base as its siblings, so `getPhysicalOp` is on the same contract surface. Mirroring the test (≈10 lines) would bring this spec up to parity with the others.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] test(workflow-operator): add unit test coverage for Hugging Face operator descriptors [texera]

Posted by "aglinxinyuan (via GitHub)" <gi...@apache.org>.
aglinxinyuan merged PR #5898:
URL: https://github.com/apache/texera/pull/5898


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org