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

rotation_between for general dimensions #257

Merged

Conversation

hyrodium
Copy link
Collaborator

@hyrodium hyrodium commented May 5, 2023

This PR fixes #256.

julia> using Rotations

julia> using StaticArrays

julia> rotation_between(SVector(1,2,3,5),SVector(1,3,2,5))  # Can be `Rotation{4}`
4×4 RotMatrix{4, Float64, 16} with indices SOneTo(4)×SOneTo(4):
  0.999334  -0.027306  0.023976  -0.00333
  0.023976   0.983017  0.136863   0.11988
 -0.027306  -0.119547  0.983017  -0.13653
 -0.00333   -0.13653   0.11988    0.98335

julia> rotation_between(SVector(3,1),SVector(1,3))  # Can be `Rotation{2}`
2×2 Angle2d{Float64} with indices SOneTo(2)×SOneTo(2)(0.927295):
 0.6  -0.8
 0.8   0.6

@hyrodium hyrodium marked this pull request as ready for review May 10, 2023 13:51
@codecov
Copy link

codecov bot commented May 10, 2023

Codecov Report

Merging #257 (8700683) into master (d56bc26) will increase coverage by 0.32%.
The diff coverage is 100.00%.

❗ Current head 8700683 differs from pull request most recent head 486de18. Consider uploading reports for the commit 486de18 to get more accurate results

@@            Coverage Diff             @@
##           master     #257      +/-   ##
==========================================
+ Coverage   89.51%   89.84%   +0.32%     
==========================================
  Files          15       17       +2     
  Lines        1593     1615      +22     
==========================================
+ Hits         1426     1451      +25     
+ Misses        167      164       -3     
Impacted Files Coverage Δ
src/Rotations.jl 100.00% <ø> (ø)
src/unitquaternion.jl 97.46% <ø> (-0.10%) ⬇️
src/rotation_between.jl 100.00% <100.00%> (ø)

... and 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@juliohm
Copy link
Member

juliohm commented May 12, 2023

What about making this helper function a new rotation type instead? Maybe a simple Between(u, v)? That would avoid using the naming convention with underscore.

@hyrodium
Copy link
Collaborator Author

What about making this helper function a new rotation type instead? Maybe a simple Between(u, v)? That would avoid using the naming convention with underscore.

What's the benefit of the new type Between?
The role of rotation_between is similar to range. These functions return various types which depend on their inputs.

julia> range(1,2,3)
1.0:0.5:2.0

julia> range(1,3)
1:3

julia> range(1,2,3) |> typeof
StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}

julia> range(1,3) |> typeof
UnitRange{Int64}
julia> rotation_between(SVector(1,2), SVector(2,1))
2×2 Angle2d{Float64} with indices SOneTo(2)×SOneTo(2)(-0.643501):
  0.8  0.6
 -0.6  0.8

julia> rotation_between(SVector(1,2,3), SVector(2,1,3))
3×3 QuatRotation{Float64} with indices SOneTo(3)×SOneTo(3)(QuaternionF64(0.981981, 0.109109, 0.109109, -0.109109)):
  0.952381  0.238095   0.190476
 -0.190476  0.952381  -0.238095
 -0.238095  0.190476   0.952381

julia> rotation_between(SVector(1,2), SVector(2,1)) |> typeof
Angle2d{Float64}

julia> rotation_between(SVector(1,2,3), SVector(2,1,3)) |> typeof
QuatRotation{Float64}

That would avoid using the naming convention with underscore.

Julia style guide, blue style, and sciml style recommend using snake_case, so there's no problem with underscores.

@juliohm
Copy link
Member

juliohm commented May 14, 2023 via email

@hyrodium hyrodium merged commit b04d039 into JuliaGeometry:master May 14, 2023
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.

Add more rotation_between methods for general dimensions
2 participants