-
Notifications
You must be signed in to change notification settings - Fork 72
[IR] Handle ONNX custom types in DataType.from_numpy #2131
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
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 support for handling ONNX custom types in DataType.from_numpy to resolve confusion with custom dtypes. It also expands test coverage by introducing parameterized tests for both standard and custom numpy dtypes.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
File | Description |
---|---|
onnxscript/ir/_enums.py | Adds special case handling for custom ONNX dtypes based on dtype.names |
onnxscript/ir/_enums_test.py | Introduces parameterized tests to cover new custom and standard types |
❌ 7 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 updates the DataType.from_numpy function to correctly handle ONNX custom types by mapping custom numpy dtypes to the corresponding ONNX data types. Key changes include:
- Adding support for special cases (custom dtypes) based on the dtype’s "names" attribute.
- Updating the tests to use parameterized cases including both standard numpy types and ONNX custom types.
- Adding necessary imports (ml_dtypes, onnx._custom_element_types, parameterized) to support the new test cases.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
File | Description |
---|---|
onnxscript/ir/_enums.py | Updated from_numpy to support custom ONNX dtypes using dtype.names |
onnxscript/ir/_enums_test.py | Refactored tests to use parameterized inputs for broader coverage |
Comments suppressed due to low confidence (1)
onnxscript/ir/_enums_test.py:109
- [nitpick] Consider renaming the '_' parameter to a more descriptive name such as 'test_name' to improve readability in the test signature.
def test_from_numpy_takes_np_dtype_and_returns_data_type(self, _: str, np_dtype: np.dtype, onnx_type: _enums.DataType):
Fixes microsoft#1893 where the IR was confused about ONNX custom types. In the long run we should update onnx to use ml_dtypes.
Fixes microsoft#1893 where the IR was confused about ONNX custom types. In the long run we should update onnx to use ml_dtypes.
Fixes microsoft#1893 where the IR was confused about ONNX custom types. In the long run we should update onnx to use ml_dtypes.
Fixes #1893 where the IR was confused about ONNX custom types. In the long run we should update onnx to use ml_dtypes.