Skip to content

Commit

Permalink
Changed default for histogram bins from closed-right to closed-left (J…
Browse files Browse the repository at this point in the history 
…uliaLang#185)

* Change default for histogram bins from closed-right to closed-left

* Deprecation warning in histogram funcs if kw arg "closed" unspecified

As the default for "closed" has changed from :left to :right, print
a deprecation warning for a while if "closed" is not specified
explicitly.

* Adapt histogram tests to temp. deprecation of default for "closed" arg

* Add description of "FIXME: closed" comments to test/hist.jl
  • Loading branch information
@oschulz @ararslan
oschulz authored and ararslan committed Mar 15, 2017
1 parent b69e06a commit 1b815de
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 56 deletions.
76 changes: 50 additions & 26 deletions src/hist.jl
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,22 @@
import Base: show, ==, push!, append!

# Mechanism for temporary deprecation of default for "closed" (because default
# value has changed). After deprecation is lifed, remove "_check_closed_arg"
# and all calls to it, and replace every ":default_left" with ":left". Also
# remove "closed=:left" in tests for all lines marked "FIXME: closed".
function _check_closed_arg(closed::Symbol, funcsym)
if closed == :default_left
Base.depwarn("Default for keyword argument \"closed\" has changed from :right to :left.", funcsym)
:left
else
closed
end
end


## nice-valued ranges for histograms
function histrange{T}(v::AbstractArray{T}, n::Integer, closed::Symbol=:right)
function histrange{T}(v::AbstractArray{T}, n::Integer, closed::Symbol=:default_left)
closed = _check_closed_arg(closed,:histrange)
F = float(T)
nv = length(v)
if nv == 0 && n < 0
Expand All @@ -16,7 +31,8 @@ function histrange{T}(v::AbstractArray{T}, n::Integer, closed::Symbol=:right)
histrange(F(lo), F(hi), n, closed)
end

function histrange{F}(lo::F, hi::F, n::Integer, closed::Symbol=:right)
function histrange{F}(lo::F, hi::F, n::Integer, closed::Symbol=:default_left)
closed = _check_closed_arg(closed,:histrange)
if hi == lo
start = F(hi)
step = one(F)
Expand Down Expand Up @@ -108,14 +124,14 @@ type Histogram{T<:Real,N,E} <: AbstractHistogram{T,N,E}
end
end

Histogram{T,N}(edges::NTuple{N,AbstractVector},weights::AbstractArray{T,N},closed::Symbol=:right) =
Histogram{T,N,typeof(edges)}(edges,weights,closed)
Histogram{T,N}(edges::NTuple{N,AbstractVector},weights::AbstractArray{T,N},closed::Symbol=:default_left) =
Histogram{T,N,typeof(edges)}(edges,weights,_check_closed_arg(closed,:Histogram))

Histogram{T,N}(edges::NTuple{N,AbstractVector},::Type{T},closed::Symbol=:right) =
Histogram(edges,zeros(T,map(x -> length(x)-1,edges)...),closed)
Histogram{T,N}(edges::NTuple{N,AbstractVector},::Type{T},closed::Symbol=:default_left) =
Histogram(edges,zeros(T,map(x -> length(x)-1,edges)...),_check_closed_arg(closed,:Histogram))

Histogram{N}(edges::NTuple{N,AbstractVector},closed::Symbol=:right) =
Histogram(edges,Int,closed)
Histogram{N}(edges::NTuple{N,AbstractVector},closed::Symbol=:default_left) =
Histogram(edges,Int,_check_closed_arg(closed,:Histogram))

function show(io::IO, h::AbstractHistogram)
println(io, typeof(h))
Expand All @@ -130,14 +146,14 @@ end
(==)(h1::Histogram,h2::Histogram) = (==)(h1.edges,h2.edges) && (==)(h1.weights,h2.weights) && (==)(h1.closed,h2.closed)

# 1-dimensional
Histogram{T}(edge::AbstractVector, weights::AbstractVector{T}, closed::Symbol=:right) =
Histogram((edge,), weights, closed)
Histogram{T}(edge::AbstractVector, weights::AbstractVector{T}, closed::Symbol=:default_left) =
Histogram((edge,), weights, _check_closed_arg(closed,:Histogram))

Histogram{T}(edge::AbstractVector, ::Type{T}, closed::Symbol=:right) =
Histogram(edge, zeros(T,length(edge)-1), closed)
Histogram{T}(edge::AbstractVector, ::Type{T}, closed::Symbol=:default_left) =
Histogram(edge, zeros(T,length(edge)-1), _check_closed_arg(closed,:Histogram))

Histogram(edge::AbstractVector,closed::Symbol=:right) =
Histogram(edge, Int, closed)
Histogram(edge::AbstractVector,closed::Symbol=:default_left) =
Histogram(edge, Int, _check_closed_arg(closed,:Histogram))

function push!{T,E}(h::Histogram{T,1,E}, x::Real,w::Real)
i = if h.closed == :right
Expand Down Expand Up @@ -165,15 +181,19 @@ function append!{T}(h::AbstractHistogram{T,1}, v::AbstractVector,wv::WeightVec)
h
end

fit(::Type{Histogram},v::AbstractVector, edg::AbstractVector; closed::Symbol=:right) =
append!(Histogram(edg,closed), v)
fit(::Type{Histogram},v::AbstractVector; closed::Symbol=:right, nbins=sturges(length(v))) =
fit(::Type{Histogram},v::AbstractVector, edg::AbstractVector; closed::Symbol=:default_left) =
append!(Histogram(edg,_check_closed_arg(closed,:fit)), v)
fit(::Type{Histogram},v::AbstractVector; closed::Symbol=:default_left, nbins=sturges(length(v))) = begin
closed = _check_closed_arg(closed,:fit)
fit(Histogram, v, histrange(v,nbins,closed); closed=closed)
end

fit{W}(::Type{Histogram},v::AbstractVector, wv::WeightVec{W}, edg::AbstractVector; closed::Symbol=:right) =
append!(Histogram(edg,W,closed), v, wv)
fit(::Type{Histogram},v::AbstractVector, wv::WeightVec; closed::Symbol=:right, nbins=sturges(length(v))) =
fit{W}(::Type{Histogram},v::AbstractVector, wv::WeightVec{W}, edg::AbstractVector; closed::Symbol=:default_left) =
append!(Histogram(edg,W,_check_closed_arg(closed,:fit)), v, wv)
fit(::Type{Histogram},v::AbstractVector, wv::WeightVec; closed::Symbol=:default_left, nbins=sturges(length(v))) = begin
closed = _check_closed_arg(closed,:fit)
fit(Histogram, v, wv, histrange(v,nbins,closed); closed=closed)
end

# N-dimensional
function push!{T,N}(h::Histogram{T,N},xs::NTuple{N,Real},w::Real)
Expand Down Expand Up @@ -204,12 +224,16 @@ function append!{T,N}(h::AbstractHistogram{T,N}, vs::NTuple{N,AbstractVector},wv
h
end

fit{N}(::Type{Histogram}, vs::NTuple{N,AbstractVector}, edges::NTuple{N,AbstractVector}; closed::Symbol=:right) =
append!(Histogram(edges,closed), vs)
fit{N}(::Type{Histogram}, vs::NTuple{N,AbstractVector}; closed::Symbol=:right, nbins=sturges(length(vs[1]))) =
fit{N}(::Type{Histogram}, vs::NTuple{N,AbstractVector}, edges::NTuple{N,AbstractVector}; closed::Symbol=:default_left) =
append!(Histogram(edges,_check_closed_arg(closed,:fit)), vs)
fit{N}(::Type{Histogram}, vs::NTuple{N,AbstractVector}; closed::Symbol=:default_left, nbins=sturges(length(vs[1]))) = begin
closed = _check_closed_arg(closed,:fit)
fit(Histogram, vs, histrange(vs,nbins,closed); closed=closed)
end

fit{N,W}(::Type{Histogram}, vs::NTuple{N,AbstractVector}, wv::WeightVec{W}, edges::NTuple{N,AbstractVector}; closed::Symbol=:right) =
append!(Histogram(edges,W,closed), vs, wv)
fit{N}(::Type{Histogram},vs::NTuple{N,AbstractVector}, wv::WeightVec; closed::Symbol=:right, nbins=sturges(length(vs[1]))) =
fit{N,W}(::Type{Histogram}, vs::NTuple{N,AbstractVector}, wv::WeightVec{W}, edges::NTuple{N,AbstractVector}; closed::Symbol=:default_left) =
append!(Histogram(edges,W,_check_closed_arg(closed,:fit)), vs, wv)
fit{N}(::Type{Histogram},vs::NTuple{N,AbstractVector}, wv::WeightVec; closed::Symbol=:default_left, nbins=sturges(length(vs[1]))) = begin
closed = _check_closed_arg(closed,:fit)
fit(Histogram, vs, wv, histrange(vs,nbins,closed); closed=closed)
end
67 changes: 37 additions & 30 deletions test/hist.jl
View file
Original file line number Diff line number Diff line change
@@ -1,29 +1,34 @@
# See src/hist.jl for meaning of "FIXME: closed" comments.

using StatsBase
using Base.Test

# test hist
@test sum(fit(Histogram,[1,2,3]).weights) == 3
@test fit(Histogram,Int[]).weights == Int[]
@test fit(Histogram,[1]).weights == [1]
@test fit(Histogram,[1,2,3],[0,2,4]) == Histogram([0,2,4],[2,1])
@test fit(Histogram,[1,2,3],[0,2,4]) != Histogram([0,2,4],[1,1])
@test fit(Histogram,[1,2,3],0:2:4) == Histogram(0:2:4,[2,1])
@test all(fit(Histogram,[1:100;]/100,0.0:0.01:1.0).weights .==1)
@test fit(Histogram,[1,1,1,1,1]).weights[1] == 5
@test sum(fit(Histogram,(rand(100),rand(100))).weights) == 100
@test sum(fit(Histogram,[1,2,3], closed=:left).weights) == 3 # FIXME: closed
@test fit(Histogram,Int[], closed=:left).weights == Int[] # FIXME: closed
@test fit(Histogram,[1], closed=:left).weights == [1] # FIXME: closed
@test fit(Histogram,[1,2,3],[0,2,4], closed=:left) == Histogram([0,2,4],[1,2], :left) # FIXME: closed
@test fit(Histogram,[1,2,3],[0,2,4], closed=:left) != Histogram([0,2,4],[1,1], :left) # FIXME: closed
@test fit(Histogram,[1,2,3],0:2:4, closed=:left) == Histogram(0:2:4,[1,2], :left) # FIXME: closed
@test all(fit(Histogram,[0:99;]/100,0.0:0.01:1.0, closed=:left).weights .==1) # FIXME: closed
@test fit(Histogram,[1,1,1,1,1], closed=:left).weights[1] == 5 # FIXME: closed
@test sum(fit(Histogram,(rand(100),rand(100)), closed=:left).weights) == 100 # FIXME: closed
@test fit(Histogram,1:100,nbins=5,closed=:right).weights == [20,20,20,20,20]
@test fit(Histogram,1:100,nbins=5,closed=:left).weights == [19,20,20,20,20,1]
@test fit(Histogram,0:99,nbins=5,closed=:right).weights == [1,20,20,20,20,19]
@test fit(Histogram,0:99,nbins=5,closed=:left).weights == [20,20,20,20,20]

@test fit(Histogram,(1:100,1:100),nbins=5).weights == diagm([20,20,20,20,20])
@test fit(Histogram,(1:100,1:100),nbins=(5,5)).weights == diagm([20,20,20,20,20])
# FIXME: closed (all lines in this block):
@test fit(Histogram,(0:99,0:99),nbins=5, closed=:left).weights == diagm([20,20,20,20,20])
@test fit(Histogram,(0:99,0:99),nbins=(5,5), closed=:left).weights == diagm([20,20,20,20,20])

@test fit(Histogram,1:100,weights(ones(100)),nbins=5).weights == [20,20,20,20,20]
@test fit(Histogram,1:100,weights(2*ones(100)),nbins=5).weights == [40,40,40,40,40]
# FIXME: closed (all lines in this block):
@test fit(Histogram,0:99,weights(ones(100)),nbins=5, closed=:left).weights == [20,20,20,20,20]
@test fit(Histogram,0:99,weights(2*ones(100)),nbins=5, closed=:left).weights == [40,40,40,40,40]

@test eltype(fit(Histogram,1:100,weights(ones(Int,100)),nbins=5).weights) == Int
@test eltype(fit(Histogram,1:100,weights(ones(Float64,100)),nbins=5).weights) == Float64
# FIXME: closed (all lines in this block):
@test eltype(fit(Histogram,1:100,weights(ones(Int,100)),nbins=5, closed=:left).weights) == Int
@test eltype(fit(Histogram,1:100,weights(ones(Float64,100)),nbins=5, closed=:left).weights) == Float64

# histrange
# Note: atm histrange must be qualified
Expand All @@ -47,28 +52,30 @@ using Base.Test
@test StatsBase.histrange(Int64[1:5;], 1, :left) == 0:5:10
@test StatsBase.histrange(Int64[1:10;], 1, :left) == 0:10:20

@test StatsBase.histrange([1, 2, 3, 4], 4) == 0.0:1.0:4.0
@test StatsBase.histrange([1, 2, 2, 4], 4) == 0.0:1.0:4.0
@test StatsBase.histrange([1, 10], 4) == 0.0:5.0:10.0
@test StatsBase.histrange([1, 20], 4) == 0.0:5.0:20.0
@test StatsBase.histrange([1, 600], 4) == 0.0:200.0:600.0
@test StatsBase.histrange([1, -1000], 4) == -1500.0:500.0:500.0
# FIXME: closed (all lines in this block):
@test StatsBase.histrange([0, 1, 2, 3], 4, :left) == 0.0:1.0:4.0
@test StatsBase.histrange([0, 1, 1, 3], 4, :left) == 0.0:1.0:4.0
@test StatsBase.histrange([0, 9], 4, :left) == 0.0:5.0:10.0
@test StatsBase.histrange([0, 19], 4, :left) == 0.0:5.0:20.0
@test StatsBase.histrange([0, 599], 4, :left) == 0.0:200.0:600.0
@test StatsBase.histrange([-1, -1000], 4, :left) == -1000.0:500.0:0.0

# Base issue #13326
l,h = extrema(StatsBase.histrange([typemin(Int),typemax(Int)], 10))
l,h = extrema(StatsBase.histrange([typemin(Int),typemax(Int)], 10, :left)) # FIXME: closed
@test l <= typemin(Int)
@test h >= typemax(Int)

@test_throws ArgumentError StatsBase.histrange([1, 10], 0)
@test_throws ArgumentError StatsBase.histrange([1, 10], -1)
@test_throws ArgumentError StatsBase.histrange([1.0, 10.0], 0)
@test_throws ArgumentError StatsBase.histrange([1.0, 10.0], -1)
@test_throws ArgumentError StatsBase.histrange(Float64[],-1)
@test_throws ArgumentError StatsBase.histrange([0.], 0)
# FIXME: closed (all lines in this block):
@test_throws ArgumentError StatsBase.histrange([1, 10], 0, :left)
@test_throws ArgumentError StatsBase.histrange([1, 10], -1, :left)
@test_throws ArgumentError StatsBase.histrange([1.0, 10.0], 0, :left)
@test_throws ArgumentError StatsBase.histrange([1.0, 10.0], -1, :left)
@test_throws ArgumentError StatsBase.histrange(Float64[],-1, :left)
@test_throws ArgumentError StatsBase.histrange([0.], 0, :left)


# hist show
show_h = sprint(show, fit(Histogram,[1,2,3]))
show_h = sprint(show, fit(Histogram,[0,1,2], closed=:left)) # FIXME: closed
@test contains(show_h, "edges:\n 0.0:1.0:3.0")
@test contains(show_h, "weights: $([1,1,1])")
@test contains(show_h, "closed: right")
@test contains(show_h, "closed: left")

0 comments on commit 1b815de

Please sign in to comment.