Use new GraphInitializers tensor APIs to simplify codebase - #423
justinchuby wants to merge 1 commit into
Conversation
Co-authored-by: justinchuby <11205048+justinchuby@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #423 +/- ##
=======================================
Coverage 84.57% 84.57%
=======================================
Files 52 52
Lines 6501 6501
Branches 1329 1329
=======================================
Hits 5498 5498
Misses 643 643
Partials 360 360 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Pull request overview
Updates the test suite to use the newer GraphInitializers tensor-centric APIs (get_tensor(), tensors(), tensor_items()) instead of repeatedly dereferencing initializer Value.const_value, improving readability and aligning tests with the current public API surface.
Changes:
- Replaced
initializers["name"].const_valuewithinitializers.get_tensor("name")in multiple tests. - Replaced loops over
.values()/.items()+.const_valuewith.tensors()/.tensor_items()where appropriate. - Kept behavior-sensitive cases using
get_tensor()(which returnsdefaultwhenconst_valueisNone) to preserve existing test intent.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/onnx_ir/serde_test.py | Uses initializers.get_tensor() for initializer tensor assertions in serde tests. |
| src/onnx_ir/passes/common/shape_inference_test.py | Updates initializer access patterns and preserves None-const-value coverage via get_tensor(). |
| src/onnx_ir/passes/common/initializer_deduplication_test.py | Switches initializer tensor reads to get_tensor() in deduplication tests. |
| src/onnx_ir/passes/common/constant_manipulation_test.py | Updates lifted-initializer verification to compare directly against get_tensor() results. |
| src/onnx_ir/external_data_test.py | Converts external-data tests to use get_tensor() for initializer tensor access. |
| src/onnx_ir/_safetensors/_safetensors_test.py | Replaces initializer iteration/access with tensors() and tensor_items() for safetensors tests. |
| src/onnx_ir/_io_test.py | Uses get_tensor() for initializer tensor assertions in IO tests. |
|
These are all tests. So I don't know how useful the API is, especially when the setter is not mirroring it. @gramalingam do you have thoughts? |
Accesses to initializer tensors across the test suite still used the old three-layer pattern (
initializers["name"].const_value,for value in .values(): value.const_value) introduced before theget_tensor()/tensors()/tensor_items()APIs existed.Changes
initializers["name"].const_value→initializers.get_tensor("name")— applied acrossexternal_data_test.py,serde_test.py,_safetensors_test.py,_io_test.py,shape_inference_test.py,initializer_deduplication_test.py,constant_manipulation_test.pyfor value in .values(): value.const_value→for tensor in .tensors()— where only the tensor is neededfor name, value in .items(): value.const_value→for name, tensor in .tensor_items()— where name + tensor are both neededProduction code is left unchanged: those loops either require the
Valueobject for operations unrelated to the tensor (naming,replace_all_uses_with, etc.) or deliberately skipNoneconst_values — behaviour that differs fromtensors()'s strict-raise semantics.