-
Notifications
You must be signed in to change notification settings - Fork 129
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 robust errors to gallery #2129
base: main
Are you sure you want to change the base?
Conversation
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.
@aghaynes thank you so much for adding this! I think many users will find this super helpful! Can you make a couple of small updates and we can merge?
DESCRIPTION
Outdated
@@ -79,6 +79,7 @@ Suggests: | |||
parameters (>= 0.20.2), | |||
parsnip (>= 0.1.7), | |||
rmarkdown, | |||
sandwich, |
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.
Since this is an article and not a proper vignette that ships with the package, we don't need to list sandwich under Suggests
. Instead we can add it under Config/Needs/website
vignettes/articles/gallery.Rmd
Outdated
mutate(subject_id = dplyr::row_number(), .by = trt) | ||
lmod <- lm(response ~ trt + grade, data = dat) | ||
|
||
library(sandwich) |
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.
can we namespace the sandwich functions instead of attaching, please?
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.
@aghaynes this looks great!! Thank you for adding this; I think it will be helpful for many users.
I just have one more question about the exponentiation NOTE, and I think we can merge! 🕺🏼 🕺🏼
vignettes/articles/gallery.Rmd
Outdated
tbl_robust | ||
``` | ||
|
||
Note that depending on your model, you may also need to set other arguments in `tidy_robust` (e.g. exponentiate). |
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.
Do we need this comment? Could we not just include tbl_regression(exponentiate=TRUE)
instead?
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.
Great catch! I changed the model to a logit (which is actually anyway better for this outcome), and included the exp argument directly.
What changes are proposed in this pull request?
As discussed in the issue, here's an example of using robust standard errors. I had to add a dependency (under suggests) for sandwich. I hope thats OK. If you happen to know of an existing dependency that contains a function for modifying the vcov matrix, i'm happy to try that instead.
<< Addition of an example on robust standard errors to the gallery vignette >>
If there is an GitHub issue associated with this pull request, please provide link.
closes #2076
Reviewer Checklist (if item does not apply, mark is as complete)
pkgdown::build_site()
. Check the R console for errors, and review the rendered website.devtools::test_coverage()
usethis::use_spell_check()
runs with no spelling errors in documentationWhen the branch is ready to be merged into master:
NEWS.md
with the changes from this pull request under the heading "# gtsummary (development version)
". If there is an issue associated with the pull request, reference it in parentheses at the end update (seeNEWS.md
for examples).usethis::use_version(which = "dev")
usethis::use_spell_check()
again