Skip to content

Support None as op input in GraphBuilder#2868

Merged
justinchuby merged 3 commits intomainfrom
justinchu/builder-none
Mar 27, 2026
Merged

Support None as op input in GraphBuilder#2868
justinchuby merged 3 commits intomainfrom
justinchu/builder-none

Conversation

@justinchuby
Copy link
Copy Markdown
Collaborator

@justinchuby justinchuby commented Mar 27, 2026

Summary

Support `None` as a valid input to `op.OpType()` calls in the onnxscript builder. This is needed for ONNX ops with optional inputs (e.g., Gemm's optional C input).

Changes

  • Add `None` to the `VALUE_LIKE` union type
  • Update `_input_to_ir_value` to return `ir.Value | None` and pass `None` through
  • Update `_partition_inputs_attributes` and `call_op` type signatures to accept `None`
  • Add tests for `None` inputs with standard and custom domain ops

Also removed pylint because it is not doing much good nowadays.

Justin Chu added 2 commits March 26, 2026 17:40
Signed-off-by: Justin Chu <justinchuby@noreply.github.com>
Signed-off-by: Justin Chu <justinchuby@noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds support for passing None as an op input in onnxscript’s imperative GraphBuilder, enabling correct construction of nodes with optional inputs (e.g., Gemm’s optional C input).

Changes:

  • Extends VALUE_LIKE and input conversion to allow None to pass through as an omitted/optional input.
  • Updates builder call paths (_partition_inputs_attributes, call_op) to accept None in input sequences.
  • Adds unit tests validating None behavior for both standard-domain and custom-domain ops.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
onnxscript/_internal/builder.py Extends accepted input types and conversion/call signatures to preserve None inputs.
onnxscript/_internal/builder_test.py Adds tests asserting None is preserved in node input slots for optional inputs.
Comments suppressed due to low confidence (1)

onnxscript/_internal/builder.py:366

  • _partition_inputs_attributes can return inputs containing None (e.g., when schema is None it returns inputs unchanged). The return type annotation currently excludes None, which makes the typing inconsistent with the new optional-input behavior and with _cast_inputs (which returns Sequence[ir.Value | None]). Please update the return type (and ideally the accepted inputs type) to include None/VALUE_LIKE so the signatures match actual runtime behavior.
        inputs: Sequence[ir.Value | ir.TensorProtocol | None],
        kwargs: dict[str, Any],
    ) -> tuple[Sequence[ir.Value | ir.TensorProtocol], dict[str, Any]]:
        if schema is None:
            return inputs, kwargs

Comment thread onnxscript/_internal/builder.py
Comment thread onnxscript/_internal/builder.py
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.90%. Comparing base (847801c) to head (ca315b5).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2868      +/-   ##
==========================================
+ Coverage   71.87%   71.90%   +0.02%     
==========================================
  Files         239      239              
  Lines       29150    29175      +25     
  Branches     2875     2876       +1     
==========================================
+ Hits        20953    20978      +25     
  Misses       7219     7219              
  Partials      978      978              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Justin Chu <justinchuby@noreply.github.com>
@justinchuby justinchuby enabled auto-merge (squash) March 27, 2026 01:05
@justinchuby justinchuby merged commit 0cced88 into main Mar 27, 2026
30 of 33 checks passed
@justinchuby justinchuby deleted the justinchu/builder-none branch March 27, 2026 01:25
justinchuby added a commit that referenced this pull request Apr 17, 2026
Support \`None\` as a valid input to \`op.OpType()\` calls in the
onnxscript builder. This is needed for ONNX ops with optional inputs
(e.g., Gemm's optional C input).

- Add \`None\` to the \`VALUE_LIKE\` union type
- Update \`_input_to_ir_value\` to return \`ir.Value | None\` and pass
\`None\` through
- Update \`_partition_inputs_attributes\` and \`call_op\` type
signatures to accept \`None\`
- Add tests for \`None\` inputs with standard and custom domain ops

Also removed pylint because it is not doing much good nowadays.

---------

Signed-off-by: Justin Chu <justinchuby@noreply.github.com>
Co-authored-by: Justin Chu <justinchuby@noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Development

Successfully merging this pull request may close these issues.

3 participants