Closed hillalex closed 4 weeks ago
Attention: Patch coverage is 97.61905%
with 1 line
in your changes missing coverage. Please review.
Project coverage is 97.40%. Comparing base (
3960395
) to head (8632ba8
). Report is 9 commits behind head on main.
Files with missing lines | Patch % | Lines |
---|---|---|
R/biokinetics.R | 96.00% | 1 Missing :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
This all looks great to me. The only additional thing I would say it could be sensible to add would be another column for the timings of the titre dtaw and/or their last exposure. We require calendar dates currently, which in all likelihood is the format the data will be in. But I do think it could be possible for their data to already be relative. So time = 0 could be their last exposure and some positive number after that could be when their titres were taken. If colleagues wisg to use this package with already processed/published data (quite likely I think!), or publicly available data, then the data is likely to be in this relative form, instead of calendar dates. Dates are quite sensitive in terms of identifiability, so allowing for these sorts of timings would be great. I'm a little unsure how to offer either one of these options, but I do think if that is easy to do, it would be worth it! Apart from that, it all looks great.
This all looks great to me. The only additional thing I would say it could be sensible to add would be another column for the timings of the titre dtaw and/or their last exposure. We require calendar dates currently, which in all likelihood is the format the data will be in. But I do think it could be possible for their data to already be relative. So time = 0 could be their last exposure and some positive number after that could be when their titres were taken. If colleagues wisg to use this package with already processed/published data (quite likely I think!), or publicly available data, then the data is likely to be in this relative form, instead of calendar dates. Dates are quite sensitive in terms of identifiability, so allowing for these sorts of timings would be great. I'm a little unsure how to offer either one of these options, but I do think if that is easy to do, it would be worth it! Apart from that, it all looks great.
This is a really helpful insight, thanks. I think I'll merge this PR and add support for relative time data on a new branch to make it easier to review.
stan_id
topid
to make the interface clearer and less implementation specifictitre
tovalue
to be more in line with other seroanalytics packages and to distinguish more clearly fromtitre_type
obs_id
,t_since_last_exp
,t_since_min_date
get_stan_data
so that the user can see the arguments passed to the model (useful for debugging)cens_me_idx
argumentAlso updates the data vignette to define the input variables. The simplified input format can be seen here: https://github.com/seroanalytics/epikinetics/blob/8632ba84064a40a85c433d16c9a5d4b7e5301ee9/vignettes/data.Rmd#L17