Skip to content
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

Fix as_gt() examples #502

Merged
merged 2 commits into from
Feb 20, 2025
Merged

Conversation

jdblischak
Copy link
Collaborator

The as_gt() examples are skipped during R CMD check since they generate a lot of extraneous output (#334, #338). However, this means they are not regularly tested. While working on #497, I discovered that the examples with gs_power_ahr() and gs_power_wlr() were broken.

Error in gs_power_wlr() : 
  gs_power_wlr: please input the total_spend to the futility spending function.

I did a git bisect to determine that the example broke in a6a0166 when we updated gs_power_ahr() and gs_power_wlr() to require that users specify total_spend (#396, #398).

I fixed the examples as was done for as_rtf() in #398 and also updated the examples to be run during R CMD check to prevent this from happening again. The downsides are that gsDesign2.Rcheck/gsDesign2-Ex.Rout gets bloated with all the as_gt() output, and the examples are also decently long-running. If CRAN complains about either of these on the next submission, then we can re-evaluate the strategy. An alternative would be to copy the examples into a test file, and then return to skipping the examples during R CMD check.

@jdblischak jdblischak self-assigned this Feb 19, 2025
Copy link
Collaborator

@nanxstats nanxstats left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking great, thanks for identifying this problematic behavior and the neat correction.

One small issue: CRAN will almost surely flag this check note as a problem, considering their build machine has even lower single-core performance than mine:

* checking examples ... [27s/15s] NOTE
Examples with CPU (user + system) or elapsed time > 5s
       user system elapsed
as_gt 8.552  0.254   4.681

You probably know that the standard approach to avoid this note is by wrapping the examples in \donttest{} - although they will still be ran.

@jdblischak
Copy link
Collaborator Author

@nanxstats I have wrapped the as_gt.gs_design() examples in \donttest{}, and now as_gt() does not appear in any of the GitHub Actions R CMD check results.

There are some other example sections that occasionally run over 5s in some of the GitHub Actions jobs, but those weren't affected by my PR. I propose we wait and clean up any remaining longer-running examples in one-go prior to the next release (or the next time they are updated).

@jdblischak jdblischak requested a review from nanxstats February 20, 2025 16:00
Copy link
Collaborator

@nanxstats nanxstats left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Looking good to me.

@LittleBeannie LittleBeannie merged commit fc3e9a6 into Merck:main Feb 20, 2025
7 checks passed
@jdblischak jdblischak deleted the fix-as-gt-examples branch February 20, 2025 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants