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

Use getFromNamespace instead of ::: to fix S3 dispatch for as_gt() #299

Merged
merged 1 commit into from
Nov 15, 2024

Conversation

jdblischak
Copy link
Collaborator

Avoid R CMD check NOTE about accessing internal functions between packages we maintain (in this case {gsDesign2})

xref: #284, #287, Merck/gsDesign2#490, Merck/gsDesign2#492

@jdblischak jdblischak self-assigned this Nov 15, 2024
@jdblischak
Copy link
Collaborator Author

Demonstration that it still works:

library("gsDesign2")
library("simtrial")
## Attaching package: ‘simtrial’
##
## The following object is masked from ‘package:gsDesign2’:
##
##     as_gt
##

# works without gsDesign2:: prefix
gs_design_ahr() |> summary() |> as_gt()
fixed_design_ahr(
  enroll_rate = define_enroll_rate(duration = 18, rate = 1),
  fail_rate = define_fail_rate(
    duration = c(4, 100),
    fail_rate = log(2) / 12,
    hr = c(1, .6),
    dropout_rate = .001
  )
) |> summary() |> as_gt()

@jdblischak
Copy link
Collaborator Author

All the builds returned no NOTEs. Though in this case there was no NOTE beforehand, presumably because the dev version of {gsDesign2} is being installed (#297). It seems that it is ok with ::: as long as the internal function exists. Still safer to use getFromNamespace() though to avoid any potential problems during CRAN submission

Copy link
Collaborator

@LittleBeannie LittleBeannie left a comment

Choose a reason for hiding this comment

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

Thank you, @jdblischak !

@LittleBeannie LittleBeannie merged commit 18a7211 into Merck:main Nov 15, 2024
7 checks passed
@jdblischak jdblischak deleted the getfromnamespace branch November 15, 2024 21:15
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.

2 participants