fix grouping of zero-height bar in barplot
#3058
Conversation
| @@ -110,7 +110,11 @@ 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)) | |||
There was a problem hiding this comment.
As far as I can tell just changing this to .>= works too.
There was a problem hiding this comment.
I am afraid not. Given two stacked bars of heights [-6, 0], the second bar would stacked on the first one as expected.
There was a problem hiding this comment.
Oh. You do remind me that we should determine the is_pos only based other bars inside of the same group, instead of globally. I will append a commit later. Thanks.
There was a problem hiding this comment.
Sorry for the delayed update. I have been too busy recently :(. A new commit has been added. You can test it using:
import Makie, CairoMakie
fig = Makie.Figure()
ax = Makie.Axis(fig[1, 1])
xs = [1, 1, 1, 1]
ys = [0, 1, -1, -1]
stack = [1, 2, 1, 2]
dodge = [1, 1, 2, 2]
color = Makie.cgrad(:Set2_8)[1:4]
Makie.barplot!(ax, xs, ys; stack, dodge, color, bar_labels=string.(ys))
display(fig)
|
It would be good to extend the tests in https://github.com/MakieOrg/Makie.jl/blob/master/test/stack.jl to include cases that got fixed here. While you're at it you could also merge that with https://github.com/MakieOrg/Makie.jl/blob/master/test/barplot_labels.jl into a barplot.jl. Also still needs a note in https://github.com/MakieOrg/Makie.jl/blob/master/NEWS.md on what this pr fixes. |
|
ok. I will update it later. |
|
Please let me know if any further modification is required. thanks |
|
Looks fine to me but I think @SimonDanisch or @jkrumbiegel should take another look at this since I don't know the barplot code that well. |
|
To be honest, I don't know the grouping code very well either.. I guess that has been @jkrumbiegel and @piever expertise ;) |
|
Not my code either, but looks ok to me :) |
fix #3055