fsolt / dotwhisker

Dot-and-Whisker Plots of Regression Results
https://fsolt.org/dotwhisker/
Other
60 stars 10 forks source link

Error in adding brackets #63

Closed sammo3182 closed 7 years ago

sammo3182 commented 7 years ago

add_brackets failed to be functional when using changed var names after dwplot(...) + relabel_y_axis(...). I checked relabel_y_axis and add_brackets, and they work well separately. But when using them together causes border issues. I guess it is caused by the recent update of the grid package, but I haven't found where exactly the error comes from. @fsolt, could you have a check on the issue when you have time? Or let me know if this is only a problem for PC users.

Here's a minimum example of the problem:

library(dotwhisker)
data(mtcars)
# Works well without var name changing
m1 <- lm(mpg ~ wt + cyl + disp + gear, data = mtcars)
p1 <- dwplot(m1)

b1 <- list(c("Overall", "wt", "wt"))
g1 <- p %>% add_brackets(b)
grid.arrange(g1)    # to display

# Fail when using changed var name
p2 <- dwplot(m1) +relabel_y_axis(c("Weight", "Cylinders", "Displacement",  "Gears"))
b2 <- list(c("Overall", "Weight", "Weight"))

g2 <- p2 %>% add_brackets(b2)

grid.arrange(g)    # to display

# Works with original names
p3 <- dwplot(m1) +relabel_y_axis(c("Weight", "Cylinders", "Displacement",  "Gears))
b3 <- list(c("Overall", "wt", "wt"))

g3 <- p3 %>% add_brackets(b3)

grid.arrange(g)    # to display

You should get the following series of warning when doing the g2 part and failed to plot g2:

Warning messages:
1: In max(v1, v2) : no non-missing arguments to max; returning -Inf
2: In min(v1, v2) : no non-missing arguments to min; returning Inf
3: In max(v1, v2) : no non-missing arguments to max; returning -Inf
4: In min(v1, v2) : no non-missing arguments to min; returning Inf
5: In max(v1, v2) : no non-missing arguments to max; returning -Inf
6: In min(v1, v2) : no non-missing arguments to min; returning Inf
7: In min(x) : no non-missing arguments to min; returning Inf
8: In max(x) : no non-missing arguments to max; returning -Inf
fsolt commented 7 years ago

This doesn't seem like a bug to me, Hu. As you point out, all you have to do is use the actual variable names rather than the labels. Do you have a real use case in mind where you couldn't specify the actual variables?

sammo3182 commented 7 years ago

You are right. The vignette example needs to be updated, though. I'll work on that once I get some time.

fsolt commented 7 years ago

Hmm--the vignette is working fine for me using the variable labels. In fact, it throws errors if I use the variable names. So back to square one, I guess.

fsolt commented 7 years ago

Okay, so the problem is in relabel_y_axis rather than in add_brackets. It seems that the behavior of ggplot2::scale_y_continuous has changed--but only when there's just a single model? Weird. Anyway, I think the solution is to deprecate relabel_y_axis (without fixing it for the single model case) and make relabel_predictors work both before and after dwplot. I'll push that in a couple of minutes.