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

Implement and test exp for Ratio #291

Merged
merged 5 commits into from
Mar 26, 2022
Merged

Conversation

jacg
Copy link
Contributor

@jacg jacg commented Mar 23, 2022

Close #290.

Please have a look if this is up to standard. If so, I'd be happy to add exp2, ln, log, log2 and log10 in a similar fashion.

src/si/ratio.rs Outdated
Comment on lines 80 to 82
/// Implementation of various stdlib exponentiation and logarithm functions
#[cfg(feature = "std")]
impl<U, V> Ratio<U, V>
Copy link
Owner

Choose a reason for hiding this comment

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

Go ahead and merge this impl block with the trig block and update the comments. Same for the tests below. rename mod inv_trig to mod float and include all of the functions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

I have also included exp_m1 and ln_1p which weren't in the original list.

log takes a second argument, besides self. I've given it the same type as self but I'm not sure whether we want to be more flexible. So I've added that one in a separate commit.

I've also added [must_use = ...] to the inverse-trig functions which were already there. Also in a separate commit, in case you don't want it. The must_use is there from the beginning for all the new functions that I added.

@iliekturtles
Copy link
Owner

Things look good after a quick skim so I kicked off the test workflows. I'll review in full detail later today.

For the [must_use = ...] attributes I do want to keep them and will add a new issue to review other existing methods.

src/si/ratio.rs Outdated Show resolved Hide resolved
@iliekturtles iliekturtles merged commit 66d3e85 into iliekturtles:master Mar 26, 2022
@jacg jacg deleted the exp-log branch March 29, 2022 12:43
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.

Suport .exp() for Ratio
2 participants