-
Notifications
You must be signed in to change notification settings - Fork 11
Create a convenience constructor for Value #179
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a convenience constructor function for creating Value
objects with a more user-friendly interface. The constructor allows users to specify values through multiple parameter combinations and includes validation logic.
Key changes:
- New
value()
convenience constructor function with flexible parameter handling - Updated import to include the new
value
function in the public API - Type annotation improvement for the existing
tensor()
function
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
src/onnx_ir/_convenience/_constructors.py | Adds new value() function and fixes type annotation for tensor() |
src/onnx_ir/init.py | Exports the new value function in the public API |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #179 +/- ##
==========================================
+ Coverage 76.51% 76.62% +0.11%
==========================================
Files 40 40
Lines 4879 4894 +15
Branches 971 977 +6
==========================================
+ Hits 3733 3750 +17
+ Misses 857 854 -3
- Partials 289 290 +1 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought ir.Value was easy enough? OR it's for consistency?
The main thing was it required that the shapes to be an ir Shape object,
and the type to the an ir TypeProtocol (not dtype), which may not be very
straightforward for simple usages.
This convenience function allows a sequence as shape (np like), as well as
dtype to specify value type.
Le lun. 8 sept. 2025 à 09:14, Ti-Tai Wang ***@***.***> a
écrit :
… ***@***.**** approved this pull request.
I thought ir.Value was easy enough? OR it's for consistency?
—
Reply to this email directly, view it on GitHub
<#179 (review)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACVPTOAZRFPLNTLLKOYPE7D3RWTNJAVCNFSM6AAAAACFZSU43GVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTCOJXGIZDQMRXHE>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
A convenience constructor for Value that supports an easier way of supplying arguments.