Skip to content

Conversation

@effigies
Copy link
Member

@effigies effigies commented Nov 2, 2023

We had a pair of functions as_int() and int_to_float() that handled bugs in numpy that existed at least as recently as 1.11, but which no longer exist in 1.20, our current minimum.

This PR replaces as_int(...) with int(...) and int_to_float(..., dtype) with dtype(...) and deprecates these functions. Regression tests to ensure that these native Python/numpy functions work as expected are retained.

@effigies effigies changed the title RF: Remove longdouble<->int hacks RF: Deprecate longdouble<->int hacks Nov 2, 2023
@effigies
Copy link
Member Author

effigies commented Nov 2, 2023

@matthew-brett You added these back in #70, but I think I'm right that they can go away at this point. Any objections? Any other regression tests worth putting in?

@larsoner or @mscheltienne Would you mind having a review, since you were just dealing with these?

@effigies effigies added this to the 5.2.0 milestone Nov 2, 2023
@codecov
Copy link

codecov bot commented Nov 2, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (dabfc46) 92.25% compared to head (bbfd009) 92.21%.

Additional details and impacted files
@@ Coverage Diff @@ ## master #1272 +/- ## ========================================== - Coverage 92.25% 92.21% -0.04%  ========================================== Files 99 99 Lines 12466 12455 -11 Branches 2564 2561 -3 ========================================== - Hits 11500 11486 -14  - Misses 643 645 +2  - Partials 323 324 +1 
Files Coverage Δ
nibabel/casting.py 86.17% <100.00%> (-1.06%) ⬇️
nibabel/conftest.py 93.75% <100.00%> (-6.25%) ⬇️
nibabel/arraywriters.py 96.86% <91.66%> (ø)

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@larsoner larsoner left a comment

Choose a reason for hiding this comment

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

Much simpler!

@effigies
Copy link
Member Author

effigies commented Nov 6, 2023

Thanks for the review!

@effigies effigies merged commit f815aba into nipy:master Nov 6, 2023
@effigies effigies deleted the rf/remove-longdouble-hacks branch December 3, 2023 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants