-
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
Issue #338: Add double dispatch for epidist_family
#365
Conversation
Oops, looks like I branched off the transparency issue rather than main. Maybe this can be fixed anyway if that branch is merged. Edit: yes that worked fine. |
18d7d41
to
8ca944a
Compare
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.
This looks great. One question about when we add custom info to family
It's a real bug! And the integration test with gamma caught it! Nice stuff. > family_gamma1 <- epidist_family(prep_obs, family = stats::Gamma)
> family_gamma2 <- epidist_family(prep_obs, family = stats::Gamma(link = "log"))
> family_gamma1$reparam
[1] "mu" "shape"
> family_gamma2$reparam
[1] "mu" "shape"
> family_gamma1$family
[1] "custom"
> family_gamma2$family
[1] "custom" (Bug is that it's not doing the reparam right. Edit: should be fixed) |
I think this is in a good place, I expect CI to hopefully pass this time. Then it remains to improve the documentation and tests a bit and iron out the discussion points above. |
Needs a redoc and work on the gamma method for reparam to pass checks |
…eparameterisation, the rest is the same for all families
…ame, document and lint
cfff3df
to
a0fad1f
Compare
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.
This looks really great. One outstanding question about when to validate data
Description
This draft PR will close #338. It refactors the structure of
epidist_family
:epidist_family_model
S3 to dispatch on the "model specific parts" of the family.add_dpar_info
helper to put thedpar
information other thanmu
there conveniently for use inepidist_family_model
epidist_family_reparam
to perform the only family specific hard coded part: reparam to line upbrms
and Stanepidist_family
to call these nicer modular functionsWe have added a new issue to decide how to export functions in future (including those introduced by this PR): #79
Checklist