Conversation
mkborregaard
left a comment
mkborregaard
left a comment
There was a problem hiding this comment.
Thanks, this looks nice. I've a few questions.
|
|
||
| # Settings for the axes | ||
| legend --> false | ||
| yflip := true |
There was a problem hiding this comment.
Why do you override settings for these three attributes? Might --> not be better?
There was a problem hiding this comment.
yflip needs to be known in order to get the sorting order right. I'm not sure how to read that attribute if it wasn't set explicitly, and I thought top-to-bottom was most natural.
xlims and yticks should probably be "soft" assignments, I agree. Will modify.
There was a problem hiding this comment.
Actually, I've learned that if I don't assign yticks:=, the labels don't get preserved. I'd really rather show the size of each cluster than the cumulative sizes. Do you know a fix?
There was a problem hiding this comment.
You can get the value by plotattributes[:yticks], I think. You can also read it, and then explicitly assign what you read to it.
There was a problem hiding this comment.
No, if I try to reference plotattributes[:yticks] when it was left unset in the invoking plot command, I get a "key not found" error. I suppose that unset attributes aren't given values until later in the pipeline.
There was a problem hiding this comment.
I've just avoided the issue by making yflip a keyword setting in silhouetteplot. It's true by default, but setting it to false is respected.
| plt = plot([],label="") | ||
| for i in 1:k | ||
| idx = (a.==i) # members of cluster i | ||
| si = sort(s[idx],rev=true) |
There was a problem hiding this comment.
better to use filter here which avoids allocating the temporary idx array
There was a problem hiding this comment.
Sorry, I don't know how that works when the filtered array is not the one on which the selection is predicated. It's a boolean vector, so it's probably not creating a big performance issue.
| si = sort(s[idx],rev=true) | ||
| @series begin | ||
| linealpha --> 0 | ||
| seriestype := :shape |
There was a problem hiding this comment.
why do you use shape? I think just a line with a fillto argument might be simpler (try to run plot(sort(randn(1000)), fillto = 0) to see the effect) and easier?
There was a problem hiding this comment.
Does fillto work horizontally?
|
Can I get an update on the changes requested to this PR? |
|
@tobydriscoll really sorry for the ridiculously late response here. Could you rebase this onto master resolving the conflicts, and add an example to the Readme documenting the use of this function? Then I'll merge. Thanks, and sorry again. |
tobydriscoll
force-pushed
the
silhouette
branch
from
14e6985 to
474165c
Compare
|
Updated the example in documentation string and the README. |
3 participants
This goes to address issue JuliaPlots/Plots.jl#5495.