-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Consistent rounding rules for float display with fixed precision #70751
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Consistent rounding rules for float display with fixed precision #70751
Conversation
r? @cramertj (rust_highfive has picked a reviewer for you, use r? to override) |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
r? @dtolnay |
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.
I haven't gotten a chance to review the implementation yet but it looks like there is some test code commented out in flt2dec/mod.rs that probably shouldn't be. Is that on purpose? What's the deal with those tests?
t!(format!("{:.3e}", 1.2345e6f64), "1.234e6"); | ||
t!(format!("{:10.3e}", 1.2345e6f64), " 1.234e6"); | ||
t!(format!("{:+10.3e}", 1.2345e6f64), " +1.234e6"); | ||
t!(format!("{:+10.3e}", -1.2345e6f64), " -1.234e6"); | ||
t!(format!("{:.3e}", 1.2345e6f64), "1.235e6"); | ||
t!(format!("{:10.3e}", 1.2345e6f64), " 1.235e6"); | ||
t!(format!("{:+10.3e}", 1.2345e6f64), " +1.235e6"); | ||
t!(format!("{:+10.3e}", -1.2345e6f64), " -1.235e6"); |
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.
It looks to me like these were correct before. Round-to-even on 1234.5 should go to 1234, not 1235.
@ayushmishra2005 Ping from triage, would you mind addressing the review comments above? thanks |
@crlf0710 I am closing this pull request for now because I don't have the bandwidth to address them. I will raise new PR after addressing the above comments. |
#70336