Implement schema-based input/attribute partitioning in GraphBuilder by gramalingam · Pull Request #2837 · microsoft/onnxscript

gramalingam · 2026-03-03T21:55:17Z

Convert onnx.defs.OpSchema to ir.schemas.OpSignature via from_op_schema and delegate to separate_input_attributes_from_arguments
Add allow_extra_args parameter to separate_input_attributes_from_arguments for rejecting unexpected positional arguments (default True for compat)
Builder uses strict mode: allow_extra_kwargs=False, allow_extra_args=False
Refactor _build test helper: accept TypeSpec, optional trace_function, return ir.Graph directly
Add comprehensive tests for input/attribute partitioning

- Convert onnx.defs.OpSchema to ir.schemas.OpSignature via from_op_schema and delegate to separate_input_attributes_from_arguments - Add allow_extra_args parameter to separate_input_attributes_from_arguments for rejecting unexpected positional arguments (default True for compat) - Builder uses strict mode: allow_extra_kwargs=False, allow_extra_args=False - Refactor _build test helper: accept TypeSpec, optional trace_function, return ir.Graph directly - Add comprehensive tests for input/attribute partitioning Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

onnxscript/_internal/builder_test.py

    )

-    onnx_model = ir.Model(graph=graph, ir_version=10)
+    resolved_inputs = [builder._resolve_type_spec(t) for t in input_types]


onnxscript/_internal/builder_test.py

-        output.type = output_type.type  # TODO: need merge_type method in ir.Value
-        output.merge_shapes(output_type.shape)
+        if output_types is not None:
+            resolved_outputs = [builder._resolve_type_spec(t) for t in output_types]


onnxscript/_internal/builder_test.py

+            trace_function=_dummy,
+        )
+        x, y = graph.inputs
+        node = list(graph)[0]


onnxscript/_internal/builder_test.py

+            trace_function=_add,
+        )
+        x, y = graph.inputs
+        node = list(graph)[0]


onnxscript/_internal/builder_test.py

+            trace_function=_gemm,
+        )
+        a, b, c = graph.inputs
+        node = list(graph)[0]


onnxscript/_internal/builder_test.py

+            trace_function=_gemm_no_c,
+        )
+        a, b = graph.inputs
+        node = list(graph)[0]


onnxscript/_internal/builder_test.py

+            input_types=[FLOAT[3, 4], FLOAT[4, 5]],
+            trace_function=_gemm_no_attrs,
+        )
+        node = list(graph)[0]


onnxscript/_internal/builder_test.py

+            trace_function=_concat,
+        )
+        x, y, z = graph.inputs
+        node = list(graph)[0]


onnxscript/_internal/builder_test.py

+        )
+        data, starts, ends, axes, steps = graph.inputs
+
+        slice_node = list(graph)[0]


codecov · 2026-03-03T22:06:21Z

Codecov Report

❌ Patch coverage is 97.05882% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.87%. Comparing base (555f81e) to head (acc5090).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
onnxscript/_internal/builder_test.py	96.77%	1 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2837      +/-   ##
==========================================
+ Coverage   71.79%   71.87%   +0.07%     
==========================================
  Files         239      239              
  Lines       29019    29092      +73     
  Branches     2864     2867       +3     
==========================================
+ Hits        20834    20909      +75     
+ Misses       7213     7210       -3     
- Partials      972      973       +1

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

justinchuby · 2026-03-03T23:07:23Z

onnxscript/_internal/builder.py

-        # Not implemented yet
-        del schema
-        return inputs, kwargs
+        if schema is None:


The Op object has the signature cached. Would it be possible to use that?

Copilot

Pull request overview

This PR activates schema-based input/attribute partitioning in GraphBuilder, which was previously stubbed out. It also adds an allow_extra_args parameter to separate_input_attributes_from_arguments for strict positional-argument validation, and refactors the _build test helper to return ir.Graph directly (removing the intermediate ir.Model wrapper).

Changes:

Added allow_extra_args parameter to separate_input_attributes_from_arguments in param_manipulation.py, enforcing a check for extra positional arguments when a schema is known and no variadic parameter exists.
Implemented GraphBuilder._partition_inputs_attributes in builder.py to delegate to separate_input_attributes_from_arguments via ir.schemas.OpSignature.from_op_schema, using strict mode (fill_defaults=False, allow_extra_args=False).
Refactored the _build test helper in builder_test.py to accept TypeSpec inputs, make trace_function optional, return ir.Graph directly, and added a comprehensive PartitionInputsAttributesTest suite.

Reviewed changes

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

File	Description
`onnxscript/_internal/param_manipulation.py`	Adds `allow_extra_args` parameter and validates extra positional args after the main loop
`onnxscript/_internal/builder.py`	Implements `_partition_inputs_attributes` using schema-based partitioning instead of a no-op stub
`onnxscript/_internal/builder_test.py`	Refactors `_build` helper and adds `PartitionInputsAttributesTest` covering 9 new test cases

github-project-automation bot added this to ONNX Script Review Board Mar 3, 2026

github-project-automation bot moved this to Todo in ONNX Script Review Board Mar 3, 2026

github-advanced-security bot found potential problems Mar 3, 2026

View reviewed changes

justinchuby approved these changes Mar 3, 2026

View reviewed changes

github-project-automation bot moved this from Todo to Done in ONNX Script Review Board Mar 3, 2026

justinchuby requested a review from Copilot March 3, 2026 23:05

Copilot started reviewing on behalf of justinchuby March 3, 2026 23:06 View session

justinchuby reviewed Mar 3, 2026

View reviewed changes

Copilot AI reviewed Mar 3, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement schema-based input/attribute partitioning in GraphBuilder#2837

Implement schema-based input/attribute partitioning in GraphBuilder#2837
gramalingam wants to merge 1 commit intomainfrom
rama/params

gramalingam commented Mar 3, 2026

Uh oh!

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

codecov bot commented Mar 3, 2026 •

edited

Loading

Uh oh!

justinchuby Mar 3, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

gramalingam commented Mar 3, 2026

Uh oh!

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

codecov bot commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

justinchuby Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Mar 3, 2026 •

edited

Loading