Skip to content

Conversation

@glopesdev
Copy link
Contributor

@glopesdev glopesdev commented Oct 22, 2024

NumPy 2.0 disallows silent overflow when converting to unsigned dtypes which was causing parsing to fail when reading timestamped registers.

This PR introduces an early explicit conversion to prevent overflow and ensure full compatibility with NumPy 1.26+ and NumPy 2.0+.

Fixes #32

@glopesdev glopesdev added the fix Pull request that fixes an issue label Oct 22, 2024
Copy link
Member

@bruno-f-cruz bruno-f-cruz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line 83 of io module should be changed to:

stride = np.int32(data[1] + 2)

The current state of the library is passing tests because all tests don't account for files where the number of rows is larger than 255. The behavior of the numpy library can be reproduce using:

import numpy as np

255 // np.uint8(2)
# 127
257 // np.uint8(2)
# throws OverflowError: out of bounds for uint8

Not sure if this warrants its own unittest since i doubt it will ever happen again once the cast is applied, but I defer that decision to you.

@glopesdev
Copy link
Contributor Author

glopesdev commented Oct 29, 2024

@bruno-f-cruz Good catch, I have used the default int type rather than specifically int32 but it shouldn't really matter.

Adding regression testing for this is important. However, since that in itself required quite the significant refactoring to get right (with new features!) I will add it in the next PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Pull request that fixes an issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Numpy 2.0 breaks read function

3 participants