-
Notifications
You must be signed in to change notification settings - Fork 298
Fix guess_bounds precision for circular coords at numpy v2 #6793
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
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v3.14.x #6793 +/- ##
========================================
Coverage 90.39% 90.39%
========================================
Files 91 91
Lines 24761 24763 +2
Branches 4639 4639
========================================
+ Hits 22382 22384 +2
Misses 1608 1608
Partials 771 771 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
40a398d to
336a21d
Compare
trexfeathers
left a comment
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.
Thanks for tackling this @rcomer! I have one suggestion, for honouring the original dtype instead of always using float64.
|
Great, thanks! Once we've got In case it isn't clear: I can't merge it now because that would mean cutting a new release candidate and waiting another two weeks. |
|
Thanks @trexfeathers - I didn't know about |
|
Do I need to rebase against v3.14.x? Also will the whatnew need to move? |
Co-authored-by: Martin Yeo <[email protected]>
0e5790b to
13f9d4f
Compare
|
I have attempted to follow the pattern of recent patches for the whatsnew. |
|
Thank you for being so accommodating @trexfeathers! |
🚀 Pull Request
Description
Fixes #6738, or at least it fixes my use case 😉
The coordinate's modulus is a python float. It seems that at numpy v1 doing arithmetic between a
np.float32and afloatgave anp.float64. At numpy v2 it quite reasonably gives anp.float32, but that means we are now calculating at lower precision than we were. Making the modulus into anp.float64seems like the simplest way to reinstate the previous behaviour.Consult Iris pull request check list
Add any of the below labels to trigger actions on this PR: