epiverse-trace / serofoi

Estimates the Force-of-Infection of a given pathogen from population based sero-prevalence studies
https://epiverse-trace.github.io/serofoi/
Other
17 stars 4 forks source link

Add failsafe for `foi_model` argument in `fit_seromodel()` to avoid errors #91

Closed jamesmbaazam closed 10 months ago

jamesmbaazam commented 1 year ago

In fit_seromodel() here https://github.com/epiverse-trace/serofoi/blob/d7f66d789ea7238e8d96765e4266c26f91cfd7a5/R/modelling.R#L150-L167, any value passed to the argument foi_model will "work" until the point https://github.com/epiverse-trace/serofoi/blob/d7f66d789ea7238e8d96765e4266c26f91cfd7a5/R/modelling.R#L169-L183 when it fails.

This could potentially lead to issues for the user. One solution will be to make it fail fast by specifying the values of the argument in the function definition as foi_model = c("constant", "tv_normal_log", "tv_normal"), and then match the argument as the first step in the function body as:

fit_seromodel(<some arguments>, foi_model = c("constant", "tv_normal_log", "tv_normal")){
foi_model <- match.args(foi_model)
.
.
.
}

This way, an erroneous value passed to foi_model will fail early.

I will submit a pull request for this.

jamesmbaazam commented 1 year ago

Here's a reprex of the issue when I pass "a" to foi_model

library(serofoi)
data("serodata")
serodata <- serofoi::prepare_serodata(serodata)
seromodel_fit <- serofoi::fit_seromodel(serodata = serodata,
                             foi_model = "a")
#> Error in (function (classes, fdef, mtable) : unable to find an inherited method for function 'sampling' for signature '"NULL"'

Created on 2023-07-06 with reprex v2.0.2

jamesmbaazam commented 1 year ago

@ntorresd I see you self-assigned but I have already addressed this issue in the linked PR. Could you rather assign me and review the PR?