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

Change trigonometric functions to return ratio instead of bare values #187

Merged
merged 1 commit into from May 11, 2020
Merged

Change trigonometric functions to return ratio instead of bare values #187

merged 1 commit into from May 11, 2020

Conversation

adamreichold
Copy link
Contributor

Fixes #185

… s.t. identities like x.sin().asin() = x are well-typed.
@adamreichold
Copy link
Contributor Author

@iliekturtles While doing this, I was a bit surprised by the test

#[test]
fn from() {
    let r1: Angle<V> = Angle::<V>::from(V::one());
    let r2: Angle<V> = V::one().into();
    let _: V = V::from(r1);
    let _: V = r2.into();
}

for Angle. Shouldn't these kind of transformations be limited to Ratio as the above basically assumes radian (resp. the base unit)?

@iliekturtles
Copy link
Owner

Thanks for the PR! Initial review from my phone looks good. I'll plan on merging once I can do a quick review from a computer.

For the From/Into impls I can't remember the history off the top of my head. Once I'm back at a computer I'll review the history and consider removing.

@iliekturtles iliekturtles merged commit afd4e51 into iliekturtles:master May 11, 2020
@iliekturtles
Copy link
Owner

Merged! Thanks for your patience. I'll look at your other PR next.

For the From/Into impls they were included with the original PR to add Angle. There was no discussion about them and perhaps they were copied from the Ratio implementation. I've add #188 to discuss removing them.

@adamreichold adamreichold deleted the ratio-for-trig-functions branch May 11, 2020 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change trigonometric functions to return Ratio?
2 participants