-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Redirect as_gt() to simtrial when it is masked by gsDesign2::as_gt() #467
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @jdblischak !
Hi @jdblischak, it looks this PR is still in progress. Is it ready for merge now? |
Note: we are going to try these short-term fixes for now. |
I'm okay with merging these two PRs at the moment. In the long run, I'd still like to explore the possibility of I could also ask the author of gt to provide an S3 generic in gt, but I think it's too late now (the world will be even messier if |
IIUC I think this strategy could work. gsDesign2 already has gt in Imports. simtrial might need to be move it from Suggests to Imports though. The r-lib/generics docs explicitly recommend re-exporting the generic. So if the gt package added an
|
Honestly, I'm not sure if making gt a hard dependency is ideal. For one example: gt depends on juicyjuice, which depends on V8. It creates installation issues sometimes in the real-world: rstudio/gt#1179 However, I'm not a maintainer or active contributor to any of these packages, so YMMV 😁 |
I don't think the presentation of results as tables is a core function of this package, so we shouldn't take a hard dependency on gt. Recently I tried to install gt in a fresh R installation, and I was impressed by the number of packages being dragged into my system. Although personally I'm not interested in tables, someday I might experiment on an alternative lightweight implementation. (Re: rstudio/gt#1179, I've never liked the idea of fully inlining CSS---it's a crime in my eyes). P.S. Technically, if gt does provide an |
Counterpart to Merck/simtrial#287 (xref: Merck/simtrial#284)
cc: @yihui
Fixes the S3 redirection when gsDesign2::as_gt() masks simtrial::as_gt(), as below: