Skip to content

Function name changes#241

Open
sschildhauer wants to merge 35 commits into
mainfrom
runmod_namechange
Open

Function name changes#241
sschildhauer wants to merge 35 commits into
mainfrom
runmod_namechange

Conversation

@sschildhauer

Copy link
Copy Markdown
Collaborator

Renaming functions as recommended by Kristen:
run_mod -> run_serodynamics
post_summ -> summarize_posterior
plot_jags_trace -> plot_trace
plot_jags_dens -> plot_density
plot_jags_Rhat -> plot_rhat
plot_jags_effect -> plot_ess

Will change all documentation and references throughout package.

@sschildhauer sschildhauer changed the title Runmod namechange Function name changes May 28, 2026
@claude

This comment has been minimized.

@codecov

codecov Bot commented May 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.56522% with 5 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
R/Run_Mod.R 0.00% 5 Missing ⚠️
Files with missing lines Coverage Δ
R/plot_density.R 100.00% <ø> (ø)
R/plot_ess.R 96.15% <ø> (ø)
R/plot_predicted_curve.R 98.68% <ø> (ø)
R/plot_rhat.R 96.42% <100.00%> (ø)
R/plot_trace.R 96.42% <ø> (ø)
R/prep_priors.R 87.50% <ø> (ø)
R/prep_strat_list.R 100.00% <ø> (ø)
R/rebuild_sr_model_attributes.R 100.00% <ø> (ø)
R/run_serodynamics.R 100.00% <100.00%> (ø)
R/summarize_posterior.R 100.00% <100.00%> (ø)
... and 3 more

@sschildhauer

Copy link
Copy Markdown
Collaborator Author

@claude, can you please make the recommended function name changes and change documentation and references throughout. Please update the news and version as well. Please rerun all tests with the function name changes. Keep all edits within this thread.

@claude

claude Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

Claude finished @sschildhauer's task in 13m 7s —— View job


Function Renames: Complete

Tasks:

  • Explore repo structure and current state
  • Rename post_summsummarize_posterior (source, docs, tests, snapshots)
  • Rename plot_jags_traceplot_trace (source, docs, tests, snapshots)
  • Rename plot_jags_densplot_density (source, docs, tests, snapshots)
  • Rename plot_jags_Rhatplot_rhat (source, docs, tests, snapshots)
  • Rename plot_jags_effectplot_ess (source, docs, tests, snapshots)
  • Fix lingering run_mod cross-references across R files, vignettes, pkgdown
  • Rename example file inst/examples/run_mod-examples.Rrun_serodynamics-examples.R
  • Regenerate NAMESPACE + man pages via devtools::document()
  • Update NEWS.md (new bullet noting the breaking renames)
  • Bump version in DESCRIPTION (0.0.0.9055 → 0.0.0.9056)
  • Run all tests (devtools::test()PASS 110 | FAIL 0 | WARN 0 | SKIP 0)
  • Run spelling check (added ess to inst/WORDLIST)
  • Commit and push (2e99086)

Summary

Completed all 6 function renames end-to-end (the 1 partial rename from the prior commit, plus the 5 still-pending ones):

Old name New name
run_mod run_serodynamics
post_summ summarize_posterior
plot_jags_trace plot_trace
plot_jags_dens plot_density
plot_jags_Rhat plot_rhat
plot_jags_effect plot_ess

Updates touched: R source files, inst/examples/, man/ (regenerated), NAMESPACE (regenerated), test files, snapshot directories (both base and darwin variants), pkgdown/_pkgdown.yml, vignettes/articles/getting-started.qmd, vignettes/articles/os-snapshot-analysis.qmd, R/ cross-reference doc comments (unpack_jags, use_att_names, plot_predicted_curve, prep_priors, nepal_sees_jags_output, prep_strat_list, rebuild_sr_model_attributes), .github/copilot-instructions.md, data-raw/nepal_sees_jags_output.R. NEWS.md gained a top-level bullet noting the breaking change.

devtools::test() reports 110 PASS, 0 FAIL, 0 WARN, 0 SKIP — replacing the prior 5 could not find function "run_mod" failures. spelling::spell_check_package() is clean.

