From aec556d7e093b68c800385941655e69d47cdb88a Mon Sep 17 00:00:00 2001 From: Adam Beckmeyer Date: Wed, 17 Apr 2019 22:42:00 -0400 Subject: [PATCH] Remove unnecessary splatting from internal interfaces This gives a marginal improvement on speed and memory use during first plot, but the biggest benefit is that it makes the code easier to read and the profiling significantly easier to grok. --- src/Gadfly.jl | 24 ++++++++++-------------- src/geom/subplot.jl | 2 +- src/guide.jl | 6 +++--- src/scale.jl | 27 +++++++++++++++------------ 4 files changed, 29 insertions(+), 30 deletions(-) diff --git a/src/Gadfly.jl b/src/Gadfly.jl index b517bc2af..6cba5aa73 100755 --- a/src/Gadfly.jl +++ b/src/Gadfly.jl @@ -653,7 +653,7 @@ function render_prepare(plot::Plot) # I. Scales layer_aess = Scale.apply_scales(IterTools.distinct(values(scales)), - datas..., subplot_datas...) + vcat(datas, subplot_datas)) # set defaults for key titles keyvars = [:color, :shape] @@ -809,28 +809,24 @@ function render_prepared(plot::Plot, # IV. Geometries themes = Theme[layer.theme === nothing ? plot.theme : layer.theme for layer in plot.layers] - zips = trim_zip(plot.layers, layer_aess, - layer_subplot_aess, - layer_subplot_datas, - themes) - - compose!(plot_context, - [compose(context(order=layer.order), render(layer.geom, theme, aes, - subplot_aes, subplot_data, - scales)) - for (layer, aes, subplot_aes, subplot_data, theme) in zips]...) + zips = trim_zip(plot.layers, layer_aess, layer_subplot_aess, + layer_subplot_datas, themes) + for (layer, aes, subplot_aes, subplot_data, theme) in zips + r = render(layer.geom, theme, aes, subplot_aes, subplot_data, scales) + compose!(plot_context, r) + end#for # V. Guides - guide_contexts = Any[] + guide_contexts = Guide.PositionedGuide[] for guide in guides guide_context = render(guide, plot.theme, plot_aes, dynamic) - if guide_context != nothing + if !isnothing(guide_context) append!(guide_contexts, guide_context) end end tbl = Guide.layout_guides(plot_context, coord, - plot.theme, guide_contexts...) + plot.theme, guide_contexts) if table_only return tbl end diff --git a/src/geom/subplot.jl b/src/geom/subplot.jl index 08bc4b113..b824dd1fc 100644 --- a/src/geom/subplot.jl +++ b/src/geom/subplot.jl @@ -181,7 +181,7 @@ function render(geom::SubplotGrid, theme::Gadfly.Theme, Gadfly.Aesthetics[layer_aes_grid[k][i, j] for k in 1:length(geom.layers)], Gadfly.Data[layer_data_grid[k][i, j] - for k in 1:length(geom.layers)]...) + for k in 1:length(geom.layers)]) for (k, stats) in enumerate(layer_stats) Stat.apply_statistics(stats, scales, coord, layer_aes_grid[k][i, j]) diff --git a/src/guide.jl b/src/guide.jl index 5e13b7c4c..4aecb4752 100644 --- a/src/guide.jl +++ b/src/guide.jl @@ -125,10 +125,10 @@ end # A guide graphic is a position associated with one or more contexts. # Multiple contexts represent multiple layout possibilites that will be # optimized over. -struct PositionedGuide +struct PositionedGuide{P<:GuidePosition} ctxs::Vector{Context} order::Int - position::GuidePosition + position::P end @@ -1129,7 +1129,7 @@ end function layout_guides(plot_context::Context, coord::Gadfly.CoordinateElement, theme::Gadfly.Theme, - positioned_guides::PositionedGuide...) + positioned_guides::Vector{PositionedGuide}) # Organize guides by position guides = DefaultDict(() -> (Tuple{Vector{Context}, Int})[]) for positioned_guide in positioned_guides diff --git a/src/scale.jl b/src/scale.jl index 183dc12f8..f7295abbd 100644 --- a/src/scale.jl +++ b/src/scale.jl @@ -18,9 +18,9 @@ include("color_misc.jl") iscategorical(scales::Dict{Symbol, Gadfly.ScaleElement}, var::Symbol) = haskey(scales, var) && isa(scales[var], DiscreteScale) -function apply_scales(scales, aess::Vector{Gadfly.Aesthetics}, datas::Gadfly.Data...) +function apply_scales(scales, aess::Vector{Gadfly.Aesthetics}, datas::Vector{Gadfly.Data}) for scale in scales - apply_scale(scale, aess, datas...) + apply_scale(scale, aess, datas) end for (aes, data) in zip(aess, datas) @@ -28,9 +28,9 @@ function apply_scales(scales, aess::Vector{Gadfly.Aesthetics}, datas::Gadfly.Dat end end -function apply_scales(scales, datas::Gadfly.Data...) +function apply_scales(scales, datas::Vector{Gadfly.Data}) aess = Gadfly.Aesthetics[Gadfly.Aesthetics() for _ in datas] - apply_scales(scales, aess, datas...) + apply_scales(scales, aess, datas) aess end @@ -177,9 +177,12 @@ alpha_continuous(; minvalue=0.0, maxvalue=1.0, labels=nothing, format=nothing, m ContinuousScale([:alpha], identity_transform, minvalue=minvalue, maxvalue=maxvalue, labels=labels, format=format, minticks=minticks, maxticks=maxticks, scalable=scalable) +# Need to wrap Gadfly.Data in array for apply_scale methods +apply_scale(scale::Gadfly.ScaleElement, aess::Vector{Gadfly.Aesthetics}, data::Gadfly.Data) = + apply_scale(scale, aess, Gadfly.Data[data]) function apply_scale(scale::ContinuousScale, - aess::Vector{Gadfly.Aesthetics}, datas::Gadfly.Data...) + aess::Vector{Gadfly.Aesthetics}, datas::Vector{Gadfly.Data}) for (aes, data) in zip(aess, datas) for var in scale.vars vals = getfield(data, var) @@ -362,7 +365,7 @@ alpha_discrete(; labels=nothing, levels=nothing, order=nothing) = DiscreteScale([:linestyle], labels=labels, levels=levels, order=order) -function apply_scale(scale::DiscreteScale, aess::Vector{Gadfly.Aesthetics}, datas::Gadfly.Data...) +function apply_scale(scale::DiscreteScale, aess::Vector{Gadfly.Aesthetics}, datas::Vector{Gadfly.Data}) for (aes, data) in zip(aess, datas) for var in scale.vars label_var = Symbol(var, "_label") @@ -411,7 +414,7 @@ const color_none = NoneColorScale element_aesthetics(scale::NoneColorScale) = [:color] function apply_scale(scale::NoneColorScale, - aess::Vector{Gadfly.Aesthetics}, datas::Gadfly.Data...) + aess::Vector{Gadfly.Aesthetics}, datas::Vector{Gadfly.Data}) for aes in aess aes.color = nothing end @@ -429,7 +432,7 @@ const color_identity = IdentityColorScale element_aesthetics(scale::IdentityColorScale) = [:color] function apply_scale(scale::IdentityColorScale, - aess::Vector{Gadfly.Aesthetics}, datas::Gadfly.Data...) + aess::Vector{Gadfly.Aesthetics}, datas::Vector{Gadfly.Data}) for (aes, data) in zip(aess, datas) data.color === nothing && continue aes.color = discretize_make_ia(data.color) @@ -512,7 +515,7 @@ end @deprecate discrete_color_manual(colors...; levels=nothing, order=nothing) color_discrete_manual(colors...; levels=levels, order=order) function apply_scale(scale::DiscreteColorScale, - aess::Vector{Gadfly.Aesthetics}, datas::Gadfly.Data...) + aess::Vector{Gadfly.Aesthetics}, datas::Vector{Gadfly.Data}) levelset = OrderedSet() for (aes, data) in zip(aess, datas) data.color === nothing && continue @@ -620,7 +623,7 @@ const color_continuous_gradient = color_continuous ### WHY HAVE THIS ALIAS? @deprecate continuous_color(;minvalue=nothing, maxvalue=nothing) color_continuous(;minvalue=nothing, maxvalue=nothing) function apply_scale(scale::ContinuousColorScale, - aess::Vector{Gadfly.Aesthetics}, datas::Gadfly.Data...) + aess::Vector{Gadfly.Aesthetics}, datas::Vector{Gadfly.Data}) cdata = skipmissing(Iterators.flatten(i.color for i in datas if i.color != nothing)) if !isempty(cdata) cmin, cmax = extrema(cdata) @@ -692,7 +695,7 @@ struct LabelScale <: Gadfly.ScaleElement end function apply_scale(scale::LabelScale, - aess::Vector{Gadfly.Aesthetics}, datas::Gadfly.Data...) + aess::Vector{Gadfly.Aesthetics}, datas::Vector{Gadfly.Data}) for (aes, data) in zip(aess, datas) data.label === nothing && continue aes.label = discretize(data.label) @@ -732,7 +735,7 @@ end element_aesthetics(scale::IdentityScale) = [scale.var] function apply_scale(scale::IdentityScale, - aess::Vector{Gadfly.Aesthetics}, datas::Gadfly.Data...) + aess::Vector{Gadfly.Aesthetics}, datas::Vector{Gadfly.Data}) for (aes, data) in zip(aess, datas) getfield(data, scale.var) === nothing && continue setfield!(aes, scale.var, getfield(data, scale.var))