Skip to content

Commit

Permalink
Compare nothing by identity rather than by value
Browse files Browse the repository at this point in the history
Comparing nothing by value (==, =!) rather than by identity (===, !==)
can have negative consequences on code speed. This commit changes all
nothing comparisons to use the isnothing and issomething utilities
defined in misc.jl for consistency.
  • Loading branch information
non-Jedi committed Apr 19, 2019
1 parent a539497 commit da4a328
Show file tree
Hide file tree
Showing 23 changed files with 240 additions and 246 deletions.
37 changes: 20 additions & 17 deletions src/Gadfly.jl
Original file line number Diff line number Diff line change
Expand Up @@ -388,14 +388,14 @@ function render_prepare(plot::Plot)
# they are missing.
datas = Array{Data}(undef, length(plot.layers))
for (i, layer) in enumerate(plot.layers)
if layer.data_source === nothing && isempty(layer.mapping)
if isnothing(layer.data_source) && isempty(layer.mapping)
layer.data_source = plot.data_source
layer.mapping = plot.mapping
datas[i] = plot.data
else
datas[i] = Data()

if layer.data_source === nothing
if isnothing(layer.data_source)
layer.data_source = plot.data_source
end

Expand All @@ -416,7 +416,7 @@ function render_prepare(plot::Plot)
if isa(layer.geom, Geom.SubplotGeometry)
for subplot_layer in layers(layer.geom)
subplot_data = Data()
if subplot_layer.data_source === nothing
if isnothing(subplot_layer.data_source)
subplot_layer.data_source = layer.data_source
end

Expand All @@ -434,7 +434,7 @@ function render_prepare(plot::Plot)
coord = plot.coord
for layer in plot.layers
coord_type = element_coordinate_type(layer.geom)
if coord === nothing
if isnothing(coord)
coord = coord_type()
elseif typeof(coord) != coord_type
error("Plot uses multiple coordinates: $(typeof(coord)) and $(coord_type)")
Expand Down Expand Up @@ -503,7 +503,7 @@ function render_prepare(plot::Plot)

unscaled_aesthetics = setdiff(used_aesthetics, scaled_aesthetics)

_theme(plt, lyr) = lyr.theme == nothing ? plt.theme : lyr.theme
_theme(plt, lyr) = isnothing(lyr.theme) ? plt.theme : lyr.theme

# Add default scales for statistics.
layer_stats_with_theme = map(plot.layers, layer_stats) do l, stats
Expand Down Expand Up @@ -532,17 +532,17 @@ function render_prepare(plot::Plot)
in(var, mapped_aesthetics) || continue

var_data = getfield(plot.data, var)
if var_data == nothing
if isnothing(var_data)
for data in datas
var_layer_data = getfield(data, var)
if var_layer_data != nothing
if issomething(var_layer_data)
var_data = var_layer_data
break
end
end
end

var_data == nothing && continue
isnothing(var_data) && continue

t = classify_data(var_data)
if scale_exists(t, var)
Expand All @@ -560,7 +560,7 @@ function render_prepare(plot::Plot)
t = :categorical
for data in Iterators.flatten((datas, subplot_datas))
val = getfield(data, var)
if val != nothing && val != :categorical
if issomething(val) && val != :categorical
t = classify_data(val)
end
end
Expand Down Expand Up @@ -659,24 +659,27 @@ function render_prepare(plot::Plot)
keyvars = [:color, :shape]
for (i, layer) in enumerate(plot.layers)
for kv in keyvars
fflag = (getfield(layer_aess[i], Symbol(kv,"_key_title")) == nothing) && haskey(layer.mapping, kv) && !isa(layer.mapping[kv], AbstractArray)
fflag = (isnothing(getfield(layer_aess[i], Symbol(kv,"_key_title")))) &&
haskey(layer.mapping, kv) &&
!isa(layer.mapping[kv], AbstractArray)
fflag && setfield!(layer_aess[i], Symbol(kv,"_key_title"), string(layer.mapping[kv]))
end
end

for kv in keyvars
fflag = (getfield(layer_aess[1], Symbol(kv,"_key_title")) == nothing) && haskey(plot.mapping, kv) && !isa(plot.mapping[kv], AbstractArray)
fflag = (isnothing(getfield(layer_aess[1], Symbol(kv,"_key_title")))) &&
haskey(plot.mapping, kv) && !isa(plot.mapping[kv], AbstractArray)
fflag && setfield!(layer_aess[1], Symbol(kv,"_key_title"), string(plot.mapping[kv]))
end

# Auto-update color scale if shape==color
catdatas = vcat(datas, subplot_datas)
shapev = getfield.(catdatas, :shape)
di = (shapev.!=nothing) .& (shapev.== getfield.(catdatas, :color))
di = issomething.(shapev) .& (shapev.== getfield.(catdatas, :color))