Not done: No deprecation shims for the old names — per the trigger comment's scope ("make the recommended function name changes"), this is a hard rename. Downstream users of the old names will need to update their calls. If you'd prefer soft deprecation via lifecycle::deprecate_warn() aliases, let me know and I'll add them.
• Branch: runmod_namechange • Commit: 2e99086

Completes the rename initiative started for run_mod -> run_serodynamics:

  post_summ()        -> summarize_posterior()
  plot_jags_trace()  -> plot_trace()
  plot_jags_dens()   -> plot_density()
  plot_jags_Rhat()   -> plot_rhat()
  plot_jags_effect() -> plot_ess()

Source files, examples, tests, snapshot directories, NAMESPACE,
roxygen-generated .Rd pages, pkgdown reference index, and articles
all updated to the new names. NEWS.md notes the breaking change and
DESCRIPTION version bumped to 0.0.0.9056.

Co-authored-by: Samuel Hisaji Schildhauer <sschildhauer@users.noreply.github.com>
@github-actions

github-actions Bot commented May 29, 2026

Copy link
Copy Markdown
Contributor

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

@github-actions github-actions Bot requested review from Kwan-Jenny and d-morrison and removed request for Kwan-Jenny and d-morrison May 29, 2026 18:31
@github-actions github-actions Bot requested review from Kwan-Jenny and d-morrison and removed request for Kwan-Jenny and d-morrison May 29, 2026 19:24
@github-actions github-actions Bot requested review from Kwan-Jenny and d-morrison and removed request for Kwan-Jenny and d-morrison May 29, 2026 19:32
@claude

This comment has been minimized.

@claude

This comment has been minimized.

@claude

This comment has been minimized.

@claude

This comment has been minimized.

@claude

This comment has been minimized.

@sschildhauer

Copy link
Copy Markdown
Collaborator Author

@claude, please assist in generating the linux snaps from the previous review. Please also review after creating and committing changes.

@claude

claude Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Claude finished @sschildhauer's task in 5m 46s —— View job


Linux Snapshots Generated + Final Review

  • Explore current branch state and verify missing snapshot dir
  • Check JAGS availability in environment (JAGS 4.3.2, R 4.6.0)
  • Run devtools::test(filter = "run_serodynamics") → 9 new snapshot files created in tests/testthat/_snaps/run_serodynamics/
  • Re-run filter to confirm against new snapshots → PASS 26 | FAIL 0
  • Run full test suite → PASS 110 | FAIL 0 | WARN 0 | SKIP 0
  • Commit and push new snapshots → fcb0181
  • Review final state of PR

Snapshot Commit Details

Generated the missing Linux/Windows reference snapshots that commit 9491dd2 had correctly deleted (they were stale, missing the Stratification column from #240) but never replaced. Ran devtools::test(filter = "run_serodynamics") on Linux with JAGS 4.3.2 — that produced the 9 expected files. Re-running the filter against the new files reports PASS 26 | FAIL 0, and the full suite reports PASS 110 | FAIL 0 | WARN 0 | SKIP 0.

Spot-checked the new files:

File Header Notes
nostrat-fitted_residuals.csv Subject,Iso_type,Stratification,t,fitted,residual Stratification column present, value None
strat-fitted_residuals.csv Subject,Iso_type,Stratification,t,fitted,residual Stratification column present, value typhi/other
sim-strat-fitted_residuals.csv Subject,Iso_type,Stratification,t,fitted,residual Stratification column present, value stratum 1/stratum 2
nostrat-curve-params.csv Iteration,Chain,Parameter,Iso_type,Stratification,Subject,value ✅ Full schema

Sizes are within ~0.01% of the matching _snaps/darwin/run_serodynamics/ files, as expected for the floating-point divergence between Linux/Windows and macOS that motivates the variant split in the first place.

There was an unrelated tests/testthat/_snaps/as_case_data.md deletion that testthat triggered during the test run (the test now writes to the r45/ variant on R ≥ 4.5, so testthat treats the base file as orphaned on this R version). I deliberately did not stage that deletion — it would remove the snapshot baseline for R < 4.5 and is unrelated to this PR's scope. If you intentionally want to drop R < 4.5 support, do it in a separate PR.


Final Code Review

All previously flagged issues remain fixed. No new issues introduced by this snapshot commit, and no new issues found on a fresh re-read of the renamed files.

Concern Status
6 function renames consistent across R/, man/, tests, snapshots (both base & darwin), vignettes, pkgdown, copilot-instructions
run_mod() deprecation shim — lifecycle::deprecate_warn(when = "0.0.0.9060"), @seealso [run_serodynamics()], @keywords internal
Version bump in DESCRIPTION (0.0.0.9060) matches deprecation shim's when
NEWS.md documents breaking renames + deprecation shim
relationship argument set on all dplyr *_join() calls (many-to-one in R/run_serodynamics.R:185-187)
Explicit return() in run_serodynamics() and summarize_posterior()
.data pronoun usage limited to data-masking verbs (filter, mutate) — no soft-deprecated tidy-selection usage in plot_density.R, plot_ess.R, plot_rhat.R, plot_trace.R, summarize_posterior.R, run_serodynamics.R
No-op mutate(Parameter = .data$Parameter) removed from plot_ess.R / plot_rhat.R
calc_fit_mod() uses data (not dl_sub), fixes the row-count regression
man/run_mod.Rd regenerated with proper \link{run_serodynamics} cross-reference
Darwin & base fitted_residuals snapshots both include the Stratification column
Base Linux/Windows run_serodynamics snapshots present ✅ (this commit)

One small note (non-blocking): the snapshot commit adds ~50 MB of CSV reference data (1.15M lines) — large but unavoidable for the strat/nostrat curve-params files since they're the full posterior draws. The package already accepted this footprint for the darwin/ variant, so this is just symmetry, not a new cost.

CI should now go green on ubuntu-latest and windows-latest; macos-latest was already passing.

• Branch: runmod_namechange • Commit: fcb0181

These were missing on the branch (only the macOS `darwin/` variant was
committed). With no baseline snapshots, ubuntu-latest and windows-latest
CI runners saw "new snapshot" errors on all 9 assertions in
test-run_serodynamics.R.

Generated by running `devtools::test(filter = "run_serodynamics")` on
Linux with JAGS 4.3.2 and R 4.6.0; re-running the same filter against
the committed files reports PASS 26 | FAIL 0, and the full test suite
reports PASS 110 | FAIL 0.

Co-authored-by: Samuel Hisaji Schildhauer <sschildhauer@users.noreply.github.com>
@sschildhauer

Copy link
Copy Markdown
Collaborator Author

@claude, please review

@claude

claude Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Claude finished @sschildhauer's task in 2m 18s —— View job


Code Review: PR #241 — Function Name Changes

  • Gather context from PR diff and prior reviews
  • Review R source changes for correctness, style, and idiomatic patterns
  • Review tests, snapshots, and documentation
  • Verify all previously flagged issues remain addressed
  • Post review findings

Summary: Ready to Merge ✅

All previously flagged issues are fixed and CI is fully green (9 passed, 1 skipped — Claude Code Review correctly skips on bot-authored commits). The renames are consistent across R/, man/, inst/examples/, tests/, base and darwin/ snapshot directories, vignettes/, pkgdown/_pkgdown.yml, .github/copilot-instructions.md, and NEWS.md. The Linux/Windows snapshots flagged as missing in my previous review (fcb0181) are present and include the Stratification column.

Previously Flagged Issue Status
6 function renames consistent across source/man/tests/snapshots/vignettes/pkgdown
run_mod() deprecation shim — lifecycle::deprecate_warn(when = "0.0.0.9060"), @seealso [run_serodynamics()], @keywords internal
Version bump in DESCRIPTION (0.0.0.9060) matches the shim's when tag
NEWS.md documents the breaking renames + deprecation shim
relationship = "many-to-one" set on dplyr::left_join() in R/run_serodynamics.R:185-187
Explicit return() in run_serodynamics() and summarize_posterior()
.data pronoun usage limited to data-masking verbs (no soft-deprecated tidy-selection usage)
No-op mutate(Parameter = .data$Parameter) removed from plot_ess.R / plot_rhat.R
calc_fit_mod() uses data (not dl_sub), fixes the row-count regression
Stale rshape doc updates
Unused #' @importFrom lifecycle deprecated removed
man/run_mod.Rd regenerated with proper \link{run_serodynamics} cross-reference
Both base & darwin/ fitted_residuals snapshots include Stratification column
Base Linux/Windows run_serodynamics snapshots present

One Minor Pre-existing Issue (Non-blocking)

plot_trace.R:79 — typo in the collapse condition

if (sum(lengths(trace_strat_list) == 1)) {

The closing paren is misplaced relative to the sibling functions. This evaluates lengths(trace_strat_list) == 1 (a logical vector marking which sub-lists have exactly one plot), then sum() of that vector is truthy whenever any sub-list has a single plot — not when the total list contains exactly one plot.

Compare the sibling plot functions, which both have the parentheses in the intended position:

  • plot_rhat.R:82: if (sum(lengths(rhat_strat_list)) == 1)
  • plot_ess.R:81: if (sum(lengths(eff_strat_list)) == 1)

This is pre-existing — it survived the rename unchanged (the file is a +9/-9 rename) — so it's out of scope for this PR. I'd recommend a follow-up to fix it; in practice the bug means a multi-strat trace plot call where one of the strata happens to have one isotype's plot collapses to that single plot rather than returning the full nested list.

Fix in a follow-up PR →


Spot Checks That Came Up Clean

  • R/run_serodynamics.R:127dplyr::filter(.data[[strat]] == i) uses .data[[strat]] inside a data-masking verb (filter), which is correct. (.data[[var]] is only soft-deprecated in tidy-selection contexts.)
  • R/Run_Mod.R:10-18 — shim correctly forwards ... to run_serodynamics() after emitting the deprecation; the lifecycle helper handles the Defunct transition timing.
  • NAMESPACE exports both run_mod (for the deprecation shim) and run_serodynamics; all five plot renames and summarize_posterior are exported and the old names are absent.
  • man/figures/lifecycle-*.svg files are added as required by lifecycle::badge("deprecated") rendering in man/run_mod.Rd.
  • pkgdown/_pkgdown.yml lists only the new names in reference: — the deprecated run_mod is correctly absent from the index (it'll still appear in the function list under "internal" because of @keywords internal, which is what we want).
  • tests/testthat/test-run_serodynamics.R correctly references run_serodynamics() directly (not via the shim), avoiding the deprecation warning under lifecycle_verbosity = "error".
  • tests/testthat/_snaps/run_serodynamics/ directory present with 9 CSV files; all fitted_residuals*.csv snapshots checked for the Stratification column (per the Debug calcfitmod #240 fix).

LGTM — approve and merge whenever. The plot_trace.R typo is a minor pre-existing issue that should be addressed in a separate PR to keep this rename focused.

• Branch: runmod_namechange

@sschildhauer

Copy link
Copy Markdown
Collaborator Author

Hi @d-morrison, I believe that this PR is ready for merge. I created an issue for the small parenthesis issue in plot_trace. Thank you for taking a look.

@sschildhauer sschildhauer requested a review from d-morrison June 17, 2026 23:46
@d-morrison

Copy link
Copy Markdown
Member

@sschildhauer unfortunately we got some merge conflicts (I think from merging #207 just now); please resolve

@d-morrison d-morrison removed their request for review June 18, 2026 17:01
Merge commit '91c7b091e601e9ba4acb2d51ca6b59aa4630aef6'

#Conflicts:
#	DESCRIPTION
#	NEWS.md
#	inst/WORDLIST
#	inst/examples/run_mod-examples.R
#	man/run_mod.Rd
#	tests/testthat/_snaps/darwin/run_mod/nostrat-curve-params-specpriors.csv
#	tests/testthat/_snaps/darwin/run_mod/nostrat-curve-params.csv
#	tests/testthat/_snaps/darwin/run_mod/sim-strat-curve-params.csv
#	tests/testthat/_snaps/darwin/run_mod/strat-curve-params.csv
#	tests/testthat/test-plot_jags_densitydx.R
#	tests/testthat/test-plot_jags_effectivedx.R
#	tests/testthat/test-plot_jags_rhatdx.R
#	tests/testthat/test-plot_jags_tracedx.R
#	tests/testthat/test-run_mod.R
@claude

This comment has been minimized.

sschildhauer and others added 3 commits June 19, 2026 15:42
The merge with main brought in old test files referencing functions that
were renamed in this PR. These stale tests would fail with "could not
find function" under RUN_HEAVY_TESTS=true.

Also removed test-run_mod.R: its heavy tests call the deprecated run_mod()
shim, which emits lifecycle::deprecate_warn(). With lifecycle_verbosity =
"error" (set in tests/testthat/setup.R), the deprecation becomes an error
that suppressWarnings() cannot catch. The underlying functionality is fully
covered by test-run_serodynamics.R.

Also fixed double-asterisk typo in NEWS.md bullet ("* *" -> "*").

Co-authored-by: sschildhauer <sschildhauer@ucdavis.edu>
@claude

claude Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Claude finished @sschildhauer's task in 2m 24s —— View job


Code Review: PR #241 — Function Name Changes

  • Gather context: check current branch state and diff vs origin/main
  • Review R source changes for correctness and idiomatic patterns
  • Review merge conflict resolution quality
  • Review tests, snapshots, and documentation
  • Verify all previously flagged issues remain addressed
  • Post review findings

Summary: Ready to Merge ✅

The merge conflict with main (from #207) was resolved correctly in commits e6b2f04 and e47d324. The stale old-name test files (test-run_mod.R, test-plot_jags_*.R) were removed — the replacement files under the new names already exist and cover the same tests. All previously flagged issues from prior reviews remain fixed.

Version was bumped to 0.1.0.9001 in DESCRIPTION, which is correct for a dev version after the 0.1.0 CRAN release.


Previously Reported Issues: All Fixed ✅

Issue Status
6 function renames consistent across source/man/tests/snapshots/vignettes/pkgdown
run_mod() deprecation shim — lifecycle::deprecate_warn(when = "0.0.0.9060"), @seealso [run_serodynamics()], @keywords internal
Version in DESCRIPTION bumped (now 0.1.0.9001)
NEWS.md documents the breaking renames + deprecation shim
relationship = "many-to-one" set on dplyr::left_join() in R/run_serodynamics.R:185-187
Explicit return() in run_serodynamics() and summarize_posterior()
.data pronoun limited to data-masking verbs — no soft-deprecated tidy-selection usage
No-op mutate(Parameter = .data$Parameter) removed from plot_ess.R / plot_rhat.R
calc_fit_mod() uses data (not dl_sub), fixes the row-count regression
Unused #' @importFrom lifecycle deprecated removed
man/run_mod.Rd regenerated with proper \link{run_serodynamics} cross-reference
Both base & darwin/ fitted_residuals snapshots include Stratification column
Linux/Windows run_serodynamics snapshots (9 files) present
Stale old-name test files removed after merge with main

Merge Conflict Resolution (New in This Sync)

Commits e6b2f04/e47d324 resolved the conflict from #207 cleanly:

  • Removed tests/testthat/test-run_mod.R, test-plot_jags_densitydx.R, test-plot_jags_effectivedx.R, test-plot_jags_rhatdx.R, test-plot_jags_tracedx.R — these were the old-name test files superseded by the renamed counterparts already on this branch.
  • NEWS.md conflict resolved by retaining the ## New features rename bullet at the top of the development section.

These resolutions are correct.


Pre-existing Issue (Non-blocking, Tracked Separately)

plot_trace.R:79 still has the misplaced closing paren noted in the previous review:

# Current (wrong):
if (sum(lengths(trace_strat_list) == 1)) {

# Should match siblings plot_rhat.R:82 and plot_ess.R:81:
if (sum(lengths(trace_strat_list)) == 1) {

This was pre-existing before this PR and is tracked in a separate issue — out of scope here.


Spot Checks

  • NAMESPACE: old plot names absent; plot_density, plot_ess, plot_rhat, plot_trace, run_serodynamics, summarize_posterior all exported; run_mod retained for the deprecation shim. ✅
  • tests/testthat/_snaps/run_serodynamics/: all 9 CSV files present, fitted_residuals headers include Stratification column. ✅
  • R/run_serodynamics.R:127: .data[[strat]] correctly used inside dplyr::filter() (data-masking context, not selection). ✅
  • R/Run_Mod.R: deprecation shim correctly forwards ... to run_serodynamics() with lifecycle::deprecate_warn(when = "0.0.0.9060"). The when version predates 0.1.0.9001 — semantically correct (records when first deprecated). ✅

LGTM — approve and merge whenever.
• Branch: runmod_namechange

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.

Change run_mod name to srm()

3 participants