fix width computation bug in histogram

Project.toml

@@ -1,7 +1,7 @@
 name = "AlgebraOfGraphics"
 uuid = "cbdf2221-f076-402e-a563-3d30da359d67"
 authors = ["Pietro Vertechi <pietro.vertechi@veos.digital>"]
-version = "0.6.1"
+version = "0.6.2"
 
 [deps]
 Colors = "5ae59095-9a9b-59fe-a467-6f913c188581"

src/algebra/layers.jl

@@ -56,12 +56,9 @@ function compute_attributes(pl::ProcessedLayer)
 
     merge!(attrs, Dictionary(valid_options(; color, cycle)))
 
-    # avoid automatic bar width computation in Makie
-    if (plottype <: BarPlot) && !haskey(attrs, :width)
-        x = first(positional)
-        width = (x isa AbstractRange) && (length(positional) == 2) ? step(x) : 1.0
-        insert!(attrs, :width, width)
-    end
+    # avoid automatic bar width computation in Makie (issue #277)
+    # TODO: consider only implementing this when `x` is categorical
+    (plottype <: BarPlot) && !haskey(attrs, :width) && insert!(attrs, :width, 1)
 
     # remove unnecessary information 
     return filterkeys(!in((:col, :row, :layout, :alpha)), attrs)

src/transformations/histogram.jl

@@ -3,13 +3,19 @@ const categoricalplottypes = [BarPlot, Heatmap, Volume]
 function compute_edges(intervals::Tuple, bins, closed)
     bs = bins isa Tuple ? bins : map(_ -> bins, intervals)
     return map(intervals, bs) do (min, max), b
-        b isa AbstractRange && return b
+        b isa AbstractVector && return b
         b isa Integer && return histrange(float(min), float(max), b, closed)
-        msg = "only AbstractRange and Integer or tuples thereof are accepted as bins"
+        msg = "only AbstractVector and Integer or tuples thereof are accepted as bins"
         throw(ArgumentError(msg))
     end
 end
 
+function midpoints(edges::AbstractVector)
+    i0, i1 = firstindex(edges), lastindex(edges)
+    front, tail = view(edges, i0:i1-1), view(edges, i0+1:i1)
+    return (front .+ tail) ./ 2
+end
+
 function midpoints(edges::AbstractRange)
     min, s, l = minimum(edges), step(edges), length(edges)
     return range(min + s / 2, step=s, length=l - 1)
@@ -41,7 +47,9 @@ function (h::HistogramAnalysis)(input::ProcessedLayer)
 
     output = map(input) do p, n
         hist = _histogram(Tuple(p); pairs(n)..., pairs(options)...)
-        return (map(midpoints, hist.edges)..., hist.weights), (;)
+        edges, weights = hist.edges, hist.weights
+        named = length(edges) == 1 ? (; width=diff(first(edges))) : (;)
+        return (map(midpoints, edges)..., weights), named
     end
 
     N = length(input.positional)
@@ -62,12 +70,12 @@ end
 
 Compute a histogram.
 
-The attribute `bins` can be an `Integer`, an `AbstractRange`, or a `Tuple`
-of either integers or ranges (useful for 2- or 3-dimensional histograms).
+The attribute `bins` can be an `Integer`, an `AbstractVector` (in particular, a range), or
+a `Tuple` of either integers or abstract vectors (useful for 2- or 3-dimensional histograms).
 When `bins` is an `Integer`, it denotes the approximate number of equal-width
 intervals used to compute the histogram. In that case, the range covered by the
 intervals is defined by `datalimits` (defaults to the extrema of the data).
-When `bins` is an `AbstractRange`, it denotes the intervals directly.
+When `bins` is an `AbstractVector`, it denotes the intervals directly.
 
 `closed` determines whether the the intervals are closed to the left or to the right.
 

test/analyses.jl

@@ -245,6 +245,15 @@ end
     @test labels == map(AlgebraOfGraphics.to_label, processedlayer.labels)
 end
 
