Skip to content

Creating a default print for sr_model#114

Open
sschildhauer wants to merge 127 commits into
mainfrom
default_print
Open

Creating a default print for sr_model#114
sschildhauer wants to merge 127 commits into
mainfrom
default_print

Conversation

@sschildhauer

@sschildhauer sschildhauer commented Jul 9, 2025

Copy link
Copy Markdown
Collaborator

Three CI failures in the print.sr_model PR: tidyselect deprecation warnings from .data in non-data-masking contexts, Rd files out of sync with source, and a missing snapshot for the unstratified test case.

Changes

  • Fix .data in tidyselect contexts.by, names_from, and values_from are tidyselect args, not data-masking; replaced with string column names:

    # Before
    dplyr::summarise(.by = c(.data$Stratification, .data$Iso_type, .data$Parameter), ...)
    tidyr::pivot_wider(names_from = .data$Parameter, values_from = .data$median_val)
    
    # After
    dplyr::summarise(.by = c("Stratification", "Iso_type", "Parameter"), ...)
    tidyr::pivot_wider(names_from = "Parameter", values_from = "median_val")
  • Regenerate documentation — ran devtools::document() to sync man/print.sr_model.Rd and four other Rd files that had drifted from source; also fixed [dplyr::tbl_df][tibble::tbl_df] in the print_tbl param doc.

  • Add missing snapshot — added the Linux/non-Darwin snapshot for the unstratified sr_model print test (the macOS variant will be generated by CI on the first passing run).

@codecov

codecov Bot commented Jul 9, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.73684% with 1 line in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
R/print.sr_model.R 94.73% 1 Missing ⚠️
Files with missing lines Coverage Δ
R/Run_Mod.R 100.00% <ø> (ø)
R/expect_snapshot_data.R 100.00% <ø> (ø)
R/print.sr_model.R 94.73% <94.73%> (ø)

@github-actions

github-actions Bot commented Jul 9, 2025

Copy link
Copy Markdown
Contributor

📖 https://ucd-serg.github.io/serodynamics/preview/pr114
Preview documentation for this PR (at commit 7888183)

@sschildhauer

Copy link
Copy Markdown
Collaborator Author

I am a little confused with the warning/error I am getting here. It looks like there is a scoping issue relating to the base dataset when using dplyr, but when I fix the issue the tests are producing a warning. It seems like the solution might be in conflicting with each other. Grateful for any help!

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 8, 2026 14:25
@sschildhauer

Copy link
Copy Markdown
Collaborator Author

@copilot apply changes based on the comments in this thread

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.

Comment thread tests/testthat/test-print.sr_model.R Outdated
Comment thread tests/testthat/_snaps/print.sr_model.md Outdated
Comment thread tests/testthat/_snaps/darwin/print.sr_model.md
Comment thread R/print.sr_model.R

Copilot AI commented May 8, 2026

Copy link
Copy Markdown
Contributor

@copilot apply changes based on the comments in this thread

Applied in commit 6384e0c:

  • Removed suppressWarnings() from around pivot_wider() entirely — confirmed it produces no warnings when called on properly summarised data (each Parameter value appears exactly once per Stratification/Iso_type combination). The original warnings being suppressed were the tidyselect .data deprecation warnings, which were already fixed in a prior commit.
  • Updated the snapshot for the "no strat" test to match the test code that now calls print(results, print_tbl = TRUE).

The pipeline in print.sr_model() is now a clean chain with no warning suppression needed.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 8, 2026 18:15
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.

Comment thread man/print.sr_model.Rd Outdated
Comment thread man/print.sr_model.Rd Outdated
Comment thread tests/testthat/test-print.sr_model.R
Comment thread tests/testthat/_snaps/print.sr_model.md Outdated
Comment thread tests/testthat/_snaps/darwin/print.sr_model.md Outdated
@sschildhauer

Copy link
Copy Markdown
Collaborator Author

@copilot apply changes based on the comments in this thread

Copilot AI review requested due to automatic review settings May 8, 2026 20:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants