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
fix issue #567 (ergm), fix issue #572 (svyglm) #611
Conversation
note that a problem will subsist with ergm::get.miss.dyads, which is deprecated
Code is very clean and I love the tests! Left some comments -- am trying to make |
still depends on `process_lm` and `tidy.summary.lm`
also modified PR #611 to make clear what is returned
@alexpghayes Hi -- I have implemented your suggestions. Quick question -- would you be interested in removing some dependencies to the package? Update -- I've explored both options above, and while removing |
Few more comments. I'm planning on looking at dependencies more systematically in #582 at some point. Personally I find |
Hi @alexpghayes All suggested changes implemented, although let me know if you want Not sure why the PR creates a conflict. |
Removed conflicts with base branch. |
Thank you so much! |
You're welcome. I like working on I think it'd be a good idea for me to stop adding to this PR for now, and to come back to it if In the meantime, I'll have a look at how to augment |
Thank you both for taking care of this and your patience! |
This pull request has been automatically locked. If you believe you have found a related problem, please file a new issue (with a reprex: https://reprex.tidyverse.org) and link to this issue. |
Fix issue #567 (warnings with
ergm
objects)The code now calls the
ergm:::summary.ergm
function internally, to avoid a warning about the function not being meant to be called directly.Note that a problem will subsist withergm::get.miss.dyads
(used in theglance
method), which is deprecated.Fixed thanks to @krivit.ergm::get.miss.dyads
has been replaced with the code fromergm
3.9.4. Note sure it's doing the right thing, though: see my discussion of the issue in #567.Fix issue #572 (
glance
method not working forsvyglm
objects)The code adds
tidy.svyglm
andglance.svyglm
. The first one isjust a copy ofclose totidy.(g)lm
. The second one is identical toglance.glm
, except:The
survey:::summary.svyglm
function is called instead ofsummary
(which evaluates tostats::summary.glm
).The
BIC
is computed differently, usingsurvey:::dBIC(model, maximal = model
) (a maximal model can be provided, but the default is to use the samemodel
).No
logLik
is returned, given thatsvyglm
is not fitted via ML.I have added an example for
glance.svyglm
, provided by @IndrajeetPatil in #572.Side comment: BIC in
glm
andsvyglm
The docs for
glance.glm
andglance.svyglm
(in@evalRd return_glance()
) say that they return the BIC, but they do not always do so:(Comments below made obsolete by changes sollicited by @alexpghayes in the comments below.)
The biggest change is that I had to add a dirty hack tofinish_glance
inutilities.R
in order to extract the AIC from asvyglm
object, since the result ofsurvey:::AIC.svyglm
is not similar to that ofstats::AIC
(the first one is a 3-element named vector, the second one is a 1-element vector).A similar hack was already in the code (for the deviance of REML objects created bylmerMod
).