Update g_forest() to support col_x and col_ci in the same column. - #1507
Update g_forest() to support col_x and col_ci in the same column.#1507wwojciech wants to merge 8 commits into
g_forest() to support col_x and col_ci in the same column.#1507Conversation
|
Hi @danielinteractive , @gmbecker - Can you pl. review this new PR? Thank you! |
danielinteractive
left a comment
danielinteractive
left a comment
There was a problem hiding this comment.
Thanks @wwojciech please see my question below
|
Thanks @danielinteractive , please see the update code. |
… when the point estimate and CI share a column.
|
Hi @danielinteractive - I had to update the code to ensure backward compatibility, specifically to allow If you have a chance, could you please take a look at the updated code? |
| lwr <- upr <- lwr_t <- upr_t <- rep(NA_real_, nrow(tbl_df)) | ||
| } else { | ||
| sym_size <- rep(1, nrow(tbl_df)) | ||
| lwr <- x_ci[, ncol(x_ci) - 1] |
There was a problem hiding this comment.
are we sure here that ncol(x_ci) is >= 2?
There was a problem hiding this comment.
Yes, we are. If col_ci is not NULL (and nrow(tbl_df) >= 1), lines 284--288 ensure that the CI column has at least two elements. Consequently, lines 301--308 ensure that x_ci has at least two columns.
If nrow(tbl_df) == 0, then x_ci is NULL, so x_ci[, i] is NULL for any index i.
| sym_size <- rep(1, nrow(tbl_df)) | ||
| lwr <- x_ci[, ncol(x_ci) - 1] | ||
| upr <- x_ci[, ncol(x_ci)] | ||
| lwr_t <- x_ci_t[, ncol(x_ci_t) - 1] |
There was a problem hiding this comment.
same question here
There was a problem hiding this comment.
Same answer: Yes, we are (x_ci_t is derived directly from x_ci).
2 participants
Fixes #1499