-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
S3 structure refactor #70
Conversation
Call with @seabbs:
Next steps:
Path to doing this:
|
My end of the call: This is looking really good. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall this looks good.
I would like some justification for the stan code inside epidist and I think we need a bunch of issues to address already discussed things (or discuss them more).
Thanks @seabbs! Have:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice looks good. This seems like a really solid basis to be getting started with. Happy all comments are now issues.
Description
This is my branch for playing with the S3 refactor. So far:
latent_truncation_censoring_adjusted_delay()
into generics:epidist_prepare
epidist_stancode
epidist_priors
epidist_formula
epidist_family
epidist
epidist_ltcad
class (an abbreviation of latent truncation censoring adjusted delay)obs_prep <- epidist_prepare(obs, model = "ltcad")
thenepidist(obs_prep)
replicates the functionality oflatent_truncation_censoring_adjusted_delay(obs)
Remaining questions
epidist
? This would be a feature extension i.e. making is easier for the user to provide their own priors. What about just letting them pass inbrms
priors?custom_stancode
andfamily
/formula
generics interact?custom_stancode
prepare
generic is called as a part of theepidist
fitting generic? i.e. in one step. If it's quite hard and limits what we can do, I'm fine to have the two-step processepidist::epidist
rather than have everything passed in as arguments. I guess this removes possible functionality from the user but simplifies things? Perhaps it's also going to be in some ways required if thecustom_stancode
is a function of somefamily
object...epidist_stancode
method and putting the relevant Stan code and options to modify it in the other relevant methods. If a user really really wants to change the underlying Stan code a lot then they can generate it and modify it themselves rather than interfacing with it via passing long strings.Closes #21.
Checklist