JuliaPlots / GraphRecipes.jl

Graph-related recipes to be used with Plots.jl
Other
168 stars 28 forks source link

Custom Node Shapes #136

Open enuit opened 4 years ago

enuit commented 4 years ago

It would be great to have custom node shapes. I found that the current nodes are basically of type Tuple{Float64, Float64}. So I thought this could be easily achieved by adding a new type

struct CustomShape
    shape::Array{Tuple{Float64, Float64}, 1}
end

and add the last two lines of

            elseif nodeshape[node_number] == :ellipse
                nodeheight = (yextent[2] - yextent[1])
                push!(node_vec_vec_xy, partialellipse(0, 2π, [x[i], y[i]],
                                                      80, nodewidth/2, nodeheight/2))
            elseif typeof(nodeshape[node_number]) == CustomShape        # This line
                push!(node_vec_vec_xy, nodeshape[node_number].shape)   #This  line

here https://github.com/JuliaPlots/GraphRecipes.jl/blob/7210d3615719c6f09f457c924efff72a75759e03/src/graphs.jl#L489.

However, this doesn't work. If I try to use it, a Error

ERROR: MethodError: no method matching plot(::GraphRecipes.GraphPlot; node_shape=GraphRecipes.CustomShape([(0.0, 0.0), (0.0, 1.0), (1.0, 1.0), (1.0, 0.0)]))
Stacktrace:
 [1] graphplot(::Array{Int64,2}; kw::Base.Iterators.Pairs{Symbol,GraphRecipes.CustomShape,Tuple{Symbol},NamedTuple{(:node_shape,),Tuple{GraphRecipes.CustomShape}}}) at /home/wissmann/.julia/packages/RecipesBase/aQmWx/src/RecipesBase.jl:356
 [2] top-level scope at REPL[7]:1

is raised. I don't understand this error since I don't see any type restriction on node_shape. Is this a good way of solving the problem? Can you help me in understanding why the rises?

JackDevine commented 4 years ago

I agree that this would be a major improvement to the current node shape infrastructure, thanks for sharing your thoughts!

That said, I don't understand the specific implementation of your proposal. The current design is that x[i], y[i], nodewidth and nodeheight are passed to a function that generates the perimeter of the node. It seems to me that your proposal, nodeshape[node_number].shape already contains all of the above information. Does that means that you have a loop elsewhere that creates an array of custom shapes that are positioned and sized correctly?

Perhaps a different way would be that the user passes a function that accepts the coordinates+height+width of a node and returns the outline of the node as an array of tuples. There are a lot of legitimate ways to solve this problem, so we may have to consider a few different ideas before we land on the best one.

Feel free to make a pull request with any work that you have done so far, since it makes it a lot easier to understand/review your thinking if I can see all of the code. If you aren't able to get the code to run, then just add a WIP label at the start of the pull request title, that way I will know not to merge.

I can tell you right now that I like your idea at a high level, so if you make a pull request, you wont be at risk of it being rejected entirely. Once you have the pull request open, feel free to tag me and I will try to give you some pointers on how to get things working.

enuit commented 4 years ago

Thank you for your input and the help. I think passing a function is a much better idea then passing the shape directly. In my approach, the scaled shape would have to be created by the user. So in the above Pull request I changed my example to work with a function taking x[i],y[i] and nodewidth, nodeheight as input parameters.

Alas, I can't get it to work. Somehow, my custom nodeshape seems to be forwarded to the markershape of Plots.jl which passes it to GR. Are nodes plotted as markers? If so, I can't find the position in the code where they are passed to Plots.jl. Plots.jl wants something of the Type Plots.Shape for custom markers, right?. In this case, I don't understand how you handle that internally as your shapes seem to be of Type Array{Tuple{Float64, Float64}, 1}. If you could give me a hint on how a node is plotted and where that happens, maybe I can figure out the rest by myself.