improve faceting #46

greimel · 2020-07-07T20:51:09Z

Fixes #42

The case where layouting is done with multiple x and y columns (as in the tutorial) is unchanged.

iris = dataset("datasets", "iris")
cols = style([:SepalLength, :SepalWidth], [:PetalLength :PetalWidth])
grp = style(layout_x = dims(1), layout_y = dims(2), color = :Species)
geom = spec(Scatter) + linear
aog2 = data(iris) * cols * grp * geom
aog2 |> draw

The case where faceting is done with a categorical variable is improved: Note the shared x and y labels and the labels on top and to the right.

mpg = dataset("ggplot2", "mpg")
mpg[!,:grpx] .= rand.(Ref(["a", "b", "c"]))
mpg[!,:grpy] .= rand.(Ref(["aa", "bb"]))

cols = style(:Displ, :Hwy)
grp = style(color = :Cyl => categorical, layout_x = :grpx, layout_y = :grpy)
scat = spec(Scatter)
pipeline = cols * scat
aog = data(mpg) * grp * pipeline
aog |> draw

Since I don't know to internal well enough, this could probably be implemented more elegantly. Let me know what I can do to improve the code.

Edit: Things to improve

Should we also link all the x-axis when faceting by a categorical variable?
Placement of legend is not nice

greimel · 2020-07-08T12:05:55Z

I now use protrusions for spanned x and y labels so that the legend is correctly centered.

I would like to acknowledge help by @jkrumbiegel on Slack. He suggested to use protrusions for the grey labels a while ago and showed me how to use them + clever padding for the spanned labels.

src/makielayout_integration.jl

greimel · 2020-07-08T22:09:57Z

src/makielayout_integration.jl

@@ -92,6 +105,72 @@ function layoutplot!(scene, layout, ts::Algebraic)
            @warn "Automated legend was not possible due to $e"
        end
    end
+
+    ax1 = axs[end,1]


Due to my use of fake data, I didn't realize that I got the spanned x and y labels wrong. This is now fixed.

Now I take them from one ax1.

It would probably be preferable to take them directly from split(trace.value)?

Did you experiment with the scenario where not all pairwise group combinations exist? That is, it's not a perfect grid but some plots are missing. If we want to support that, we'd need to make sure that axs[end, 1] does the right thing there.

piever · 2020-07-10T16:29:56Z

src/makielayout_integration.jl

@@ -40,6 +45,13 @@ function set_defaults!(attrs::Attributes)
    get!(attrs, :markersize, Observable(8px))
 end

+pkeys(aog) = aog.dict.vals[1].context.pkeys


Maybe better to specify the type of aog, as this seems very specific. aog.dict.vals[1] should probably be first(values(aog)). Also, it'd probably be better to introduce pkeys(c::AbstractContext), because the data context has a pkeys field, but other contexts don't, so pkeys(::AbstractContext) = nothing should probably be the general fallback.

Somehow, this feels a bit fragile, because layoutplot! accepts any Algebraic. Could you do it somewhere during the pipeline, where it is more clear what objects one is dealing with? For example, somewhere around here: https://github.com/JuliaPlots/AlgebraOfGraphics.jl/blob/master/src/makielayout_integration.jl#L49

Actually never mind, this can probably stay as is, I'm working on a simplification of the system, so this will have to change in the near future anyways.

piever · 2020-07-10T16:40:42Z

src/makielayout_integration.jl

+            (MakieLayout.protrusionsobservable(ax) for ax in axs[:, 1])...
+        )
+
+        pady = Node(10.0)


Same as above on padx.

piever · 2020-07-10T16:41:33Z

src/makielayout_integration.jl

+            (MakieLayout.protrusionsobservable(ax) for ax in axs[end, :])...
+        )
+
+        padx = Node(10.0)


Do we need this here? Somehow, I imagined that the defaults of MakieLayout should also be our defaults, or is there something MakieLayout is not doing automatically?

@jkrumbiegel, would you mind helping us out? I basically copied your code from slack :-D

Are similar the paddings between regular axis labels and tick labels similarly hard coded in MakieLayout? Or could extract the padding from the given plot and and use it here?

well you could of course just use one of the padding attributes of one of the LAxis instances, it's the same thing but if you have a spanning label, this parameter doesn't belong to one of the spanned axes but really to the group... I just wanted to show in the example how you can correctly position the label, it's your decision how you want to expose that parameter :)

By default, the padding should be identical for all panels, right? (The axis labels are always nicely aligned). That is, I can just take the attribute from any panel.

I'm not sure if I understand what you want to know. Normally, an axis has an x and y label and there is a padding value for those against the tick labels. What you're doing here is not using those labels at all, but simply sticking another label across the outer axes and padding it so it looks good. The extra padding value that you need can come from anywhere, but why would you reuse it from one of the axes? Then the user would have to go to that axis and change the padding value there. I think it makes most sense to make these values keyword arguments of the AoG function that creates the grid. I don't know if the AoG data structure in the end has a space for storing this attribute, though.

Thanks, I think I misunderstood which distance the padding measures.

piever · 2020-07-10T16:45:23Z

src/makielayout_integration.jl

@@ -92,6 +105,72 @@ function layoutplot!(scene, layout, ts::Algebraic)
            @warn "Automated legend was not possible due to $e"
        end
    end
+
+    ax1 = axs[end,1]


Did you experiment with the scenario where not all pairwise group combinations exist? That is, it's not a perfect grid but some plots are missing. If we want to support that, we'd need to make sure that axs[end, 1] does the right thing there.

greimel added 3 commits July 7, 2020 22:26

improve faceting

06af134

use protrusions

b252169

fix spanned x and y label

607cffe

clean up

57a12c3

greimel commented Jul 8, 2020

View reviewed changes

src/makielayout_integration.jl Outdated Show resolved Hide resolved

greimel added 3 commits July 8, 2020 15:32

clean up imports

1fcdf97

Merge remote-tracking branch 'origin/master' into faceting

a5a1f15

now with correct spanned labels

38acaf6

greimel commented Jul 8, 2020

View reviewed changes

piever requested changes Jul 10, 2020

View reviewed changes

greimel mentioned this pull request Jul 13, 2020

Faceting with empty panels is broken #50

Closed

Merge branch 'master' into faceting

6b279e9

piever approved these changes Jul 13, 2020

View reviewed changes

piever merged commit 66d29a3 into MakieOrg:master Jul 13, 2020

greimel deleted the faceting branch July 13, 2020 15:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

improve faceting #46

improve faceting #46

greimel commented Jul 7, 2020 •

edited

Loading

greimel commented Jul 8, 2020

greimel Jul 8, 2020

piever Jul 10, 2020

piever Jul 10, 2020

piever Jul 10, 2020

piever Jul 11, 2020

piever Jul 10, 2020

piever Jul 10, 2020

greimel Jul 13, 2020

jkrumbiegel Jul 13, 2020 •

edited

Loading

greimel Jul 13, 2020

greimel Jul 13, 2020

jkrumbiegel Jul 13, 2020

greimel Jul 13, 2020

piever Jul 10, 2020

improve faceting #46

improve faceting #46

Conversation

greimel commented Jul 7, 2020 • edited Loading

greimel commented Jul 8, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jkrumbiegel Jul 13, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

greimel commented Jul 7, 2020 •

edited

Loading

jkrumbiegel Jul 13, 2020 •

edited

Loading