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

Workaround for the coloring order issue. #166

Closed
wants to merge 1 commit into from

Conversation

ps-pat
Copy link

@ps-pat ps-pat commented Aug 26, 2021

Fix the issues #153 and #163. I don't know why that works but it seems to do. Kinda look like there is some deeper issue there, if anyone have any pointer please let me know!

@JackDevine
Copy link
Member

JackDevine commented Aug 28, 2021

Thanks for taking this on!

Bisection shows that this came up in #146

I was trying to fix an issue where Plots.jl started to treat NaN separated lists differently. I obviously didn't test corner cases enough. If Plots.jl can restore the old behaviour, then we could revert #146 and we would not have this issue. Alternatively, we will have to work out what went wrong in #146

Interesting that shifting the node names by -1 fixes the issue for the graph in #163

I tested with g = ones(9, 9) and it seems that the circshift fix doesn't work.

I think that the trick will be to look at #146 and try to work out why it is producing a list of vectors in the wrong order on line 924.

Basically, the logic of that part of the code is that it is trying to give the plotting recipe data for the perimeters of the nodes. In the past, we used to use NaN to indicate that the plotter should move to the next node. Now we have each node outline as it's own vector in a vector of vectors (we actually have a 2-tuple where the x coordinates are in the first element of the tuple and the y coordinates are in the second).

@ps-pat
Copy link
Author

ps-pat commented Aug 30, 2021

I didn't test my fix enough apparently! The weird thing is that the bug doesn't seem to occur with other attributes like nodeshape for instance. I played around with #146 and the list of vectors produced by your fix is in the same order than the one produced by older code. Maybe this is a Plot.jl issue?

@JackDevine
Copy link
Member

I didn't test my fix enough apparently!

That makes two of us!

Maybe this is a Plot.jl issue?

Interesting theory. The whole motivation for #146 was to deal with the new way that Plots.jl deals with Shapes. If we downgrading Plots.jl fixes the problem, then that would prove your point.

Right now, it is quite difficult to disentangle the graphplot code from the Plots.jl code. I wonder if it is possible to come up with a really simple example where colors and labels are handled differently by Plots.jl?

@BeastyBlacksmith
Copy link
Member

This might relate to JuliaPlots/StatsPlots.jl#442

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.

3 participants