Should we use functions to test the arguments instead of just stopifnot?
We could throw better error/warning messages
This ensures that we allow the same inputs everywhere. Now we have just a bunch of stopifnot clauses copied all over the place.
Error messages are pretty bad now because stopifnot returns uninformative errors:
pReplicate(po = 0, c = 1, alternative = "two.sided")
Error in (function (p, alternative = c("two.sided", "one.sided", "less", : 0 < p is not TRUE
We have function checks but they are often repeated many times. Maybe we should have a distinction between internal and exported functions.
For example: z2p_int() which does all the transformations but no input checks and then an exported function z2p() which checks arguments and then calls z2p_int.
In combination with the idea above, this could lead to much more modular code and much less repetition in the the code base.
Specific points
Some issues with edge cases in conversionHelpers.R
See e.g. p2z(1, "one.sided") which returns -Inf
This leads to problems in other functions as well
With alpha = 1 in thresholdIntrinsic? (We do not allow alpha = 0)
ci2p(lower = 5, upper = 5, conf.level = 0.95, ratio = TRUE, alternative = "two.sided") throws an error. However, the error is not caught in the argument checks of ci2p. It is, however, caused by ci2z(lower = 5, upper = 5, conf.level = 0.95, ratio = TRUE) which evaluates to Inf which is then caught in the input checks of z2p, which uses the previous result as argument z = Inf.
I do not know why we export certain functions. We usually export the vectorized function but in some cases we also export the non-vectorized function.
exported
.z2p_
.p2z_
.powerSignificance_
.sampleSizeReplicationSuccess_
.pSceptical_
not exported
.sampleSizeSignificance_
Argument alpha of function pvalueBound has no input checks ensuring it is between 0 and 1
Sample size based on the relative effect size d. Do we keep it? It currently does not work for type = "controlled"
Documentation: currently "level" is documented as "level for replication success", which is not correct.
Internal naming of function. "alphaS" should be called "gamma". We should change it in the functions.
pSceptical(zo = 7, zr = 7, c = 2, type = "controlled") returns 0, numerical problems
Writing new tests (type = "controlled", other functions such as levelSceptical, etc)
=========================================================================================
General propositions
Should we use functions to test the arguments instead of just
stopifnot?stopifnotclauses copied all over the place.stopifnotreturns uninformative errors:pReplicate(po = 0, c = 1, alternative = "two.sided")Error in (function (p, alternative = c("two.sided", "one.sided", "less", : 0 < p is not TRUEWe have function checks but they are often repeated many times. Maybe we should have a distinction between internal and exported functions.
z2p_int()which does all the transformations but no input checks and then an exported functionz2p()which checks arguments and then callsz2p_int.Specific points
Some issues with edge cases in
conversionHelpers.Rp2z(1, "one.sided")which returns-Infalpha = 1inthresholdIntrinsic? (We do not allowalpha = 0)ci2p(lower = 5, upper = 5, conf.level = 0.95, ratio = TRUE, alternative = "two.sided")throws an error. However, the error is not caught in the argument checks ofci2p. It is, however, caused byci2z(lower = 5, upper = 5, conf.level = 0.95, ratio = TRUE)which evaluates toInfwhich is then caught in the input checks ofz2p, which uses the previous result as argumentz = Inf.I do not know why we export certain functions. We usually export the vectorized function but in some cases we also export the non-vectorized function.
.z2p_.p2z_.powerSignificance_.sampleSizeReplicationSuccess_.pSceptical_.sampleSizeSignificance_Argument
alphaof functionpvalueBoundhas no input checks ensuring it is between 0 and 1