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

Improve spending function summary display #46

Closed
nanxstats opened this issue Feb 18, 2022 · 1 comment · Fixed by #159
Closed

Improve spending function summary display #46

nanxstats opened this issue Feb 18, 2022 · 1 comment · Fixed by #159

Comments

@nanxstats
Copy link
Collaborator

When using sfLDOF(), the summary text can be a bit confusing:

summary(sfLDOF(alpha = 0.25, t = 0.5, param = 0))
#> "Lan-DeMets O'Brien-Fleming approximation spending function with none = 1"

@elong0527 wonders if it makes more sense to display "... with no parameters".

Since the relevant logic in sfLDOF() seems intentional and might be written this way for a reason, I will let you make the decision.

Technical details

The call sequence is summary.spendfn() -> sfLDOF(). Here, since param = 0, sfLDOF() will set param to 1, and set parname to "none". Then summary.spendfn() will print parname = param (none = 1).

gsDesign/R/gsMethods.R

Lines 811 to 816 in cbf9a94

} else {
s <- paste(object$name, "spending function")
if (!is.null(object$parname) && !is.null(object$param)) {
s <- paste(s, "with", paste(object$parname, collapse = " "), "=", paste(object$param, collapse = " "))
}
}

gsDesign/R/gsSpending.R

Lines 711 to 717 in cbf9a94

if (is.null(param) || param < .005 || param > 20) param <- 1
checkScalar(param, "numeric", c(.005,20),c(TRUE,TRUE))
t[t>1] <- 1
if (param == 1){
rho <- 1
txt <- "Lan-DeMets O'Brien-Fleming approximation"
parname <- "none"

@nanxstats
Copy link
Collaborator Author

@elong0527 do you want to take a quick look at this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant