Add exclude_rows argument to g_forest() (Solves #1498). - #1503
Conversation
|
Hi @gmbecker - I created this PR following your advice. However, I noticed that some of the checks are failing during the CLA step. @gmbecker, @danielinteractive - could you add yourselves as reviewers for this PR? |
danielinteractive
left a comment
danielinteractive
left a comment
There was a problem hiding this comment.
Thanks @wwojciech , please see comments below
|
Thanks, @danielinteractive - I replied to your questions. Regarding the other suggestion, I will apply it soon, as you suggested. |
danielinteractive
left a comment
danielinteractive
left a comment
There was a problem hiding this comment.
Looks good to me, thanks @wwojciech !
Thank you @danielinteractive . Can you pl. merge it? I have no writing rights. |
|
@wwojciech I cannot either. @Melkiades and @shajoezhu , could you help us? |
|
|
||
| tbl_df <- as_result_df(tbl) | ||
| if (!is.null(exclude_rows)) { | ||
| tbl_df <- tbl_df[-exclude_rows, ] |
There was a problem hiding this comment.
I do not understand the need of this parameter inside this function when you can add the filtering before the input i.e. doing tbl_df <- tbl_df[-exclude_rows, ] just before running g_forest()
The rest is fine, it makes the function more stable ^^
There was a problem hiding this comment.
tbl_df <- tbl_df[-exclude_rows, ]
The issue is that using g_forest(tbl[-exclude_rows, ]) removes rows from the table itself. The intention is to keep these rows visible in the table while only excluding them from the forest plot.
There was a problem hiding this comment.
It makes sense! sorry I had to double check its behavior again ^^ then it is is good to go!
Melkiades
left a comment
Melkiades
left a comment
There was a problem hiding this comment.
Lgtm! Thanks @wwojciech. Could you only fix cicd?
Thanks a lot, @Melkiades! Actually, I think the CI/CD is failing because I'm merging from a forked repository. I have no idea how to solve it. Maybe it's because my repository is named Is there anyone who could help me with this? Thanks! |
|
hi @wwojciech , can you try rename the branch wwojciech:1498_g_forest_exclude_rows , and raise another PR, and let's see. it feels like an cicd with branch naming issue. thanks |
4 participants
Fixes #1498