Skip to content
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

Prevent potential data loss in numpy's dtype::get_itemsize(). #463

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

JohannesWilde
Copy link

The numpy 2.0 migration guide states [https://numpy.org/devdocs/numpy_2_0_migration_guide.html]:

"PyDataType_ELSIZE and PyDataType_SET_ELSIZE (note that the result is now npy_intp and not int)."

Even though it is rather unlikely to have have elements larger than 2GB each, prevent potential numerical overflows, which could occur when casting ssize_t to int on 64-bit systems.

The numpy 2.0 migration guide states [https://numpy.org/devdocs/numpy_2_0_migration_guide.html]:

"PyDataType_ELSIZE and PyDataType_SET_ELSIZE (note that the result is now npy_intp and not int)."

Even though it is rather unlikely to have have elements larger than 2GB each, prevent potential numerical overflows, which could occur when casting ssize_t to int on 64-bit systems.
@JohannesWilde JohannesWilde changed the title Prevent potential data loss in numpy's dtype::get_itemsize(). WIP: Prevent potential data loss in numpy's dtype::get_itemsize(). Dec 14, 2024
@JohannesWilde JohannesWilde changed the title WIP: Prevent potential data loss in numpy's dtype::get_itemsize(). Prevent potential data loss in numpy's dtype::get_itemsize(). Dec 14, 2024
@JohannesWilde
Copy link
Author

JohannesWilde commented Dec 14, 2024

As before, there is no check for numerical overflows in any of the multplications in is_c_contiguous and is_f_contiguous when doing total *= (*i);. Should someone change this?

@JohannesWilde
Copy link
Author

The file ndarray.cpp still contains several tab characters. Do you have coding rules relating to tabs?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant