TidierOrg / TidierPlots.jl

Tidier data visualization in Julia, modeled after the ggplot2 R package.
MIT License
196 stars 7 forks source link

Boxplot across colors giving ArgumentError in version 0.6.6 #86

Closed cnrrobertson closed 2 months ago

cnrrobertson commented 2 months ago

Recently updated Tidier which moved TidierPlots from 0.5.5 to 0.6.6 and including color in aes for boxplots is no longer working (not sure if it's more general than that - works with geom_point() or geom_line()).

Simple example for demonstration:

ggplot(penguins, aes(x=:species, y=:bill_depth_mm, color=:island)) +
             geom_boxplot()

with error:

Error showing value of type TidierPlots.GGPlot:
ERROR: ArgumentError: Collection must have the same value across all indices
Stacktrace:
  [1] getuniquevalue(v::Vector{ColorTypes.RGBA{Float32}}, idxs::Vector{Int64})
    @ Makie ~/.julia/packages/Makie/ND0gA/src/stats/violin.jl:48
  [2] (::Makie.var"#1124#1138")(x::Vector{…}, y::Vector{…}, color::Vector{…}, weights::MakieCore.Automatic, width::MakieCore.Automatic, range::Float64, show_outliers::Bool, whiskerwidth::Float64, show_notch::Bool, orientation::Symbol, gap::Float64, dodge::MakieCore.Automatic, n_dodge::MakieCore.Automatic, dodge_gap::Float64)
    @ Makie ~/.julia/packages/Makie/ND0gA/src/stats/boxplot.jl:142
  [3] map(::Makie.var"#1124#1138", ::Union{…}, ::Observables.Observable{…}, ::Observables.Observable{…}, ::Vararg{…}; ignore_equal_values::Bool, priority::Int64)
    @ Makie ~/.julia/packages/Makie/ND0gA/src/scenes.jl:160
  [4] map
    @ ~/.julia/packages/Makie/ND0gA/src/scenes.jl:157 [inlined]
  [5] plot!(plot::MakieCore.Plot{Makie.boxplot, Tuple{Vector{Int64}, Vector{Float64}}})
    @ Makie ~/.julia/packages/Makie/ND0gA/src/stats/boxplot.jl:85
  [6] connect_plot!(parent::Makie.Scene, plot::MakieCore.Plot{Makie.boxplot, Tuple{Vector{Int64}, Vector{Float64}}})
    @ Makie ~/.julia/packages/Makie/ND0gA/src/interfaces.jl:260
  [7] plot!(scene::Makie.Scene, plot::MakieCore.Plot{Makie.boxplot, Tuple{Vector{Int64}, Vector{Float64}}})
    @ Makie ~/.julia/packages/Makie/ND0gA/src/interfaces.jl:265
  [8] diff_plotlist!(scene::Makie.Scene, plotspecs::Vector{…}, obs_to_notify::Vector{…}, reusable_plots::IdDict{…}, plotlist::Nothing)
    @ Makie ~/.julia/packages/Makie/ND0gA/src/specapi.jl:428
  [9] (::Makie.var"#update_plotlist#1043"{Makie.Scene, Nothing, Vector{Observables.Observable}, IdDict{Makie.PlotSpec, MakieCore.Plot}})(plotspecs::Vector{Makie.PlotSpec})
    @ Makie ~/.julia/packages/Makie/ND0gA/src/specapi.jl:459
 [10] #1042
    @ ~/.julia/packages/Makie/ND0gA/src/specapi.jl:479 [inlined]
 [11] lock(f::Makie.var"#1042#1047"{Vector{Makie.PlotSpec}, Makie.var"#update_plotlist#1043"{Makie.Scene, Nothing, Vector{…}, IdDict{…}}}, l::ReentrantLock)
    @ Base ./lock.jl:229
 [12] (::Makie.var"#1041#1046"{ReentrantLock, Makie.var"#update_plotlist#1043"{Makie.Scene, Nothing, Vector{…}, IdDict{…}}})(plotspecs::Vector{Makie.PlotSpec})
    @ Makie ~/.julia/packages/Makie/ND0gA/src/specapi.jl:478
 [13] on(f::Any, observable::Observables.AbstractObservable; weak::Bool, priority::Int64, update::Bool)
    @ Observables ~/.julia/packages/Observables/YdEbO/src/Observables.jl:368
 [14] on
    @ ~/.julia/packages/Observables/YdEbO/src/Observables.jl:361 [inlined]
 [15] #on#177
    @ ~/.julia/packages/Makie/ND0gA/src/scenes.jl:135 [inlined]
 [16] on
    @ ~/.julia/packages/Makie/ND0gA/src/scenes.jl:134 [inlined]
 [17] update_plotspecs!(scene::Makie.Scene, list_of_plotspecs::Observables.Observable{Vector{Makie.PlotSpec}}, plotlist::Nothing)
    @ Makie ~/.julia/packages/Makie/ND0gA/src/specapi.jl:477
 [18] update_plotspecs!
    @ ~/.julia/packages/Makie/ND0gA/src/specapi.jl:449 [inlined]
 [19] update_gridlayout!(gridlayout::GridLayoutBase.GridLayout, nesting::Int64, oldgridspec::Nothing, gridspec::Makie.GridLayoutSpec, previous_contents::Vector{…}, new_layoutables::Vector{…})
    @ Makie ~/.julia/packages/Makie/ND0gA/src/specapi.jl:636
 [20] (::Makie.var"#1049#1051"{Makie.GridLayoutSpec, GridLayoutBase.GridLayout, Vector{Pair{Tuple{…}, Tuple{…}}}, Vector{Pair{Tuple{…}, Tuple{…}}}})()
    @ Makie ~/.julia/packages/Makie/ND0gA/src/specapi.jl:698
 [21] lock(f::Makie.var"#1049#1051"{Makie.GridLayoutSpec, GridLayoutBase.GridLayout, Vector{Pair{Tuple{…}, Tuple{…}}}, Vector{Pair{Tuple{…}, Tuple{…}}}}, l::ReentrantLock)
    @ Base ./lock.jl:229
 [22] (::Makie.var"#1048#1050"{GridLayoutBase.GridLayout, ReentrantLock, Vector{Pair{Tuple{…}, Tuple{…}}}, Vector{Pair{Tuple{…}, Tuple{…}}}})(layout_spec::Makie.GridLayoutSpec)
    @ Makie ~/.julia/packages/Makie/ND0gA/src/specapi.jl:693
 [23] on(f::Any, observable::Observables.AbstractObservable; weak::Bool, priority::Int64, update::Bool)
    @ Observables ~/.julia/packages/Observables/YdEbO/src/Observables.jl:368
 [24] on
    @ ~/.julia/packages/Observables/YdEbO/src/Observables.jl:361 [inlined]
 [25] #on#177
    @ ~/.julia/packages/Makie/ND0gA/src/scenes.jl:135 [inlined]
 [26] on
    @ ~/.julia/packages/Makie/ND0gA/src/scenes.jl:134 [inlined]
 [27] update_fig!(fig::Makie.Figure, layout_obs::Observables.Observable{Makie.GridLayoutSpec})
    @ Makie ~/.julia/packages/Makie/ND0gA/src/specapi.jl:692
 [28] plot!(fig::Makie.Figure, plot::MakieCore.Plot{MakieCore.plot, Tuple{Makie.GridLayoutSpec}})
    @ Makie ~/.julia/packages/Makie/ND0gA/src/specapi.jl:735
 [29] _create_plot(F::Function, attributes::Dict{Symbol, Any}, args::Makie.GridLayoutSpec)
    @ Makie ~/.julia/packages/Makie/ND0gA/src/figureplotting.jl:250
 [30] plot
    @ ~/.julia/packages/MakieCore/UAwps/src/recipes.jl:39 [inlined]
 [31] draw_ggplot(plot::TidierPlots.GGPlot)
    @ TidierPlots ~/.julia/packages/TidierPlots/CA1NI/src/draw.jl:151
 [32] (::TidierPlots.var"#83#84"{TidierPlots.GGPlot})()
    @ TidierPlots ~/.julia/packages/TidierPlots/CA1NI/src/show.jl:76
 [33] (::Makie.var"#279#280"{@Kwargs{}, TidierPlots.var"#83#84"{TidierPlots.GGPlot}, MakieCore.Attributes})()
    @ Makie ~/.julia/packages/Makie/ND0gA/src/theming.jl:227
 [34] lock(f::Makie.var"#279#280"{@Kwargs{}, TidierPlots.var"#83#84"{TidierPlots.GGPlot}, MakieCore.Attributes}, l::ReentrantLock)
    @ Base ./lock.jl:229
 [35] #with_theme#278
    @ ~/.julia/packages/Makie/ND0gA/src/theming.jl:223 [inlined]
 [36] with_theme
    @ ~/.julia/packages/Makie/ND0gA/src/theming.jl:222 [inlined]
 [37] show(io::IOContext{Base.TTY}, plot::TidierPlots.GGPlot)
    @ TidierPlots ~/.julia/packages/TidierPlots/CA1NI/src/show.jl:75
 [38] show(io::IOContext{Base.TTY}, ::MIME{Symbol("text/plain")}, x::TidierPlots.GGPlot)
    @ Base.Multimedia ./multimedia.jl:47
adknudson commented 2 months ago

I'll have to dig into this some more, but a temporary fix is to do the following:

ggplot(penguins, aes(x=:species, y=:bill_depth_mm, color=:island, dodge=:island)) +
    geom_boxplot()

However this causes the x-axis labels to be shifted out of place.

It seems like Makie is expecting both dodge and color to have the same grouping variable. There is a cryptic note on the Makie boxplot docs

For all indices corresponding to points within the same box, color (but not outliercolor) must have the same value.

On the TidierPlots side, we may need to automatically add the dodge aesthetic whenever color is given.

kdpsingh commented 2 months ago

@cnrrobertson, just as general knowledge, some things broke in TidierPlots 0.6.0 because we removed the dependency on AlgebraOfGraphics, so the package now directly translates its functions into Makie output. The fact that this would break some things was known, so @rdboyes has been working to unbreak things and implement some things that were missing due to the AoG dependency.

Appreciate you sharing examples of things that aren't working so we can review and fix.

rdboyes commented 2 months ago

@adknudson thanks for suggesting the workaround - this is a more specific example of a general problem I've been trying to figure out the best way to address. We need some way to encode the fact that some aes options have "grouping" attached to them and others don't (see also unexpected behaviour in more complex geom_smooth plots)

On this example, note that color does work in geom_boxplot if the x and color aesthetics are the same, e.g.

ggplot(penguins, aes(x=:species, y=:bill_depth_mm, color=:species)) +
    geom_boxplot()

image

I solved the dodge/stack ambiguity for geom_bar "manually" with the handle_position function, but I don't love the solution - I will add this to the 0.8.0 checklist

rdboyes commented 2 months ago

In dev:

ggplot(penguins, aes(x=:species, y=:bill_depth_mm, color=:island)) +
                    geom_boxplot() + lims(x = (.5, 3.5))

image

ggplot(penguins, aes(x=:bill_depth_mm, y = :species, color=:island)) +
                    geom_boxplot() + lims(y = (.5, 3.5))

image