-
Notifications
You must be signed in to change notification settings - Fork 44
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
Fix: rotate a vector of quantities with AngleAxis
#296
Conversation
Thank you for the PR! |
@hyrodium tests added. Can you check if they follow the code style? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for adding tests!
@@ -110,7 +110,7 @@ function Base.:*(aa::AngleAxis, v::StaticVector) | |||
st, ct = sincos(aa.theta) | |||
w_cross_pt = cross(w, v) | |||
m = dot(v, w) * (one(w_cross_pt[1]) - ct) | |||
T = promote_type(eltype(aa), eltype(v)) | |||
T = Base.promote_op(*, eltype(aa), eltype(v)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't fully understand the practical reason why Base.promote_op
is avoided, but Base.promote_op
is already used in other places in Rotations. jl like in this PR, so I will approve this PR.
@hyrodium can you release a patch version with this PR? |
In the current version of Rotations.jl, rotating a vector of quantities with
AngleAxis
returns a new vector with incorrect eltype:This PR fixes this bug: