Skip to content

Conversation

@gustaphe
Copy link
Collaborator

Allows Shape to hold other types than Float64, so that recipes can dispatch on the vertices.

With this change, for instance, Shape([(0u"m", 1u"s"), (1u"m", 1u"s"), (1u"m", 2u"s")]) (with Unitful units u"m" and u"s") is a valid Shape, and when plotted it will invoke the recipes for the respective axis types:

julia> using Unitful, Plots, UnitfulRecipes

julia> s = Shape([(0u"m", 1u"s"), (1u"m", 1u"s"), (1u"m", 2u"s")])
Shape{Quantity{Int64, 𝐋, Unitful.FreeUnits{(m,), 𝐋, nothing}}, Quantity{Int64, 𝐓, Unitful.FreeUnits{(s,), 𝐓, nothing}}}(Quantity{Int64, 𝐋, Unitful.FreeUnits{(m,), 𝐋, nothing}}[0 m, 1 m, 1 m], Quantity{Int64, 𝐓, Unitful.FreeUnits{(s,), 𝐓, nothing}}[1 s, 1 s, 2 s])

julia> plot(s)

shape


"get an array of tuples of points on a circle with radius `r`"
function partialcircle(start_θ, end_θ, n = 20, r=1)
Tuple{Float64,Float64}[(r*cos(u),r*sin(u)) for u in range(start_θ, stop=end_θ, length=n)]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The removal of the type annotation here is only necessary for creating circles with a radius ::MyType. The type in these tuples will be that of *(::MyType, ::Float64), but I don't think annotating that changes the type of the vector.

If this annotation is very important for performance, this change could easily be reverted, it's not a very important function.

@codecov
Copy link

codecov bot commented Apr 14, 2021

Codecov Report

Merging #3436 (b196396) into master (0a70359) will increase coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head b196396 differs from pull request most recent head a359f21. Consider uploading reports for the commit a359f21 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3436      +/-   ##
==========================================
+ Coverage   63.64%   63.65%   +0.01%     
==========================================
  Files          27       27              
  Lines        6582     6585       +3     
==========================================
+ Hits         4189     4192       +3     
  Misses       2393     2393              
Impacted Files Coverage Δ
src/components.jl 57.06% <100.00%> (+0.35%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0a70359...a359f21. Read the comment docs.

Comment on lines +208 to +213
function rotate(shape::Shape, θ::Real, c = center(shape))
x, y = coords(shape)
cx, cy = c
x_new = rotate_x.(x, y, θ, cx, cy)
y_new = rotate_y.(x, y, θ, cx, cy)
Shape(x_new, y_new)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Rotating a Shape{Int, Int} will leave you with a Shape{Float64, Float64}, so rotate! doesn't work in general. I did a quick benchmark, and this function if anything seems slightly faster than the rotate! version.

Copy link
Member

@daschw daschw left a comment

Choose a reason for hiding this comment

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

Looks great!

@daschw daschw merged commit e3d13aa into JuliaPlots:master Apr 28, 2021
@daschw
Copy link
Member

daschw commented Apr 28, 2021

Thanks!

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.

2 participants