add '-2*log(L)' to anova tidy renamer list#1239
Merged
Merged
Conversation
Collaborator
|
If we make a change here I'd advocate for a syntactic name that doesn't
require backticks to interface with dplyr, etc
…On Tue, Mar 25, 2025, 9:46 AM Ben Bolker ***@***.***> wrote:
We recently changed the name "deviance" to "-2*log(L)" in anova output in
lme4 ("deviance" is technically incorrect, and confusing, for the value we
were actually reporting). This does not affect broom directly, but it
affects the cardx package <https://github.com/insightsengineering/cardx>,
which calls broom::tidy() on anova() output from lme4. Because "-2*log(L)"
isn't in the list of renamers
<https://github.com/tidymodels/broom/blob/main/R/stats-anova.R#L55-L88>
that tidy.anova uses, this now throws a warning (and makes cardx fail its
checks).
It's a little hard to implement a test for this change since lme4 is
handled by broom.mixed and not broom ... you could do something hacky
like this, but I'll leave it up to you to decide if you want to do that.
m <- lm(mpg ~ cyl + hp, mtcars)aa <- anova(m)
names(aa)[5] <- "-2*log(L)"
expect_no_warning(broom::tidy(aa))
I know pushing out a new release is a hassle, but at least it's been 6
months since the last release and there are some other improvements to
include in this release ... do you think you might be able to do this soon
... ?
------------------------------
You can view, comment on, or merge this pull request online at:
#1239
Commit Summary
- 6c97bc8
<6c97bc8>
add '-2*log(L)' to anova tidy renamer list
File Changes
(1 file <https://github.com/tidymodels/broom/pull/1239/files>)
- *M* R/stats-anova.R
<https://github.com/tidymodels/broom/pull/1239/files#diff-7f3b99a01cc8f29596522c10c72622e8dc56c6051800d342ca48d2a1ecddd772>
(4)
Patch Links:
- https://github.com/tidymodels/broom/pull/1239.patch
- https://github.com/tidymodels/broom/pull/1239.diff
—
Reply to this email directly, view it on GitHub
<#1239>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADTBG23GAKTFEGC4MCEHZID2WFT3NAVCNFSM6AAAAABZX2OFO6VHI2DSMVQWIX3LMV43ASLTON2WKOZSHE2DMNZZGQYDINQ>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
Contributor
Author
|
FWIW what my PR does is to rename "-2*log(L)" and "X.2.log.L." (the name-mangled version) to "minus2logL", which is clunky but is indeed syntactically safe ... (Our rationale for choosing "-2*log(L)" as the label even though it's syntactically unsafe is that it's hard to think of a short, syntactically safe alternative ...) |
simonpcouch
approved these changes
Mar 26, 2025
simonpcouch
left a comment
Collaborator
There was a problem hiding this comment.
Thanks for the PR! Just saw your email as well--happy to get this on CRAN in the coming days.
|
This pull request has been automatically locked. If you believe the issue addressed here persists, please file a new PR (with a reprex: https://reprex.tidyverse.org) and link to this one. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We recently changed the name "deviance" to "-2*log(L)" in
anovaoutput inlme4("deviance" is technically incorrect, and confusing, for the value we were actually reporting). This does not affectbroomdirectly, but it affects the cardx package, which callsbroom::tidy()onanova()output from lme4. Because "-2*log(L)" isn't in the list of renamers thattidy.anovauses, this now throws a warning (and makescardxfail its checks).It's a little hard to implement a test for this change since
lme4is handled bybroom.mixedand notbroom... you could do something hacky like this, but I'll leave it up to you to decide if you want to do that.I know pushing out a new release is a hassle, but at least it's been 6 months since the last release and there are some other improvements to include in this release ... do you think you might be able to do this soon ... ?