Support Legends

[greimel]

tbl1 = (x = rand(100), y = rand(100), z = 20 .* rand(100), grp1 = rand(["a", "b"], 100), grp2 = rand(["c", "d"], 100))

cols = mapping(:x => "The X", :y => "The Y");
grp = mapping(marker = :grp1 => nonnumeric => "Group", color = :grp2 => "Other", markersize = :z);
scat = visual(Scatter)
aog = data(tbl1) * cols * scat * grp

fig = aog |> draw
Legend(fig.figure[1,2], aog)
fig

Todo

• combine legends when the same variable is used more than once
• support Colorbar (future PR)
• automatically add legend when calling draw(...) (do you want that @piever?)

[piever]

Thanks a lot, this looks pretty nice already!

I think there is no need to have the legend drawn automatically in draw (it'd be easy to add that later in any case), but it would help to have an example of drawing the legend in the Example Gallery (could probably be the same as the one you're showing above, plus one with a colorbar).

Btw, a couple of practical things to add docs to gallery.jl:

• to try things locally, you need to be on the master branch of documenter (otherwise, it does not handle many svgs in one page well)
• it's probably better to rebase this branch (conflict-free) before adding docs, because I've just modified the whole doc setup

[greimel]

It turned out that I need to do some adjustments for the more general setup here.

I've not pushed this yet, but locally this works

That is, you can mix and match PlotTypes and continous/categorical scales now and use a variable more than once.

[greimel]

What needs to be fixed future PRs

• correctly handle composite plot types (now fall back to poly)
• specifying the order of legend elements (should be poly then line then marker)
• correctly handle the case when no plottype is given in visual (currently this errors)

A few examples

[piever]

I guess these comes from developing in Pluto? I think both these and the begin .. end blocks should be removed.

[piever]

Here and below also the indentation seems a bit off, should be 4 spaces.

[piever]

Indentation.

[piever]

Can this have a more descriptive name? Otherwise the code is a bit difficult to follow with the various Legend, _Legend_ and _legend.

I also suspect that one should reconsider the type hierarchy a bit, in that rather than the Label and KW type, it may be easier to organize things as

struct SubLegendEntry
    label::String
    attributes::Dict{Symbol, Any} # when we "consolidate", it will be more than one pair
end

struct SubLegend
    title::String
    P::PlotFunc
    entries::Vector{SubLegendEntry}
end

So that _legend would just be a sublegend function to return the SubLegend object relative to some attribute.

[piever]

Having a custom SubLegend type would make it cleaner to define the various consolidation approaches. There should probably be a merge!(sublegend1::SubLegend, sublegend2::SubLegend) method that checks that the title is the same and merges the attribute for matching entries.

[piever]

(Thinking more carefully, scratch the above type hierarchy, I try to explain my idea more clearly in #32 (comment).)

[piever]

A minor point, but it seems to me that @unpack is used too little to justify the extra dependency (even though a very light one).

[piever]

Thanks for adding the consolidation bit. I've left some comments. Other than some formatting nitpicking, I think the main point is to have a clean structure for SubLegend and SubLegendEntry that allows for merging. My suggestion is something along the lines of

struct SubLegendEntry
    label::String
    visuals::Vector{Visual} # each plottype has its own attributes
end

struct SubLegend
    title::String
    entries::Vector{SubLegendEntry}
end

We should make sure that this allow for a merge! method on sublegends that combines entries in the correct way.

Then, consolidation becomes much easier using dicts (well, an OrderedCollections.LittleDict to remember the order). For example:

sublegend_dict = LittleDict{String, SubLegend}()
for sublegend in sublegends
    key = sublegend.title
    current = get!(sublegend_dict, key, Sublegend(key, SubLegendEntry[]))
    merge!(current, sublegend)
end

[greimel]

Hi, I'll probably take a few days do address all this. I wanted to push a working version before the weekend.

[piever]

@greimel , just a heads up that I'm about to do some Entries / AxisEntries rework, so that some details of how things are done (in particular legend consolidations) will likely need to change. I was thinking to merge this as is (even if a bit preliminar) and adjust it as I do the refactor. (Otherwise you'd have to work on things that may likely need to change in the near future...)

Do you have some local changes you'd like to push or can I go ahead and merge this version?

[greimel]

hi @piever, sorry that this has stalled a bit. I've not worked on this in the meantime, I was busier than I thought with other things.

Despite the messy state it should be fine to merge. If necessary I will help cleaning this up later.

[greimel]

also feel free to commit the suggested changes.