supress_colorkey = false
for (aes, data) in zip(layer_aess[di], catdatas[di])
aes.shape_key_title==nothing && (aes.shape_key_title=aes.color_key_title="Shape")
isnothing(aes.shape_key_title) && (aes.shape_key_title=aes.color_key_title="Shape")
colorf = scales[:color].f
scales[:color] = Scale.color_discrete(colorf, levels=scales[:shape].levels, order=scales[:shape].order)
Scale.apply_scale(scales[:color], [aes], Gadfly.Data(color=getfield(data,:color)) )
Expand Down Expand Up @@ -713,7 +716,7 @@ function render_prepare(plot::Plot)

if !supress_keys
for (KT, kv) in zip(keytypes, keyvars)
fflag = !all([getfield(aes, kv)==nothing for aes in [plot_aes, layer_aess...]])
fflag = !all([isnothing(getfield(aes, kv)) for aes in [plot_aes, layer_aess...]])
fflag && !in(KT, explicit_guide_types) && push!(guides, KT())
end
end
Expand Down Expand Up @@ -756,7 +759,7 @@ function render(plot::Plot)

ctx = pad_inner(root_context, plot.theme.plot_padding...)

if plot.theme.background_color != nothing
if issomething(plot.theme.background_color)
compose!(ctx, (context(order=-1000000),
fill(plot.theme.background_color),
stroke(nothing), rectangle()))
Expand Down Expand Up @@ -807,7 +810,7 @@ function render_prepared(plot::Plot,
layer_aess), scales)

