Repository navigation
Fix two rounding errors in Angle.to_string - #20227
Merged
mhvk merged 2 commits intoAug 10, 2026
Merged
Conversation
The carry threshold was a full unit below 60 instead of half of one, so
Angle("1d2m59.49999s").to_string(precision=0) gave 1d03m00s, not
1d02m59s. Rounding and comparing to 60 needs no threshold at all. Not
NumPy's round, which disagrees on ties like 59.995 at precision 2.
Signed-off-by: Shridhar Panigrahi <[email protected]>
With fields=1 the seconds were rounded into the minutes at 30, and the
minutes then into the degrees at 30 as well, so an angle got rounded
twice: Angle("285d29m41.76s").to_string(fields=1) gave 286d, not 285d.
Signed-off-by: Shridhar Panigrahi <[email protected]>
Contributor
|
Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.
|
mhvk
approved these changes
Aug 10, 2026
mhvk
left a comment
mhvk
left a comment
Contributor
There was a problem hiding this comment.
Thanks, very nice! I wrote two nits about the examples, but thinking more about it, I think it is fine as is, since in a way your examples are things that went wrong before, where it is OK that it is obvious that what you expect is correct.
So, let me just get it in, thanks!
| [ | ||
| # Just below half a unit in the last digit shown: no carry. | ||
| ("1d2m59.49999s", 0, "1d02m59s"), | ||
| ("1d2m59.94s", 1, "1d02m59.9s"), |
Contributor
There was a problem hiding this comment.
I'd add some 9 here, as in .949999
| # At or above it: the seconds carry into the minutes, and on into | ||
| # the degrees. | ||
| ("1d2m59.5s", 0, "1d03m00s"), | ||
| ("1d2m59.96s", 1, "1d03m00.0s"), |
mhvk
added
backport-v7.2.x
mhvk
enabled auto-merge
August 10, 2026 17:42
This was referenced Aug 10, 2026
pllim
pushed a commit
that referenced
this pull request
Aug 10, 2026
pllim
added a commit
that referenced
this pull request
Aug 10, 2026
…227-on-v8.0.x Backport PR #20227 on branch v8.0.x (Fix two rounding errors in Angle.to_string)
pllim
pushed a commit
that referenced
this pull request
Aug 10, 2026
pllim
added a commit
that referenced
this pull request
Aug 10, 2026
…227-on-v7.2.x Backport PR #20227 on branch v7.2.x (Fix two rounding errors in Angle.to_string)
1 task done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Split out of #20130, where @mhvk spotted the first of these.
The seconds carry into the minutes too early:
The threshold sat a whole unit in the last digit shown below 60 instead of half of one. As @mhvk suggested it now rounds the seconds and compares against 60, so there is no threshold left. It does need the builtin
roundon a float, since numpy's scales by ten and back and calls 59.995 at precision 2 a carry where the formatting gives 59.99.And with
fields=1the angle is rounded twice, seconds into minutes and then minutes into degrees:The degrees are now rounded on the minutes and seconds together.
Over 40,000 angles per case this moves 295 for
fields=1, and 365 and 19 forfields=3at precision 0 and 1. Nothing else changes. Tests for both, checked to fail against the current code.