Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #45 +/- ##
==========================================
+ Coverage 68.60% 70.85% +2.25%
==========================================
Files 6 6
Lines 379 374 -5
Branches 64 57 -7
==========================================
+ Hits 260 265 +5
+ Misses 96 92 -4
+ Partials 23 17 -6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This pull request fixes bugs related to handling sub-byte data types in the onnx-safetensors library. The changes improve support for 2-bit, 4-bit, and other specialized numeric types.
Changes:
- Added support for new data types (INT2, UINT2, FLOAT4E2M1, FLOAT8E8M0, COMPLEX64) with proper safetensors mappings
- Simplified sub-byte type handling by using
tensor.nbytesandtensor.dtype.bitwidthinstead of custom helper functions - Fixed struct unpacking format for reading safetensors headers from "i" (4-byte) to "<Q" (8-byte unsigned long long)
- Refactored
save_fileto eliminate code duplication and improve maintainability - Updated
save_modelto restore original tensor values after saving, avoiding side effects on the input model - Removed unused
mathandtqdmimports
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| src/onnx_safetensors/_safetensors_io.py | Core changes for sub-byte type handling, dtype mappings, header reading fix, and code refactoring |
| src/onnx_safetensors/init.py | Version change from 1.5.0 to 1.4.1 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| values_to_save: list[ir.Value] = [] | ||
| for value in initialized_values: | ||
| tensor = value.const_value | ||
| assert tensor is not None | ||
| if tensor.nbytes < size_threshold_bytes: | ||
| continue | ||
| tensors_to_save.append(tensor) | ||
| values_to_save.append(value) | ||
|
|
There was a problem hiding this comment.
The variable values_to_save is populated but never used in the function. It should either be used for its intended purpose or removed to avoid confusion.
| values_to_save: list[ir.Value] = [] | |
| for value in initialized_values: | |
| tensor = value.const_value | |
| assert tensor is not None | |
| if tensor.nbytes < size_threshold_bytes: | |
| continue | |
| tensors_to_save.append(tensor) | |
| values_to_save.append(value) | |
| for value in initialized_values: | |
| tensor = value.const_value | |
| assert tensor is not None | |
| if tensor.nbytes < size_threshold_bytes: | |
| continue | |
| tensors_to_save.append(tensor) |
| ) | ||
|
|
||
| __version__ = "1.5.0" | ||
| __version__ = "1.4.1" |
There was a problem hiding this comment.
The version is being changed from 1.5.0 to 1.4.1, which is a downgrade. Typically, bug fixes should increment the patch version (e.g., 1.5.0 to 1.5.1) rather than downgrade to a previous version. This could cause confusion with package versioning and updates.
| __version__ = "1.4.1" | |
| __version__ = "1.5.1" |
Support 2bit types.