Feature: add logistic regression and multiple regression as options#7
Feature: add logistic regression and multiple regression as options#7
Conversation
- `rwa_multiregress()` is created as a duplicate of the original `rwa()`. - `rwa()` is used as a wrapper to call `rwa_multiregress()` and `rwa_logit()`
There was a problem hiding this comment.
Pull request overview
This pull request adds support for logistic regression alongside the existing multiple regression functionality in the relative weights analysis (RWA) package. The changes refactor the original rwa() function into specialized implementations and convert it to a wrapper that automatically detects the appropriate regression method.
Changes:
- Created
rwa_multiregress()containing the original multiple regression implementation - Created
rwa_logit()implementing logistic regression for binary outcomes - Converted
rwa()into a wrapper function with automatic method detection based on outcome variable
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 22 comments.
Show a summary per file
| File | Description |
|---|---|
| R/rwa.R | Refactored into wrapper function with method routing logic and automatic binary outcome detection |
| R/rwa_multiregress.R | Extracted original multiple regression implementation with full import statements |
| R/rwa_logit.R | New logistic regression implementation using SVD decomposition approach |
| man/rwa.Rd | Updated documentation with new method parameter and logistic regression examples |
| man/rwa_multiregress.Rd | Documentation for extracted multiple regression function |
| man/rwa_logit.Rd | Documentation for new logistic regression function |
| NAMESPACE | Added exports for new rwa_logit and rwa_multiregress functions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…in rwa_logit() and rwa_multiregress()
…gress and rwa_logit
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 15 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
R/rwa_logit.R
Outdated
| Rescaled.RelWeight = PropWeights) %>% | ||
| mutate(Sign = sign) | ||
|
|
||
| complete_cases <- nrow(drop_na(thedata)) |
There was a problem hiding this comment.
Missing tidyr namespace prefix. Line 18 uses 'tidyr::drop_na()', but this line uses 'drop_na()' without the namespace prefix. Use 'tidyr::drop_na()' for consistency.
| complete_cases <- nrow(drop_na(thedata)) | |
| complete_cases <- nrow(tidyr::drop_na(thedata)) |
…ons with examples and additional output details
…it and rwa_multiregress functions
…n plot_rwa function
…do R-squared and Lambda matrix
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Complete the documentation for the lambda return value, explaining that it is the transformation matrix mapping original correlated predictors to orthogonal variables while preserving their relationship to the outcome.
…martinctc/rwa into feature/add-logistic-regression
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 10 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Address code review feedback for API consistency and documentation: - rwa_logit: Use 'Variables' column name (matching rwa_multiregress) - rwa_logit: Use the existing Variables variable instead of 'predictors' - rwa_multiregress: Fix docs to reference rwa_multiregress() not rwa() - plot_rwa: Remove lambda fallback in caption (lambda is a matrix, not scalar) - vignette: Fix iris_logit\ to iris_logit\ (pseudo R-squared) - test-rwa: Fix incorrect comment (logistic regression does return rsquare) This ensures consistent API across both regression methods and accurate documentation throughout the package.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 11 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Code quality: Add dplyr:: prefix to select/mutate, stats:: to sd. Remove extraneous blank lines in test-rwa.R. Numerical stability: Clamp predicted probabilities to avoid Inf/-Inf in logit transformation. Add warning and fallback for zero stddev (perfect separation).
- rwa_logit: Write specific description instead of inheriting from rwa() - plot_rwa: Simplify comment (both functions now use Variables column) - vignette: Remove README-specific fig.path parameter
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@copilot Can you help summarise the changes in this PR in a numbered bulleted list? |
|
@martinctc I've opened a new pull request, #23, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
All checks passing - merging |
Summary
This branch adds logistic regression and multiple regression as additional options. This version is incremented to
0.1.2.9000as the development version (not submitted to CRAN).The changes made in this PR are:
rwa_logit()for binary outcomes (logistic regression) andrwa_multiregress()for continuous outcomes (multiple regression)rwa()to automatically detect method based on outcome variable (2 unique values → logistic, otherwise → multiple regression) with explicitmethodparameter overriderwa_logit()for edge cases (clipping predictions away from 0/1, zero standard deviation handling)plot_rwa()to handle both regression types with appropriate labelingUsage example:
Check
roxygen2::roxygenise()has been run prior to merging to ensure that.RdandNAMESPACEfiles are up to date.