-
-
Notifications
You must be signed in to change notification settings - Fork 40
Add support for "kenward" option
#1175
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
Conversation
Summary of ChangesHello @strengejacke, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request streamlines the handling of Kenward-Roger approximations within the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request adds support for the "kenward" option for degrees of freedom approximation, particularly for glmmTMB models. A significant and beneficial change is the refactoring of the Kenward-Roger implementation, moving the complex logic to the insight package, which greatly simplifies this package's codebase. The changes are generally well-executed. However, I've identified a critical bug in se_kenward() where an undefined variable is used, which will cause an error. I've also pointed out a minor style issue that impacts code readability. Addressing these points will solidify the improvements made in this PR.
|
@gemini-code-assist review |
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.
Pull Request Overview
This PR adds support for the "kenward" (Kenward-Roger) approximation method as a ci_method option for mixed models, particularly extending support to glmmTMB models. The implementation includes comprehensive refactoring of existing Kenward-Roger functions to use S3 methods and delegation to the insight package for more efficient calculations.
- Refactored Kenward-Roger functions to use S3 method dispatch and delegate calculations to
insight::get_df()andinsight::get_varcov() - Added full support for Kenward-Roger and Satterthwaite methods in
glmmTMBmodels - Updated test suite with extensive coverage for the new functionality across different model types
Reviewed Changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/testthat/test-model_parameters_df_method.R | Comprehensive test updates adding glmmTMB model testing and validating Kenward-Roger method consistency |
| R/standard_error_kenward.R | Converted to S3 methods with glmmTMB support using insight::get_varcov() |
| R/p_value_kenward.R | Simplified implementation removing model-specific methods, added REML refitting logic |
| R/dof_kenward.R | Streamlined to delegate to insight::get_df() instead of custom implementation |
| R/dof_satterthwaite.R | Simplified to use insight::get_df() delegation |
| R/ci_kenward.R | Added S3 methods with specific glmmTMB support and component handling |
| R/ci_satterthwaite.R | Added conditional component support for glmmTMB models |
| R/methods_glmmTMB.R | Enhanced with Kenward-Roger and Satterthwaite method support in parameter extraction |
| man/p_value_kenward.Rd | Updated function signatures to include ellipsis parameters |
| inst/WORDLIST | Added lcmm to spelling dictionary |
| NEWS.md | Documentation of new glmmTMB support for advanced CI methods |
| NAMESPACE | Updated exports for new S3 methods |
| DESCRIPTION | Version bump to 0.28.2.8 |
| .github/SUPPORT.md | Simplified reprex documentation reference |
| .github/CONTRIBUTING.md | Removed specific tidyverse reprex link |
| R/ci_generic.R | Added formatting comments and parameter restructuring |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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.
Code Review
This pull request is a significant and well-executed refactoring. It adds support for Kenward-Roger and Satterthwaite approximations for glmmTMB models, which is a valuable feature. The approach of moving the complex calculations to the insight package and creating generic functions in parameters greatly improves the code's structure and maintainability. The changes are consistent across different functions, and the addition of comprehensive tests ensures the new functionality is working correctly and doesn't introduce regressions. I've found one minor issue regarding some leftover dead code, which I've commented on. Overall, this is an excellent contribution.
No description provided.