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

Tracking issue for core::{f32,f64}::consts::TAU #66770

Closed
3 tasks done
m-ou-se opened this issue Nov 26, 2019 · 23 comments · Fixed by #74552
Closed
3 tasks done

Tracking issue for core::{f32,f64}::consts::TAU #66770

m-ou-se opened this issue Nov 26, 2019 · 23 comments · Fixed by #74552
Labels
B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@m-ou-se
Copy link
Member

m-ou-se commented Nov 26, 2019

Tracking issue for the TAU constant added in #66769.


Before stabilization:

  • Confirm that the numeric values we have are accurate.
  • Decide whether we are comfortable using the name TAU or instead a longer but more widely accepted name like TWO_PI could be better.
  • Wait for RFC that moves math constants from std::{f32,f64}::consts::.. to {f32,f64}::.. ?
@jonas-schievink jonas-schievink added B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Nov 26, 2019
@steveklabnik

This comment has been minimized.

@m-ou-se

This comment has been minimized.

@steveklabnik

This comment has been minimized.

@steveklabnik steveklabnik reopened this Nov 26, 2019
@jonas-schievink

This comment has been minimized.

@tesuji
Copy link
Contributor

tesuji commented Nov 26, 2019

I don't think we should land this constant.
Acording to wikipedia:

It has also been used in at least one mathematical research article,[33] authored by the τ-promoter Peter Harremoës.[34]

However, none of these proposals have received widespread acceptance by the mathematical and scientific communities.[35

@qnighy
Copy link
Contributor

qnighy commented Nov 27, 2019

Naming aside, such a constant would be definitely useful. 2π couples tightly in the mathematical notation where 2.0 * PI doesn't, like x / (2.0 * PI).

@m-ou-se
Copy link
Member Author

m-ou-se commented Nov 27, 2019

  • Confirm that the numeric values we have are accurate.

Sources:

Can someone other than me verify that I copied the number properly?

Edit: Done by dtolnay.

@m-ou-se
Copy link
Member Author

m-ou-se commented Nov 27, 2019

  • Decide whether we are comfortable using the name TAU or instead a longer but more widely accepted name like TWO_PI could be better.

Unfortunately, the constant TWO_PI or 2_PI has been used in several programming languages for 2/π instead of 2π:

Although I agree that having TWO_PI or 2_PI refer to anything other than 2π makes absolutely no sense, I do think it might cause confusion because of the existing practice.

@andrewrk
Copy link

* https://ziglang.org/documentation/master/std/#std;math.two_pi

Our std lib is still unstable, we're willing to change it to a more canonical, sensible API if you wanna lead the way here :)

@m-ou-se
Copy link
Member Author

m-ou-se commented Nov 29, 2019

Regardless of what C or other languages call their constants, I'd name this one TAU and not anything with PI. In the past five years, quite a few people have asked for a TAU constant. Looking at the amount of thumbs-ups at for example #66769 or rust-lang/rfcs#1486, it seems quite some people are in favour of TAU.

@adamnemecek
Copy link

Yeah, TAU is way, way better than TWO_PI.

@andrewrk
Copy link

ziglang/zig@2ab7f31

@m-ou-se
Copy link
Member Author

m-ou-se commented Jan 9, 2020

Can we make a decision for the name TAU based on the comments above? Or do we need more time/experience/comments/opinions/bikeshedding/something before we can make a decision?

@jonas-schievink
Copy link
Contributor

Normally we leave new library additions unstable for a few release cycles. This one seems very trivial though. In any case, some libs team member has to kick off FCP to get this stabilized, and they might not have the capacity at the moment.

@LukasKalbertodt
Copy link
Member

LukasKalbertodt commented Apr 6, 2020

One thing worth discussing before stabilizing TAU, is whether or not we want to move all constants in std::{f32,f64}::consts to be associated consts of f32/f64. This is mentioned in the alternative section of RFC 2700. This issue is tracked and discussed here. If we indeed want to move to associated consts, it would be a good idea to not stabilize tau as normal constant first.

@m-ou-se
Copy link
Member Author

m-ou-se commented Jun 2, 2020

Only things like MIN have been moved to associated constants, and there seem to be no plans yet to move mathematical constants like PI to associated constants as well. (RFC2700 explicitly didn't suggest moving the math constants.) I'd prefer to stabilize TAU without waiting for a possible RFC to show up which might move them.

@faern
Copy link
Contributor

faern commented Jun 9, 2020

I have never coded with radians in Rust before, but I started a few weeks back and I instantly defined const TAU: f32 = 2.0 * PI; everywhere. So having this constant in the standard library feels really relevant to me. And I like the name.

However, I'm really really in favor of moving the math constants to associated constants. I liked the more full on version of RFC 2700 that is linked above. Because now I have to look at my import/const define section at the top of the file to know what resolution I have available. And it's cumbersome to use 32 bit and 64 bit resolution math in the same module. As a result, I'm only in favor of stabilizing std::fxx::consts::TAU if it does not make people less in favor of deprecating the entire std::fxx::consts module soon/later.

@LukasKalbertodt
Copy link
Member

I don't have a hard opinion on whether math constants should live in consts or on the type, I think. But I was convinced by @m-ou-se's reasoning: we shouldn't wait with stabilizing TAU just because in the future we maybe want to move the constants to a different location. One additional constant in f32::consts won't make a difference.

@rust-lang/libs Does anyone of you want to start an FCP? From what I can tell, all questions have been resolved and this has been cooking on nightly for several cycles already.

@Amanieu
Copy link
Member

Amanieu commented Jun 21, 2020

@rfcbot fcp merge

@rfcbot
Copy link

rfcbot commented Jun 21, 2020

Team member @Amanieu has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Jun 21, 2020
@m-ou-se
Copy link
Member Author

m-ou-se commented Jul 10, 2020

@SimonSapin @sfackler @withoutboats Friendly ping. Can this be stabilized before the next release?

@rfcbot
Copy link

rfcbot commented Jul 10, 2020

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Jul 10, 2020
@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Jul 20, 2020
@rfcbot
Copy link

rfcbot commented Jul 20, 2020

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.