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.

Sign up for GitHub

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

[WIP] Non-Working Example for custom shapes #138

Merged
merged 8 commits into from
Nov 18, 2020
Merged

[WIP] Non-Working Example for custom shapes #138

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
85bb55f
Non-Working Example for custom shapes
Nov 8, 2020
d13992c
Added custom nodeshapes
Nov 10, 2020
68aab9c
Added a test for custom shapes and set markershape to :circle
Nov 11, 2020
daadad2
Replaced Tabs by spaces and added image for test
Nov 11, 2020
e755f04
Changed the way nodeshapes are applied to markershapes and added a
Nov 14, 2020
ad70189
Added more descriptive comment for unknown nodeshape errors.
Nov 14, 2020
8362ca1
Added different style of generating custom nodeshapes to tests
Nov 14, 2020
e286e0a
Removed old test shape
Nov 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file added assets/custom_nodeshapes.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
12 changes: 11 additions & 1 deletion src/graphs.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,11 @@ more details.
plotattributes[markerproperty] = nodeproperty
end

# Remove the markershapes from the dictionary to allow for nodeshapes not
# compatible with the shapes of plotting ( Custom nodeshape functions don't
# work without this line)
Copy link
Member

Choose a reason for hiding this comment

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

I don't agree with this comment.

The nodeshape is placed into plotattributes[:markershape] at line 355:

GraphRecipes.jl/src/graphs.jl

Line 355 in 7210d36

plotattributes[markerproperty] = nodeproperty

However, if plotattributes[:markershape] is a function (or a symbol that the backend doesn't recognize), then the backend will fail to create the marker series even though we ultimately create our own marker series later on in the recipe.

Ultimately, the whole processing of the attributes that I did from lines 337-356 was a bit of a cludge. I think that the code there is ultimately what has generated the whole confusion.

I would recommend that you add the following to line 357;

    # If we pass a value of plotattributes[:markershape] that the backend does not
    # recognize, then the backend will throw an error. The error is thrown despite the
    # fact that we override the default behavior.
    if nodeshape isa Function
        plotattributes[:markershape] = :circle
    end

The conditional is not strictly necessary, but I think it helps to show that we are only changing the disctionary when plotattributes[:markershape] is something that the backend does not recognize.

Feel free to alter the comment a little to make it easier to understand/add your own flare.

Copy link
Member

Choose a reason for hiding this comment

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

I still don't agree with this comment. In particular, I don't understand the phrase "nodeshapes not compatible with the shapes of plotting". The phrase "shapes of plotting" does have a very sentimental/artistic flare, however, is not really specific enough for this context. Rather than "shapes of plotting", I think that you should say "symbols that the backend will recognize as a valid value of plotattributes[:markershape]".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My main intention with the comment was to keep the knowledge why this special case had to be introduced. Your code and comment capture that much better. I just added a line to make clarify that the lines where introduced to allow custom nodeshapes.

I changed the code to match yours, the tests then suggested some further changes.

plotattributes[:markershape] = :circle

@assert dim in (2, 3)
_3d = dim == 3
adj_mat = get_adjacency_matrix(g.args...)
Expand Down Expand Up @@ -487,8 +492,13 @@ more details.
nodeheight = (yextent[2] - yextent[1])
push!(node_vec_vec_xy, partialellipse(0, 2π, [x[i], y[i]],
80, nodewidth/2, nodeheight/2))
elseif applicable(nodeshape[node_number], x[i], y[i], 0., 0.)
nodeheight = (yextent[2] - yextent[1])
push!(node_vec_vec_xy, nodeshape[node_number](x[i], y[i], nodewidth, nodeheight))
elseif applicable(nodeshape[node_number], x[i], y[i], 0.)
push!(node_vec_vec_xy, nodeshape[node_number](x[i], y[i], nodewidth))
else
error("Unknown nodeshape: $(nodeshape[node_number]). Choose from :circle, ellipse, :hexagon, :rect or :rectangle.")
error("Unknown nodeshape: $(nodeshape[node_number]). Choose from :circle, ellipse, :hexagon, :rect or :rectangle or write a function providing custom shapes.")
end
end
else
Expand Down
2 changes: 2 additions & 0 deletions test/figures.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ cd("../assets")
@plottest julia_dict_tree() "julia_dict_tree.png" popup=!istravis tol=0.2

@plottest funky_edge_and_marker_args() "funky_edge_and_marker_args.png" popup=!istravis tol=0.2

@plottest custom_nodeshapes() "custom_nodeshapes.png" popup=!istravis tol=0.2
end

@testset "README" begin
Expand Down
32 changes: 32 additions & 0 deletions test/functions.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,38 @@ function julia_dict_tree()
plot(TreePlot(d), method=:tree, fontsize=10, nodeshape=:ellipse, size=(1000, 1000))
end

function custom_nodeshapes()
function shapy(x_i,y_i, s)
Copy link
Member

Choose a reason for hiding this comment

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

Inconsistent whitespace conventions. I also like to have no spaces after a comma, however, the convention for the JuliaPlots orginisation is to have whitespace after commas, so shapy(x_i, y_i, s). Same for shap_wh below

out = Tuple{Float64, Float64}[(-0.5,0),(0,-0.5),(0.5,0),(0,0.5)]
map(out) do t
x = t[1]* s
y = t[2]* s
(
x + x_i,
y + y_i
)
end
end
function shapy_wh(x_i,y_i, h, w)
out = Tuple{Float64, Float64}[(-0.5,0),(0,-0.5),(0.5,0),(0,0.5)]
map(out) do t
x = t[1]* h
y = t[2]* w
(
x + x_i,
y + y_i
)
end
end
Random.seed!(6)
g = rand(5,5)
g[g .> 0.5] .= 0
for i in 1:5
g[i,i] = 0
end
graphplot(g, nodeshape=[:circle, shapy, shapy_wh, :hexagon, shapy])
end

function funky_edge_and_marker_args()
Random.seed!(6)
n = 5
Expand Down
3 changes: 3 additions & 0 deletions test/generate_figures.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,6 @@ savefig("julia_dict_tree.png")

funky_edge_and_marker_args()
savefig("funky_edge_and_marker_args.png")

custom_nodeshapes()
savefig("custom_nodeshapes.png")