fix grouping of zero-height bar in barplot (#3058)

* fix grouping of zero-height bar in `barplot`

* treat all-zero as positive

* store `last_pos` of each group

* add #3058 to NEWS

* test: add zero-height bar

* test: merge barplot-related tests into `barplot`

---------

Co-authored-by: Simon <sdanisch@protonmail.com>
Co-authored-by: Frederic Freyer <frederic481994@hotmail.de>

NEWS.md

@@ -1,6 +1,7 @@
 # News
 
 ## master
+- Fix grouping of a zero-height bar in `barplot`. Now a zero-height bar shares the same properties of the previous bar, and if the bar is the first one, its height is treated as positive if and only if there exists a bar of positive height or all bars are zero-height. [#3058](https://github.com/MakieOrg/Makie.jl/pull/3058)
 
 - Fixed a bug where Axis still consumes scroll events when interactions are disabled [#3272](https://github.com/MakieOrg/Makie.jl/pull/3272)
 

src/basic_recipes/barplot.jl

@@ -141,21 +141,24 @@ function stack_from_to(i_stack, y)
 end
 
 function stack_grouped_from_to(i_stack, y, grp)
-
     from = Array{Float64}(undef, length(y))
     to   = Array{Float64}(undef, length(y))
 
-    groupby = StructArray((; grp..., is_pos = y .> 0))
-
+    groupby = StructArray((; grp...))
     grps = StructArrays.finduniquesorted(groupby)
+    last_pos = map(grps) do (g, inds)
+        g => any(y[inds] .> 0) || all(y[inds] .== 0)
+    end |> Dict
+    is_pos = map(y, groupby) do v, g
+        last_pos[g] = iszero(v) ? last_pos[g] : v > 0
+    end
 
+    groupby = StructArray((; grp..., is_pos))
+    grps = StructArrays.finduniquesorted(groupby)
     for (grp, inds) in grps
-
         fromto = stack_from_to(i_stack[inds], y[inds])
-
         from[inds] .= fromto.from
         to[inds] .= fromto.to
-
     end
 
     (from = from, to = to)

test/barplot.jl

@@ -0,0 +1,130 @@
+@testset "Barplot" begin
+    @testset "label align" begin
+        @testset "automatic" begin
+            # for more info see https://github.com/MakieOrg/Makie.jl/issues/3160
+            # below is the best square angles behavior for bar labels
+
+            al = Makie.automatic
+
+            y_dir, flip = false, false
+            @test Makie.calculate_bar_label_align(al, 0.0, y_dir, flip) ≈ Vec2f(0.0, 0.5)
+            @test Makie.calculate_bar_label_align(al, π, y_dir, flip) ≈ Vec2f(1.0, 0.5)
+            @test Makie.calculate_bar_label_align(al, π/2, y_dir, flip) ≈ Vec2f(0.5, 1.0)
+            @test Makie.calculate_bar_label_align(al, -π/2, y_dir, flip) ≈ Vec2f(0.5, 0.0)
+
+            y_dir, flip = true, false
+            @test Makie.calculate_bar_label_align(al, 0.0, y_dir, flip) ≈ Vec2f(0.5, 0.0)
+            @test Makie.calculate_bar_label_align(al, π, y_dir, flip) ≈ Vec2f(0.5, 1.0)
+            @test Makie.calculate_bar_label_align(al, π/2, y_dir, flip) ≈ Vec2f(0.0, 0.5)
+            @test Makie.calculate_bar_label_align(al, -π/2, y_dir, flip) ≈ Vec2f(1.0, 0.5)
+
+            y_dir, flip = false, true
+            @test Makie.calculate_bar_label_align(al, 0.0, y_dir, flip) ≈ Vec2f(1.0, 0.5)
+            @test Makie.calculate_bar_label_align(al, π, y_dir, flip) ≈ Vec2f(0.0, 0.5)
+            @test Makie.calculate_bar_label_align(al, π/2, y_dir, flip) ≈ Vec2f(0.5, 0.0)
+            @test Makie.calculate_bar_label_align(al, -π/2, y_dir, flip) ≈ Vec2f(0.5, 1.0)
+
+            y_dir, flip = true, true
+            @test Makie.calculate_bar_label_align(al, 0.0, y_dir, flip) ≈ Vec2f(0.5, 1.0)
+            @test Makie.calculate_bar_label_align(al, π, y_dir, flip) ≈ Vec2f(0.5, 0.0)
+            @test Makie.calculate_bar_label_align(al, π/2, y_dir, flip) ≈ Vec2f(1.0, 0.5)
+            @test Makie.calculate_bar_label_align(al, -π/2, y_dir, flip) ≈ Vec2f(0.0, 0.5)
+        end
+
+        @testset "manual" begin
+            input = 0.0, false, false
+            for align in (Vec2f(1.0, 0.5), Point2f(1.0, 0.5), (1.0, 0.5), (1, 0), (1.0, 0))
+                @test Makie.calculate_bar_label_align(align, input...) ≈ Vec2f(align)
+            end
+        end
+
+        @testset "symbols" begin
+            input = 0.0, false, false
+            @test Makie.calculate_bar_label_align((:center, :center), input...) ≈ Makie.calculate_bar_label_align((0.5, 0.5), input...)
+        end
+
+        @testset "error" begin
+            input = 0.0, false, false
+            for align in ("center", 0.5, ("center", "center"))
+                @test_throws ErrorException Makie.calculate_bar_label_align(align, input...)
+            end
+        end
+    end
+
+    @testset "stack" begin
+        x1         = [1, 1,  1,  1]
+        grp_dodge1 = [2, 2,  1,  1]
+        grp_stack1 = [1, 2,  1,  2]
+        y1         = [2, 3, -3, -2]
+
+        x2         = [2, 2,  2,  2]
+        grp_dodge2 = [3, 4,  3,  4]
+        grp_stack2 = [3, 4,  3,  4]
+        y2         = [2, 3, -3, -2]
+
+        from, to = Makie.stack_grouped_from_to(grp_stack1, y1, (; x1 = x1, grp_dodge1 = grp_dodge1))
+        from1 = [0.0, 2.0,  0.0, -3.0]
+        to1   = [2.0, 5.0, -3.0, -5.0]
+        @test from == from1
+        @test to   == to1
+
+        from, to = Makie.stack_grouped_from_to(grp_stack2, y2, (; x2 = x2, grp_dodge2 = grp_dodge2))
+        from2 = [0.0,  0.0,  0.0,  0.0]
+        to2   = [2.0,  3.0, -3.0, -2.0]
+        @test from == from2
+        @test to   == to2
+
+        perm = [1, 4, 2, 7, 5, 3, 8, 6]
+        x = [x1; x2][perm]
+        y = [y1; y2][perm]
+        grp_dodge = [grp_dodge1; grp_dodge2][perm]
+        grp_stack = [grp_stack1; grp_stack2][perm]
+
+        from_test = [from1; from2][perm]
+        to_test = [to1; to2][perm]
+
+        from, to = Makie.stack_grouped_from_to(grp_stack, y, (; x = x, grp_dodge = grp_dodge))
+        @test from == from_test
+        @test to == to_test
+    end
+
+    @testset "zero-height" begin
+        grp_stack = [1, 2, 1, 2]
+        x = [1, 1, 2, 2]
+
+        y = [1.0, 0.0, -1.0, -1.0]
+        from = [0.0, 1.0, 0.0, -1.0]
+        to = [1.0, 1.0, -1.0, -2.0]
+        from_, to_ = Makie.stack_grouped_from_to(grp_stack, y, (; x))
+        @test from == from_
+        @test to == to_
+
+        y = [-1.0, 0.0, -1.0, -1.0]
+        from = [0.0, -1.0, 0.0, -1.0]
+        to = [-1.0, -1.0, -1.0, -2.0]
+        from_, to_ = Makie.stack_grouped_from_to(grp_stack, y, (; x))
+        @test from == from_
+        @test to == to_
+
+        y = [0.0, 1.0, -1.0, -1.0]
+        from = [0.0, 0.0, 0.0, -1.0]
+        to = [0.0, 1.0, -1.0, -2.0]
+        from_, to_ = Makie.stack_grouped_from_to(grp_stack, y, (; x))
+        @test from == from_
+        @test to == to_
+
+        y = [0.0, -1.0, -1.0, -1.0]
+        from = [0.0, 0.0, 0.0, -1.0]
+        to = [0.0, -1.0, -1.0, -2.0]
+        from_, to_ = Makie.stack_grouped_from_to(grp_stack, y, (; x))
+        @test from == from_
+        @test to == to_
+
+        y = [0.0, 1.0, -1.0, -1.0]
+        from = [0.0, 0.0, 0.0, -1.0]
+        to = [0.0, 1.0, -1.0, -2.0]
+        from_, to_ = Makie.stack_grouped_from_to(1:4, y, (; x=ones(4)))
+        @test from == from_
+        @test to == to_
+    end
+end

test/runtests.jl

@@ -29,11 +29,10 @@ using Makie: volume
     include("makielayout.jl")
     include("figures.jl")
     include("transformations.jl")
-    include("stack.jl")
     include("events.jl")
     include("text.jl")
     include("boundingboxes.jl")
     include("ray_casting.jl")
     include("PolarAxis.jl")
-    include("barplot_labels.jl")
+    include("barplot.jl")
 end