+@testset "midpoints" begin
+    edges = [1, 2, 10, 12]
+    @test midpoints(edges) ≈ [1.5, 6, 11]
+
+    edges_rg = 1:2:5
+    edges_v = [1, 3, 5]
+    @test midpoints(edges_v) ≈ midpoints(edges_rg) ≈ [2, 4]
+end
+
 @testset "histogram1D" begin
     df = (x=rand(1000), c=rand(["a", "b"], 1000))
     bins = 0:0.01:1
@@ -259,12 +268,15 @@ end
     w2 = fit(Histogram, x2, bins).weights
 
     rgx, w = processedlayer.positional
+    width = processedlayer.named[:width]
 
     @test rgx[1] ≈ (bins[1:end-1] .+ bins[2:end]) ./ 2
     @test w[1] == w1
+    @test width[1] ≈ diff(bins)
 
     @test rgx[2] ≈ (bins[1:end-1] .+ bins[2:end]) ./ 2
     @test w[2] == w2
+    @test width[2] ≈ diff(bins)
 
     bins, closed = 12, :left
     layer = data(df) * mapping(:x, color=:c) * histogram(; bins, closed, datalimits=extrema)
@@ -279,16 +291,19 @@ end
     w2 = fit(Histogram, x2, bins2).weights
 
     rgx, w = processedlayer.positional
+    width = processedlayer.named[:width]
 
     @test rgx[1] ≈ (bins1[1:end-1] .+ bins1[2:end]) ./ 2
     @test w[1] ≈ w1
+    @test width[1] ≈ diff(bins1)
 
     @test rgx[2] ≈ (bins2[1:end-1] .+ bins2[2:end]) ./ 2
     @test w[2] ≈ w2
+    @test width[2] ≈ diff(bins2)
 
     @test processedlayer.primary == NamedArguments((color=["a", "b"],))
-    @test isempty(processedlayer.named)
     @test processedlayer.attributes == NamedArguments((gap=0, dodge_gap=0))
+    @test keys(processedlayer.named) == Indices([:width])
     @test processedlayer.plottype == AlgebraOfGraphics.BarPlot
 
     labels = MixedArguments()
@@ -304,7 +319,7 @@ end
 
 @testset "weightedhistogram1d" begin
     df = (x=rand(1000), z=rand(1000), c=rand(["a", "b"], 1000))
-    bins = 0:0.01:1
+    bins = collect(0:0.01:1) # test vector of bins
 
     layer = data(df) * mapping(:x, color=:c, weights=:z) * histogram(; bins)
     processedlayer = AlgebraOfGraphics.ProcessedLayer(layer)
@@ -318,15 +333,18 @@ end
     w2 = fit(Histogram, x2, weights(z2), bins).weights
 
     rgx, w = processedlayer.positional
+    width = processedlayer.named[:width]
 
     @test rgx[1] ≈ (bins[1:end-1] .+ bins[2:end]) ./ 2
     @test w[1] ≈ w1
+    @test width[1] ≈ diff(bins)
 
     @test rgx[2] ≈ (bins[1:end-1] .+ bins[2:end]) ./ 2
     @test w[2] ≈ w2
+    @test width[2] ≈ diff(bins)
 
     @test processedlayer.primary == NamedArguments((color=["a", "b"],))
-    @test isempty(processedlayer.named)
+    @test keys(processedlayer.named) == Indices([:width])
     @test processedlayer.attributes == NamedArguments((gap=0, dodge_gap=0))
     @test processedlayer.plottype == AlgebraOfGraphics.BarPlot
 

test/runtests.jl

@@ -2,8 +2,11 @@ using AlgebraOfGraphics, Makie, Random, Statistics, Test
 
 using AlgebraOfGraphics: Sorted
 using AlgebraOfGraphics: separate
+using AlgebraOfGraphics: midpoints
 using AlgebraOfGraphics: Arguments, MixedArguments, NamedArguments
 
+using Dictionaries: Indices
+
 using KernelDensity: kde, pdf
 using StatsBase: fit, histrange, Histogram, weights
 using GLM: GLM