Closed ThomUK closed 3 years ago
Leaving ptd_rebase
with no arguments would be the correct approach here, that is we are saying we don't want to rebase. Passing in an argument of NULL is saying we want to rebase with an indeterminant value.
I think a more complete argument against this would be what if we say ptd_rebase(a = date1, b = NULL)
In this case are we going to say that we ignore b? Or is this an error? I think having NULL as a disallowed value in the first place is the ideal solution.
In my production code I have included the wrapper function below to get around this problem. If we close this issue it's available for others to use.
# a wrapper function for ptd_spc
# related to issue #91
# https://github.com/nhs-r-community/NHSRplotthedots/issues/91
# only needs to exist while the rebase argument will not accept NULL, NA, or "" values
# this wrapper accepts rebase input, and if null, it calls ptd_spc() without the argument, so that no error is thrown.
spc_chart_wrapper <- function(dtf, value_field, date_field, rebase_dates, fix_after_n_points, improvement_direction, target){
if(is.null(rebase_dates)) {
NHSRplotthedots::ptd_spc(
.data = dtf,
value_field = value,
date_field = x,
#no rebase argument
fix_after_n_points = nPointsToBaselineFrom,
improvement_direction = tolower(improvementDirection),
target = Target
)
} else {
NHSRplotthedots::ptd_spc(
.data = dtf,
value_field = value,
date_field = x,
rebase = rebase_dates, # rebase argument included
fix_after_n_points = nPointsToBaselineFrom,
improvement_direction = tolower(improvementDirection),
target = Target
)
}
}
A common use of the ptd_spc() function will be to loop through measures, some with and some without rebase dates.
In the case that a measure does not have a rebase date, currently passing NULL in throws an error.
Ideally spc_rebase() should accept NULL as input, returning NULL as output.
Current behaviour is: