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

RFC: Add rad2deg, deg2rad for Taylor1 and TaylorN #142

Merged
merged 3 commits into from
Dec 5, 2017
Merged

RFC: Add rad2deg, deg2rad for Taylor1 and TaylorN #142

merged 3 commits into from
Dec 5, 2017

Conversation

PerezHz
Copy link
Contributor

@PerezHz PerezHz commented Dec 3, 2017

Self-explaining title 😉... Was working with these functions with Taylor1 and thought it'd be nice to add them to TaylorSeries!

@coveralls
Copy link

coveralls commented Dec 3, 2017

Coverage Status

Changes Unknown when pulling 7d5d09b on PerezHz:jp/rad2deg2rad into ** on JuliaDiff:master**.

@lbenet
Copy link
Member

lbenet commented Dec 5, 2017

Thanks! LGTM!

If you are using it together with TaylorIntegration, it is maybe a good idea to include the equivalent mutating functions, e.g., rad2deg(res, a, k). This is of interest in the context of PerezHz/TaylorIntegration.jl#31

@PerezHz
Copy link
Contributor Author

PerezHz commented Dec 5, 2017

If you are using it together with TaylorIntegration, it is maybe a good idea to include the equivalent mutating functions, e.g., rad2deg(res, a, k). This is of interest in the context of PerezHz/TaylorIntegration.jl#31

That's actually a really nice suggestion! Sure, I'll add them right away!

@lbenet
Copy link
Member

lbenet commented Dec 5, 2017

Be sure to add them in the dictionaries that are used in TaylorIntegration. Let me know when you are ready...

@lbenet
Copy link
Member

lbenet commented Dec 5, 2017

I just noticed that you will have to rebase...

@PerezHz
Copy link
Contributor Author

PerezHz commented Dec 5, 2017

Be sure to add them in the dictionaries that are used in TaylorIntegration.

Good catch! Just rebased to current master, added the mutating functions, updated the Dicts and also added corresponding tests

@PerezHz
Copy link
Contributor Author

PerezHz commented Dec 5, 2017

Just saw that #143 is about to be merged... I can re-rebase to latest master once that PR is merged

@coveralls
Copy link

coveralls commented Dec 5, 2017

Coverage Status

Coverage increased (+0.03%) to 96.632% when pulling 5f53f3b on PerezHz:jp/rad2deg2rad into c00bae7 on JuliaDiff:master.

Copy link
Member

@lbenet lbenet left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

@lbenet lbenet merged commit 008b67d into JuliaDiff:master Dec 5, 2017
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.

3 participants