Skip to content

Solve #598 #600

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.

Sign up for GitHub

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

Open
M-Colley wants to merge 9 commits into
base: main
Choose a base branch
from
+23 −0
Open

Solve #598 #600

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
488e78f
Handle logical values in report_intercept function
M-Colley Apr 27, 2026
737a18a
Update test-report_intercept.R
M-Colley Apr 27, 2026
1f6ab56
Update NEWS.md for version 0.6.3.1
M-Colley Apr 27, 2026
de1c79e
Apply suggestion from @Copilot
strengejacke Apr 29, 2026
a05ba35
Merge branch 'main' into main
strengejacke Apr 29, 2026
f661beb
Fix formatting of expect_identical in test-report_intercept
M-Colley May 5, 2026
6a8be12
Update test-report_intercept.R
M-Colley May 5, 2026
2654c3c
Bump version from 0.6.3.1 to 0.6.4.1
M-Colley May 5, 2026
cc27ca2
Merge branch 'main' into main
M-Colley May 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions NEWS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
# 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).

Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copilot generated this review using guidance from repository custom instructions.
Copy link
Copy Markdown
Member

@strengejacke strengejacke Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please bump the version number in the description file.

# report 0.6.3

Bug fixes
Expand Down
2 changes: 2 additions & 0 deletions R/report_intercept.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@ print.report_intercept <- function(x, ...) {
intercept_text,
paste0(col, " = ", levels(intercept_data[[col]])[ref_level])
)
} else if (is.logical(intercept_data[[col]])) {
intercept_text <- c(intercept_text, paste0(col, " = FALSE"))
Comment thread
strengejacke marked this conversation as resolved.
Outdated
} else {
intercept_text <- c(intercept_text, paste0(col, " = [?]"))
}
Expand Down
8 changes: 8 additions & 0 deletions tests/testthat/test-report_intercept.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,12 @@ test_that("reflevel", {
as.character(report_intercept(m3)),
"The model's intercept, corresponding to f = 1, is at 0.17 (95% CI [-0.47, 0.81], t(27) = 0.55, p = 0.584)."
)

# Logical predictor
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)."
)
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
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)."
)

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member

@strengejacke strengejacke Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should simply use data(mtcars) here to re-init the data set.

})
Loading