alexanderlynl / safestats Goto Github PK

When nPlanBatch, deltaMin (and alpha) are fixed, aside from power, implied target should also be included in the safeDesign object, for the t and logrank test. Implied target: exp (E [log E-value])

Suggestion Peter: add power with and without optional stopping as options for designSafeZTest and designSafeTTest

Please make sure that you do not change the user's options, par or working directory. If you really have to do so within functions, please ensure with an immediate call of on.exit() that the settings are reset when the function is exited

#21

For all tests where possible (@AlexanderLyNL logrank and z-test if I am correct?) confidence intervals should be printed by default in the print.safeTest function. For the logrank test, the z-based version can be printed for now, with a warning. If you have finalized the code for the z- and logrank printing could you warn me, then I will format the 2x2 design accordingly!

You have examples for unexported functions which cannot run in this way. Please either add packagename::: to the function calls in the examples, omit these examples or export these functions.

• Currently, a safeTSim objects are saved as RData files
• The samples (dataGroup1 and dataGroup2) are stored in safeTSim$safeSim and in safeTSim$freqSim
• Generating these data is easy:

someData <- generateNormalData("nPlan"=c(n1Plan, n2Plan), "nSim"=nSim, "deltaTrue"=deltaTrue,
                                 "muGlobal"=muGlobal, "sigmaTrue"=sigmaTrue, "paired"=paired, "seed"=seed)

• Make RDate files smaller

Trick:

1. Generate data on demand in the vignette itself (fast)
2. Provide data as argument for (simulate.safeDesign, thus, replicateTTest)
3. Drink whisky

Please add \value to .Rd files regarding exported methods and explain the functions results in the documentation. (If a function does not return a value, please document that too, e.g. \value{No return value, called for side effects} or similar)

• Please always but solely write package names, software names and API names in single quotes in title and description.

• Please add () behind all function names in the description text. e.g: --> example()

• Please add \value to .Rd files regarding exported methods and explain the functions results in the documentation.
  (If a function does not return a value, please document that too, e.g. \value{No return value, called for side effects} or similar)

• You have examples for unexported functions which cannot run in this way.
  Please either add packagename::: to the function calls in the examples, omit these examples or export these functions.

• \dontrun{} should only be used if the example really cannot be executed (e.g. because of missing additional software, missing API keys, ...) by the user. That's why wrapping examples in \dontrun{} adds the comment ("# Not run:") as a warning for the user.
  Does not seem necessary.
  Please unwrap the examples if they are executable in < 5 sec, or replace \dontrun{} with \donttest{}.

• You write information messages to the console that cannot be easily suppressed.
  It is more R like to generate objects that can be used to extract the information a user is interested in, and then print() that object.
  Instead of print()/cat() rather use message()/warning() or if(verbose)cat(..) (or maybe stop()) if you really have to write text to the console.
  (except for print() and summary() functions)
  e.g.: safe2x2Test.R, ...

• Please make sure that you do not change the user's options, par or working directory. If you really have to do so within functions, please ensure with an immediate call of on.exit() that the settings are reset when the function is exited. e.g.:
  ...
  oldpar <- par(no.readonly = TRUE) # code line i
  on.exit(par(oldpar)) # code line i + 1
  ...
  par(mfrow=c(2,2)) # somewhere after
  ...
  e.g.: helpers, ...
  If you're not familiar with the function, please check ?on.exit. This function makes it possible to restore options before exiting a function even if the function breaks. Therefore it needs to be called immediately after the option change within a function.

What we call "nPlan" now in the t- and z-test (I am not sure about the logrank test) should be called "nPlanBatch" and should be printed at the bottom of print.safeDesign, in a note.

For prospective designs, "nPlan" should be changed to be the nPlan under optional stopping (i.e., quantile of M simulated stopping times under an optional stopping experiment with parameters set according to the design, with nMax set to infinity).

"nPlanExpected" should be available when requested in the design function: nPlanExpected is the mean of M simulated stopping times under optional stopping, with nMax set to nPlan defined as above.

Experiments in the safe stats vignette in many cases concern nPlanBatch, without optional stopping. This should be made clearer.