-
Notifications
You must be signed in to change notification settings - Fork 133
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
Add support for analysis of deviance from car #580
Conversation
- added roxygen tags `@keywords internal - fixed some markdown syntax
…g the old behavior as a default)
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.
Hi Marius, it has take me a long time, sorry about that. I think this looks good, I just had two minor questions. Maybe you can help me understand the code a little better. Cheers!
if(all(colnames(x) != "F values")) y$df.residual <- NULL | ||
if(any(colnames(x) == "Cp")) y$df <- NULL | ||
|
||
if(is.null(y$statistic)) { |
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.
Hmm, this I don't understand. Shouldn't canonize
do this?
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.
I would consider these fixes specific to analysis of deviance that do not fit into a generic function such as canonize()
-- moving this to canonize()
would imply that we would need a list of test statistics that should be reported with degrees of freedom and a list of test statistics that should not be reported with degrees of freedom.
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.
I'm not sure I understand. This section does not deal with degrees of freedom, as far as I can tell. I'm referring here to the section where statistic
is null and is replaced by deviance
.
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.
I accidentally deleted my last comment.
Maybe the most concise answer is the following: In one very specific case (chi-squared tests for generalized linear models), a column named Deviance
contains the difference in deviance which is distributed as chi-squared, and is used as the test statistic.
In canonize()
, however, we rename Deviance
to deviance
, not statistic
, because columns of this name typically do not contain the test statistic, but simply the deviance of the model (in addition to the test statistic contained in another column). Because column Deviance
containing the test statistic is the exception, not the rule, I would opt for sticking with the current behavior of canonize()
and deal with the exception within the specific method.
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.
What a mess. That makes sense, thanks for clarifying!
Hi @crsh,
This is my first shot at supporting analysis of deviance output from the car package. I need this for one of my manuscripts. There are probably a lot of things we could do to improve the functionality, but this will do in my use case, and I think something is better than nothing.
Best,
Marius