# IV. Geometries
themes = Theme[layer.theme === nothing ? plot.theme : layer.theme
themes = Theme[isnothing(layer.theme) ? plot.theme : layer.theme
for layer in plot.layers]
zips = trim_zip(plot.layers, layer_aess, layer_subplot_aess,
layer_subplot_datas, themes)
Expand All @@ -820,7 +823,7 @@ function render_prepared(plot::Plot,
guide_contexts = Guide.PositionedGuide[]
for guide in guides
guide_context = render(guide, plot.theme, plot_aes, dynamic)
if !isnothing(guide_context)
if issomething(guide_context)
append!(guide_contexts, guide_context)
end
end
Expand Down
39 changes: 14 additions & 25 deletions src/aesthetics.jl
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ function show(io::IO, data::Aesthetics)
print(io, "Aesthetics(")
for name in fieldnames(Aesthetics)
val = getfield(data, name)
if !ismissing(val) && val != nothing
if !ismissing(val) && issomething(val)
print(io, "\n ", string(name), "=")
show(io, getfield(data, name))
end
Expand Down Expand Up @@ -137,7 +137,7 @@ getindex(aes::Aesthetics, i::Integer, j::AbstractString) = getfield(aes, Symbol(
function defined_aesthetics(aes::Aesthetics)
vars = Set{Symbol}()
for name in fieldnames(Aesthetics)
getfield(aes, name) === nothing || push!(vars, name)
isnothing(getfield(aes, name)) || push!(vars, name)
end
vars
end
Expand Down Expand Up @@ -169,7 +169,7 @@ function assert_aesthetics_undefined(who::AbstractString, aes::Aesthetics, vars:
end

function assert_aesthetics_equal_length(who::AbstractString, aes::Aesthetics, vars::Symbol...)
defined_vars = Compat.Iterators.filter(var -> !(getfield(aes, var) === nothing), vars)
defined_vars = Compat.Iterators.filter(var -> issomething(getfield(aes, var)), vars)

if !isempty(defined_vars)
n = length(getfield(aes, first(defined_vars)))
Expand Down Expand Up @@ -227,18 +227,9 @@ function concat(aess)
cataes = Aesthetics()
for aes in aess
for var in fieldnames(Aesthetics)
if var in [:xviewmin, :yviewmin]
if var in [:xviewmin, :yviewmin, :xviewmax, :yviewmax]
mu, mv = getfield(cataes, var), getfield(aes, var)
setfield!(cataes, var,
mu === nothing ? mv :
mv == nothing ? mu :
min(mu, mv))
elseif var in [:xviewmax, :yviewmax]
mu, mv = getfield(cataes, var), getfield(aes, var)
setfield!(cataes, var,
mu === nothing ? mv :
mv == nothing ? mu :
max(mu, mv))
setfield!(cataes, var, isnothing(mu) ? mv : isnothing(mv) ? mu : min(mu, mv))
else
setfield!(cataes, var,
cat_aes_var!(getfield(cataes, var), getfield(aes, var)))
Expand Down Expand Up @@ -310,21 +301,21 @@ end
#
function by_xy_group(aes::T, xgroup, ygroup,
num_xgroups, num_ygroups) where T <: Union{Data, Aesthetics}
@assert xgroup === nothing || ygroup === nothing || length(xgroup) == length(ygroup)
@assert isnothing(xgroup) || isnothing(ygroup) || length(xgroup) == length(ygroup)

n = num_ygroups
m = num_xgroups

xrefs = xgroup === nothing ? [1] : xgroup
yrefs = ygroup === nothing ? [1] : ygroup
xrefs = isnothing(xgroup) ? [1] : xgroup
yrefs = isnothing(ygroup) ? [1] : ygroup

aes_grid = Array{T}(undef, n, m)
staging = Array{AbstractArray}(undef, n, m)
for i in 1:n, j in 1:m
aes_grid[i, j] = T()
end

xgroup === nothing && ygroup === nothing && return aes_grid
isnothing(xgroup) && isnothing(ygroup) && return aes_grid

function make_pooled_array(::Type{IndirectArray{T,N,A,V}}, arr::AbstractArray) where {T,N,A,V}
uarr = unique(arr)
Expand All @@ -347,8 +338,8 @@ function by_xy_group(aes::T, xgroup, ygroup,

vals = getfield(aes, var)
if typeof(vals) <: AbstractArray
if xgroup !== nothing && length(vals) !== length(xgroup) ||
ygroup !== nothing && length(vals) !== length(ygroup)
if issomething(xgroup) && length(vals) !== length(xgroup) ||
issomething(ygroup) && length(vals) !== length(ygroup)
continue
end

Expand Down Expand Up @@ -402,12 +393,10 @@ function inherit!(a::Aesthetics, b::Aesthetics;
bval = getfield(b, field)
if field in clobber_set
setfield!(a, field, bval)
elseif aval === missing || aval === nothing || aval === string || aval == showoff
elseif aval === missing || isnothing(aval) || aval === string || aval == showoff
setfield!(a, field, bval)
elseif field == :xviewmin || field == :yviewmin
bval != nothing && (aval == nothing || aval > bval) && setfield!(a, field, bval)
elseif field == :xviewmax || field == :yviewmax
bval != nothing && (aval == nothing || aval < bval) && setfield!(a, field, bval)
elseif field in (:xviewmin, :yviewmin, :xviewmax, :yviewmax)
issomething(bval) && (isnothing(aval) || aval > bval) && setfield!(a, field, bval)
elseif typeof(aval) <: Dict && typeof(bval) <: Dict
merge!(aval, getfield(b, field))
end
Expand Down
48 changes: 24 additions & 24 deletions src/coord.jl
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ function first_concrete_aesthetic_value(aess::Vector{Gadfly.Aesthetics}, vars::V
for var in vars
for aes in aess
vals = getfield(aes, var)
vals === nothing && continue
Gadfly.isnothing(vals) && continue

if !isa(vals, AbstractArray)
vals = [vals]
Expand Down Expand Up @@ -106,7 +106,7 @@ function aesthetics_type(aess::Vector{Gadfly.Aesthetics},
for var in vars
for aes in aess
vals = getfield(aes, var)
vals === nothing && continue
Gadfly.isnothing(vals) && continue

if var == :outliers
if !isempty(vals)
Expand Down Expand Up @@ -153,11 +153,11 @@ function apply_coordinate(coord::Cartesian, aess::Vector{Gadfly.Aesthetics},

xmin = xmax = first_concrete_aesthetic_value(aess, coord.xvars)

if xmin != nothing
if Gadfly.issomething(xmin)
for var in coord.xvars
for aes in aess
vals = getfield(aes, var)
vals === nothing && continue
Gadfly.isnothing(vals) && continue

if !isa(vals, AbstractArray)
vals = [vals]
Expand All @@ -169,11 +169,11 @@ function apply_coordinate(coord::Cartesian, aess::Vector{Gadfly.Aesthetics},
end

ymin = ymax = first_concrete_aesthetic_value(aess, coord.yvars)
if ymin != nothing
if Gadfly.issomething(ymin)
for var in coord.yvars
for aes in aess
vals = getfield(aes, var)
vals === nothing && continue
Gadfly.isnothing(vals) && continue

# Outliers is an odd aesthetic that needs special treatment.
if var == :outliers
Expand All @@ -196,40 +196,40 @@ function apply_coordinate(coord::Cartesian, aess::Vector{Gadfly.Aesthetics},

# viewmin/max that is set explicitly should override min/max
for aes in aess
if aes.xviewmin != nothing
xviewmin = xviewmin === nothing ? aes.xviewmin : min(xviewmin, aes.xviewmin)
if Gadfly.issomething(aes.xviewmin)
xviewmin = Gadfly.isnothing(xviewmin) ? aes.xviewmin : min(xviewmin, aes.xviewmin)
end

if aes.xviewmax != nothing
xviewmax = xviewmax === nothing ? aes.xviewmax : max(xviewmax, aes.xviewmax)
if Gadfly.issomething(aes.xviewmax)
xviewmax = Gadfly.isnothing(xviewmax) ? aes.xviewmax : max(xviewmax, aes.xviewmax)
end

if aes.yviewmin != nothing
yviewmin = yviewmin === nothing ? aes.yviewmin : min(yviewmin, aes.yviewmin)
if Gadfly.issomething(aes.yviewmin)
yviewmin = Gadfly.isnothing(yviewmin) ? aes.yviewmin : min(yviewmin, aes.yviewmin)
end

if aes.yviewmax != nothing
yviewmax = yviewmax === nothing ? aes.yviewmax : max(yviewmax, aes.yviewmax)
if Gadfly.issomething(aes.yviewmax)
yviewmax = Gadfly.isnothing(yviewmax) ? aes.yviewmax : max(yviewmax, aes.yviewmax)
end
end

xmax = xviewmax === nothing ? xmax : max(xmax, xviewmax)
xmin = xviewmin === nothing ? xmin : min(xmin, xviewmin)
ymax = yviewmax === nothing ? ymax : max(ymax, yviewmax)
ymin = yviewmin === nothing ? ymin : min(ymin, yviewmin)
xmax = Gadfly.isnothing(xviewmax) ? xmax : max(xmax, xviewmax)
xmin = Gadfly.isnothing(xviewmin) ? xmin : min(xmin, xviewmin)
ymax = Gadfly.isnothing(yviewmax) ? ymax : max(ymax, yviewmax)
ymin = Gadfly.isnothing(yviewmin) ? ymin : min(ymin, yviewmin)

# Hard limits set in Coord should override everything else
xmin = coord.xmin === nothing ? xmin : coord.xmin
xmax = coord.xmax === nothing ? xmax : coord.xmax
ymin = coord.ymin === nothing ? ymin : coord.ymin
ymax = coord.ymax === nothing ? ymax : coord.ymax
xmin = Gadfly.isnothing(coord.xmin) ? xmin : coord.xmin
xmax = Gadfly.isnothing(coord.xmax) ? xmax : coord.xmax
ymin = Gadfly.isnothing(coord.ymin) ? ymin : coord.ymin
ymax = Gadfly.isnothing(coord.ymax) ? ymax : coord.ymax

if xmin === nothing || !isfinite(xmin)
if Gadfly.isnothing(xmin) || !isfinite(xmin)
xmin = 0.0
xmax = 1.0
end

if ymin === nothing || !isfinite(ymin)
if Gadfly.isnothing(ymin) || !isfinite(ymin)
ymin = 0.0
ymax = 1.0
end
Expand Down
2 changes: 1 addition & 1 deletion src/data.jl
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ function show(io::IO, data::Data)
maxlen = 0
print(io, "Data(")
for name in fieldnames(Data)
if getfield(data, name) != nothing
if issomething(getfield(data, name))
print(io, "\n ", string(name), "=")
show(io, getfield(data, name))
end
Expand Down
6 changes: 3 additions & 3 deletions src/dataframes.jl
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ function meltdata(U::AbstractDataFrame, colgroups_::Vector{Col.GroupedColumn})
vm = um
grouped_columns = Set{Symbol}()
for colgroup in colgroups
if colgroup.columns===nothing # null => group all columns
if isnothing(colgroup.columns) # null => group all columns
vm *= un
grouped_columns = copy(allcolumns)
else
Expand All @@ -34,7 +34,7 @@ function meltdata(U::AbstractDataFrame, colgroups_::Vector{Col.GroupedColumn})

# allocate vectors for grouped columns
for (j, colgroup) in enumerate(colgroups)
cols = colgroup.columns===nothing ? allcolumns : colgroup.columns
cols = isnothing(colgroup.columns) ? allcolumns : colgroup.columns

# figure the grouped common column type
firstcol = U[first(cols)]
Expand Down Expand Up @@ -64,7 +64,7 @@ function meltdata(U::AbstractDataFrame, colgroups_::Vector{Col.GroupedColumn})
col_indicators = Array{Symbol}(undef, vm, length(colgroups))
row_indicators = Array{Int}(undef, vm, length(colgroups))

colidxs = [colgroup.columns===nothing ? collect(allcolumns) : colgroup.columns
colidxs = [isnothing(colgroup.columns) ? collect(allcolumns) : colgroup.columns
for colgroup in colgroups]

vi = 1
Expand Down

0 comments on commit da4a328

Please sign in to comment.