Skip to content

Commit

Permalink
Remove unnecessary splatting from internal interfaces
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
non-Jedi committed Apr 19, 2019
1 parent 9df68de commit 9c0a252
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 30 deletions.
24 changes: 10 additions & 14 deletions src/Gadfly.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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, (context(order=layer.order), 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
Expand Down
2 changes: 1 addition & 1 deletion src/geom/subplot.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down
6 changes: 3 additions & 3 deletions src/guide.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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
Expand Down
27 changes: 15 additions & 12 deletions src/scale.jl
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,19 @@ 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)
aes.titles = data.titles
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

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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))
Expand Down

0 comments on commit 9c0a252

Please sign in to comment.