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

Improve document for std::fmt to format float type #112760

Closed

Conversation

eval-exec
Copy link
Contributor

@eval-exec eval-exec commented Jun 18, 2023

This PR want to close #112742

@rustbot
Copy link
Collaborator

rustbot commented Jun 18, 2023

r? @Mark-Simulacrum

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jun 18, 2023
@eval-exec eval-exec marked this pull request as draft June 18, 2023 09:37
@eval-exec eval-exec force-pushed the exec/fix-fmt-float-rounding branch 2 times, most recently from 3a1cdd6 to 194d318 Compare June 18, 2023 09:39
@rust-log-analyzer

This comment has been minimized.

@eval-exec eval-exec changed the title Improve format! Precision description for float type [WIP] Improve format! Precision description for float type Jun 18, 2023
@eval-exec eval-exec changed the title [WIP] Improve format! Precision description for float type [WIP] Fix format! Precision description for float type Jun 18, 2023
@rust-log-analyzer

This comment has been minimized.

@eval-exec eval-exec force-pushed the exec/fix-fmt-float-rounding branch from 1d2ab77 to 194d318 Compare June 24, 2023 03:44
@eval-exec eval-exec changed the title [WIP] Fix format! Precision description for float type Improve document for std::fmt to format float type Jun 24, 2023
@rust-log-analyzer

This comment has been minimized.

Signed-off-by: Eval EXEC <execvy@gmail.com>
@eval-exec eval-exec force-pushed the exec/fix-fmt-float-rounding branch from 194d318 to c4b0980 Compare June 24, 2023 03:49
@eval-exec eval-exec marked this pull request as ready for review June 24, 2023 03:50
@Mark-Simulacrum
Copy link
Member

r? libs-api since it seems like this is a new guarantee, which I'm not sure if we want to make.

@rustbot rustbot assigned dtolnay and unassigned Mark-Simulacrum Jun 24, 2023
@Mark-Simulacrum Mark-Simulacrum added needs-fcp This change is insta-stable, so needs a completed FCP to proceed. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jun 24, 2023
Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

I think it would be all right to document a rounding behavior.

Comment on lines +280 to +304
//! println!("{:.0}", 12.5f64); // 12
//! println!("{:.0}", 12.3f64); // 12
//! println!("{:.0}", 11.6f64); // 12
//! println!("{:.0}", 11.5f64); // 12
//! println!("{:.0}", 11.4f64); // 11
//! println!("{:.0}", 10.5f64); // 10
//! println!("{:.0}", 9.5f64); // 10
//! println!("{:.0}", 8.5f64); // 8
//! println!("{:.0}", 7.5f64); // 8
//! println!("{:.0}", -7.5f64); // -8
//! println!("{:.0}", -8.5f64); // -8
//! println!("{:.0}", -9.5f64); // -10
//! println!("{:.0}", -10.5f64);// -10
//! println!("{:.0}", -11.4f64);// -11
//! println!("{:.0}", -11.5f64);// -12
//! println!("{:.0}", -11.6f64);// -12
//! println!("{:.0}", -12.5f64);// -12
//! println!("{:.0}", -12.6f64);// -13
//!
//! println!("{:.1}", 3.33f64); // 3.3
//! println!("{:.1}", -3.33f64);// -3.3
//! println!("{:.1}", -3.55f64);// -3.5
//! println!("{:.1}", 4.54f64); // 4.5
//! println!("{:.1}", 4.55f64); // 4.5
//! println!("{:.1}", 4.56f64); // 4.6
Copy link
Member

@dtolnay dtolnay Jul 2, 2023

Choose a reason for hiding this comment

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

This seems like an extremely excessive number of examples. Can {:.1} with 1.25 and 1.75 be sufficient to illustrate what you mean?

@@ -273,6 +273,37 @@
//! Hello, ` 123` has 3 right-aligned characters
//! ```
//!
//! For the floating-point type, it employs the
//! [roundTiesToEven](https://en.m.wikipedia.org/wiki/Rounding#Rounding_half_to_even) convention:
Copy link
Member

Choose a reason for hiding this comment

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

What does the name "roundTiesToEven" refer to exactly? I did not find that anywhere in the link.

Comment on lines +276 to +277
//! For the floating-point type, it employs the
//! [roundTiesToEven](https://en.m.wikipedia.org/wiki/Rounding#Rounding_half_to_even) convention:
Copy link
Member

Choose a reason for hiding this comment

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

This is unlikely to be the most appropriate link because the subsection you linked to is describing the operation of rounding a float to an integer, which is different in general from formatting a float to a particular precision. The text even calls this out: a "more sophisticated mode [is] used when rounding to significant figures".

//! println!("{:.1}", 4.55f64); // 4.5
//! println!("{:.1}", 4.56f64); // 4.6
//! ```
//!
Copy link
Member

Choose a reason for hiding this comment

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

There is already a paragraph above which describes how precision applies to floating point types. It would make more sense to me to explain the rounding directly in that paragraph.

//! For floating-point types, this indicates how many digits after the decimal point should be
//! printed.

@dtolnay dtolnay added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 2, 2023
@Dylan-DPC
Copy link
Member

@eval-exec any updates on this?

@Dylan-DPC
Copy link
Member

Closing this as inactive. Feel free to reöpen this pr or create a new pr if you get the time to work on this. Thanks

@Dylan-DPC Dylan-DPC closed this Feb 10, 2024
@Dylan-DPC Dylan-DPC removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Feb 10, 2024
@Dylan-DPC Dylan-DPC added the S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. label Feb 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-fcp This change is insta-stable, so needs a completed FCP to proceed. S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

document that std::fmt uses ROUND_HALF_EVEN
6 participants