Fix double periods, spaces, prevent c++ errors with Stan flags, update snapshots, etc. - #573
Conversation
…e snapshots, etc.
There was a problem hiding this comment.
Pull Request Overview
This PR focuses on improving text formatting and fixing compilation issues in the report package tests and functions. The main goals are to clean up double periods/spaces in generated text, prevent C++ compilation errors from Stan flags, and update test snapshots.
- Removes redundant library loading and duplicate skip statements in test files
- Adds a new setup file to neutralize C/C++ compilation flags that could interfere with tests
- Improves text formatting logic in report functions to prevent double periods and extra spaces
Reviewed Changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/testthat/setup-00-toolchain.R | New setup file to neutralize C/C++ compilation flags and prevent Stan-related compilation errors |
| R/report.lm.R | Enhanced text formatting with helper functions to prevent double periods and extra spaces |
| R/report.brmsfit.R | Fixed double period issue in prior descriptions |
| tests/testthat/test-report.lm.R | Improved code formatting by breaking long lines into multiple lines |
| tests/testthat/test-report.brmsfit.R | Removed duplicate skip statements and improved code formatting |
| tests/testthat/test-report.survreg.R | Removed unnecessary namespace unloading |
| tests/testthat/test-report.estimate_contrasts.R | Removed redundant library loading |
| tests/testthat/test-report.BFBayesFactor.R | Removed redundant library loading |
| sep_after <- function(x) { | ||
| x <- trimws(x) | ||
| if (!nzchar(x)) { | ||
| return("") | ||
| } # nothing to add if empty | ||
| if (grepl("[.!?]\\s*$", x)) " " else ". " # space if already ends with .!?; else ". " | ||
| } |
There was a problem hiding this comment.
The helper function sep_after lacks documentation. Add a docstring explaining its purpose: to add appropriate spacing after text fragments, returning a single space if the text already ends with punctuation, or '. ' otherwise.
| # message("FLAGS after cleanup:\n", | ||
| # paste(sprintf("%s='%s'", vars, Sys.getenv(vars)), collapse="\n")) |
There was a problem hiding this comment.
[nitpick] Consider removing the commented debug code or moving it to a conditional block that can be enabled via an environment variable for debugging purposes, rather than leaving it as commented code.
| # message("FLAGS after cleanup:\n", | |
| # paste(sprintf("%s='%s'", vars, Sys.getenv(vars)), collapse="\n")) | |
| if (identical(Sys.getenv("DEBUG_TOOLCHAIN_SETUP"), "1")) { | |
| message("FLAGS after cleanup:\n", | |
| paste(sprintf("%s='%s'", vars, Sys.getenv(vars)), collapse="\n")) | |
| } |
Fix double periods, spaces, prevent c++ errors with Stan flags, update snapshots, etc.