Update from_onnx_text method

[justinchuby]

• Change the initializers parameter to take a list of tensors.
• Add the missing tests and expose the to_onnx_text method in the root namespace.
• Update create_value_mapping to include values from subgraphs.

[codecov]

Codecov Report

Attention: Patch coverage is 61.90476% with 8 lines in your changes missing coverage. Please review.

Project coverage is 73.55%. Comparing base (24096c6) to head (7604ede).
Report is 4 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
src/onnx_ir/_convenience/__init__.py	57.14%	3 Missing and 3 partials ⚠️
src/onnx_ir/serde.py	66.66%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #80      +/-   ##
==========================================
+ Coverage   73.48%   73.55%   +0.06%     
==========================================
  Files          37       37              
  Lines        4507     4522      +15     
  Branches      906      913       +7     
==========================================
+ Hits         3312     3326      +14     
+ Misses        865      863       -2     
- Partials      330      333       +3     

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

[Copilot]

Pull Request Overview

This PR enhances ONNX text serialization by supporting a list of initializer tensors, exposes the to_onnx_text method in the root namespace, adds tests for including/excluding initializers, and improves value mapping to traverse subgraphs.

• Change from_onnx_text to accept an iterable of initializer tensors and register them
• Expose to_onnx_text at package root and add tests for both inclusion and exclusion of initializers
• Update create_value_mapping to include values from subgraphs and initializers

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
src/onnx_ir/serde_test.py	Update expected IR version and add tests covering initializers
src/onnx_ir/serde.py	Rename with_initializers to initializers and adjust logic
src/onnx_ir/_convenience/init.py	Extend create_value_mapping to traverse subgraphs
src/onnx_ir/init.py	Add to_onnx_text to exported symbols

Comments suppressed due to low confidence (3)

src/onnx_ir/serde.py:209

• Update the Raises section to reference initializers instead of the outdated with_initializers parameter name.

ValueError: If a name in `with_initializers` does not exist in the model.

src/onnx_ir/_convenience/init.py:316

• The docstring should mention that initializers (graph.initializers) are also included in the mapping now.

The mapping includes values from subgraphs. Duplicated names are omitted,

src/onnx_ir/_convenience/init.py:329

• [nitpick] Avoid shadowing the built-in input function; consider renaming this loop variable to e.g. graph_input or inp.

for input in graph.inputs:

[titaiwangms]

What is the design logic of choosing between list and dict in initializers:

ir-py/src/onnx_ir/_protocols.py

Line 301 in ccdb051

initializers: MutableMapping[str, ValueProtocol]

(The Graph is using dict still.)

[justinchuby]

There is a bit of ambiguity with the naming of "initializers". Here these are more like "tensors that will initialize values" with matching names that we can use to find the set the const_value of corresponding values. In graph the initializers provided in __init__ is actually "initialized_values". I wasn't clear about this when designing the graph object initially. But then if we name the attribute in graph "initialized_values", we will also introduce a new concept that people have not seen. So the name "initializers" may be acceptable.

Note that in graph we initialize with a list of values but when users access .initializers they get a dictionary like you mention. This was done initially to keep the data structure simple and allow look up by names. In hindsight, it may be better to keep the field as a list for consistency, but create additional methods like add_initializer and remove_initializer which operate on names. This wasn't done because once we set the api, updating it will be a breaking change.

[justinchuby]

There is a bit of ambiguity with the naming of "initializers"

Do you have suggestions around naming?