Solve #598#600
Conversation
Fixed display issue for logical predictors in report() function.
There was a problem hiding this comment.
Pull request overview
This PR addresses issue #598 by improving how report_intercept() determines and prints the intercept’s reference level when a model includes logical predictors.
Changes:
- Update intercept reference-level detection to handle logical predictors.
- Add a regression test covering a logical predictor in
lm(). - Add a NEWS entry documenting the fix for #598.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
R/report_intercept.R |
Adds a logical-predictor branch when building the intercept “corresponding to …” text. |
tests/testthat/test-report_intercept.R |
Adds a test asserting the intercept reference level prints as FALSE for a logical predictor. |
NEWS.md |
Documents the fix and introduces a new development version header. |
| # report 0.6.3.1 | ||
| * Fixed an issue in `report()` where the reference level for logical predictors was incorrectly displayed as `[?]` instead of `FALSE` for the intercept (@M-Colley, #598). | ||
|
|
There was a problem hiding this comment.
The NEWS entry introduces version 0.6.3.1, but DESCRIPTION still reports version 0.6.3. Please bump the package version in DESCRIPTION to match and keep NEWS headings consistent with the existing format (e.g., add a "Bug fixes" subsection under the new version header).
There was a problem hiding this comment.
Please bump the version number in the description file.
| mtcars$more_than_4_cyl <- as.logical(mtcars$cyl > 4) | ||
| m4 <- lm(mpg ~ more_than_4_cyl, data = mtcars) | ||
| expect_identical( | ||
| as.character(report_intercept(m4)), | ||
| "The model's intercept, corresponding to more_than_4_cyl = FALSE, is at 26.66 (95% CI [24.41, 28.92], t(30) = 24.16, p < .001)." | ||
| ) |
There was a problem hiding this comment.
This test mutates the shared mtcars dataset (mtcars$more_than_4_cyl <- ...). Since many other tests rely on mtcars, this can leak state and make the suite order-dependent. Prefer using a dedicated copy (and, if insight::get_data() requires it, assign that copy into .GlobalEnv and clean it up with on.exit()), rather than modifying mtcars itself.
| mtcars$more_than_4_cyl <- as.logical(mtcars$cyl > 4) | |
| m4 <- lm(mpg ~ more_than_4_cyl, data = mtcars) | |
| expect_identical( | |
| as.character(report_intercept(m4)), | |
| "The model's intercept, corresponding to more_than_4_cyl = FALSE, is at 26.66 (95% CI [24.41, 28.92], t(30) = 24.16, p < .001)." | |
| ) | |
| mtcars2 <- mtcars | |
| mtcars2$more_than_4_cyl <- as.logical(mtcars2$cyl > 4) | |
| # insight::get_data needs access to data in global environment | |
| assign("mtcars2", mtcars2, envir = .GlobalEnv) | |
| on.exit(rm("mtcars2", envir = .GlobalEnv), add = TRUE) | |
| m4 <- lm(mpg ~ more_than_4_cyl, data = mtcars2) | |
| expect_identical( | |
| as.character(report_intercept(m4)), | |
| "The model's intercept, corresponding to more_than_4_cyl = FALSE, is at 26.66 (95% CI [24.41, 28.92], t(30) = 24.16, p < .001)." | |
| ) |
There was a problem hiding this comment.
You should simply use data(mtcars) here to re-init the data set.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| mtcars$more_than_4_cyl <- as.logical(mtcars$cyl > 4) | ||
| m4 <- lm(mpg ~ more_than_4_cyl, data = mtcars) | ||
| expect_identical( | ||
| as.character(report_intercept(m4)), | ||
| "The model's intercept, corresponding to more_than_4_cyl = FALSE, is at 26.66 (95% CI [24.41, 28.92], t(30) = 24.16, p < .001)." | ||
| ) |
There was a problem hiding this comment.
You should simply use data(mtcars) here to re-init the data set.
| # report 0.6.3.1 | ||
| * Fixed an issue in `report()` where the reference level for logical predictors was incorrectly displayed as `[?]` instead of `FALSE` for the intercept (@M-Colley, #598). | ||
|
|
There was a problem hiding this comment.
Please bump the version number in the description file.
Solving #598 by checking for logical factor.