Update g_forest() to support col_x and col_ci in the same column.

[wwojciech]

Fixes #1499

[wwojciech]

Hi @danielinteractive , @gmbecker - Can you pl. review this new PR? Thank you!

[danielinteractive]

Thanks @wwojciech please see my question below

[wwojciech]

Thanks @danielinteractive , please see the update code.

[wwojciech]

Hi @danielinteractive - I had to update the code to ensure backward compatibility, specifically to allow col_x or col_ci to be NULL, hiding x or ci when the point estimate and CI share a column.

If you have a chance, could you please take a look at the updated code?

[Melkiades]

All in all it looks good to me!! Thanks for the addition @wwojciech. Once Daniel approves we can proceed

[danielinteractive]

I would still prefer to have the assertion as well. Reason: If this code is changed in the future ... then this logic might be broken. And you will not see it easily because the code will still run.

[wwojciech]

I would still prefer to have the assertion as well. Reason: If this code is changed in the future ... then this logic might be broken. And you will not see it easily because the code will still run.

Sure, thanks, @danielinteractive. Would the below assertion be sufficient for this purpose?

# We essentially need to check the number of columns in `x_ci` (if it is not NULL).
checkmate::assert_matrix(x_ci, min.cols = 2, null.ok = TRUE)

[wwojciech]

Thank you @danielinteractive.
@Melkiades - can you pl. approve it, (Daniel has just approved it). Thanks!

[wwojciech]

Hi @Melkiades - can you pl. have a look at this PR, Thanks!

[Melkiades]

Hi @Melkiades - can you pl. have a look at this PR, Thanks!

Good to go for me! But cannot merge it or approve it... @shajoezhu could you take a look into this?