-
Notifications
You must be signed in to change notification settings - Fork 72
[IR] Fix sequence handling in tensor function #2252
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
Fix bug in `tensor()` function to handle empty sequences and require `dtype` when value is an empty sequence. * Add a check to ensure the sequence is non-empty before performing type checks in the `tensor()` function in `onnxscript/ir/_convenience/_constructors.py`. * Raise a `ValueError` if `dtype` is `None` and `value` is an empty sequence in the `tensor()` function. * Update the `tensor()` function to handle the case when a sequence is empty explicitly. * Add a test case to check if `tensor()` raises a `ValueError` when `dtype` is `None` and `value` is an empty sequence in `onnxscript/ir/_convenience/_constructors_test.py`. * Add a test case to check if `tensor()` handles the case when a sequence is empty explicitly. --- For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/microsoft/onnxscript?shareId=XXXX-XXXX-XXXX-XXXX).
❌ 3 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
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 fixes a bug in the tensor() function to explicitly handle empty sequences and enforce that a dtype is provided when necessary.
- Updated tensor() logic in _constructors.py to check for empty sequences and raise a ValueError when dtype is None.
- Added corresponding test cases in _constructors_test.py to verify the new behavior.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
File | Description |
---|---|
onnxscript/ir/_convenience/_constructors.py | Adjusted sequence handling logic for empty and non-empty sequences. |
onnxscript/ir/_constructors_test.py | Added tests covering the error and successful empty sequence cases. |
Comments suppressed due to low confidence (1)
onnxscript/ir/_constructience/_constructors.py:105
- The int sequence check incorrectly uses 'not isinstance(value, bool)' instead of checking the element. It should be 'not isinstance(elem, bool)' to properly filter out booleans within the sequence.
if all((isinstance(elem, int) and not isinstance(value, bool)) for elem in value):
(Copilot) Fix bug in
tensor()
function to handle empty sequences and requiredtype
when value is an empty sequence.tensor()
function inonnxscript/ir/_convenience/_constructors.py
.ValueError
ifdtype
isNone
andvalue
is an empty sequence in thetensor()
function.tensor()
function to handle the case when a sequence is empty explicitly.tensor()
raises aValueError
whendtype
isNone
andvalue
is an empty sequence inonnxscript/ir/_convenience/_constructors_test.py
.tensor()
handles the case when a sequence is empty explicitly.For more details, open the Copilot Workspace session.