-
-
Notifications
You must be signed in to change notification settings - Fork 310
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
Allow arrows to take a Function to produce the vector #2597
Conversation
# and magnitude of the arrows. The function must accept `Point2f` as input. | ||
# and return Point2f or Vec2f or some array like structure as output. | ||
function convert_arguments(::Type{<: Arrows}, x::AbstractVector, y::AbstractVector, f::Function) | ||
points = Point2f.(x, y') |
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.
Shouldn't that be Point2f.(x, y)
?
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.
No, arrows always creates a matrix - the eventual positions are Point2f.(x, y')
anyway. Plus the inputs are AbstractVectors so we know that we have to make the grid.
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.
Hmm, it does one or the other depending on the type of directions:
Makie.jl/src/basic_recipes/arrows.jl
Lines 105 to 108 in c03f7a5
convert_arguments(::Type{<: Arrows}, x, y, u, v) = (Point2f.(x, y), Vec2f.(u, v)) | |
function convert_arguments(::Type{<: Arrows}, x::AbstractVector, y::AbstractVector, u::AbstractMatrix, v::AbstractMatrix) | |
(vec(Point2f.(x, y')), vec(Vec2f.(u, v))) | |
end |
That makes this a bit awkward to reason with...
To me it seems more intuitive to just directly pass on args rather than doing a conversion beforehand. I.e. arrows(xs, ys, (xs, ys) -> Vec2f.(-ys, xs)) rather than arrows(xs, ys, ps -> Vec2f[(-p[2], p[1]) for p in ps]) That seems to be what surface and volume do. I also wonder if we should just have a default conversion of |
I do like the idea of that second conversion - but it might be easier to implement it per plot type. |
Closing in favor of #3080 |
Description
Allow
arrows
to take input of the formx::AbstractVector, y::AbstractVector, [z::AbstractVector,] f::Function
Add a description of your PR here.
Type of change
Delete options that do not apply:
Checklist