Skip to content

feat: accept svyrep.design in the emmeans functions - #358

Open
szimmer wants to merge 1 commit into
pharmaverse:mainfrom
szimmer:355_svyrep_emmeans
Open

feat: accept svyrep.design in the emmeans functions#358
szimmer wants to merge 1 commit into
pharmaverse:mainfrom
szimmer:355_svyrep_emmeans

Conversation

@szimmer

@szimmer szimmer commented Aug 30, 2026

Copy link
Copy Markdown

What changes are proposed in this pull request?

  • ard_emmeans_emmeans() and ard_emmeans_contrast() now accept replicate-weight survey designs (svyrep.design), and construct_model() gained a svyrep.design method. Alongside the class check, both functions selected the data with dplyr::last(class(data)) == "survey.design", which does not identify a svyrep.design; the test is now inherits(data, c("survey.design", "svyrep.design")). (Feature Request: Support svyrep.design (replicate-weight) survey objects #355, @szimmer)

Provide more detail here as needed.

This is the last piece of #355, after #356 (the six summary ard_* methods) and #357 (the survey test and CI functions). It is what gtsummary::add_difference(test = "emmeans") requires on a replicate design; the default svy.t.test path is covered by #357.

Checked end to end against gtsummary, with #356 and #357 in place:

estimate 95% CI p
add_difference(test = "emmeans"), linearized -41.3 -83.6, 1.01 0.0549
add_difference(test = "emmeans"), replicate -41.3 -84.6, 1.92 0.0595
emmeans::contrast() on the replicate model -41.31902 0.0595

The replicate interval and p-value differ from the linearized design and match emmeans computed directly on the replicate model, so the replicate weights are genuinely carried through rather than silently ignored.

There were two problems here, not one. The class guard was the visible one. Underneath it, both functions selected the model data like this:

data_in <- if (dplyr::last(class(data)) == "survey.design") data$variables else data

That happens to work for survey.design2, whose last class element is "survey.design", but a svyrep.design fails the test entirely and would have fallen through to the else branch — passing the design object where a data frame is expected. Relaxing the guard alone would have produced a confusing downstream failure rather than support. The test is now inherits(data, c("survey.design", "svyrep.design")), which is also less brittle for survey.design subclasses generally.

construct_model() needed a svyrep.design method for the same sibling-class reason. It delegates to the survey.design method; the model is fit by survey::svyglm(), which already handles replicate designs.

emmeans itself needed no changes. The estimated marginal means come back with replicate-based standard errors, differing from the linearized design and matching emmeans::emmeans() applied to the replicate model directly:

sch.wide linearized SE replicate SE
No 28.98769 32.48311
Yes 23.42657 26.07184

Point estimates agree exactly across the two designs, as they must — the sampling weights are identical and only the variance estimation differs.

Tests (tests/testthat/test-svyrep.design_emmeans.R) cover both functions, construct_model(), that the standard errors are replicate-based rather than a silent fallback to linearization, and a data.frame case confirming the changed class test did not alter the non-survey path.

Reference GitHub issue associated with pull request.

Addresses #355. With #356 and #357, this completes it.


Pre-review Checklist (if item does not apply, mark is as complete)

  • All GitHub Action workflows pass with a ✅
  • PR branch has pulled the most recent updates from master branch: usethis::pr_merge_main()
  • If a bug was fixed, a unit test was added.
  • If a new ard_*() function was added, it passes the ARD structural checks from cards::check_ard_structure().
  • If a new ard_*() function was added, set_cli_abort_call() has been set.
  • If a new ard_*() function was added and it depends on another package (such as, broom), is_pkg_installed("broom") has been set in the function call and the following added to the roxygen comments: @examplesIf do.call(asNamespace("cardx")$is_pkg_installed, list(pkg = "broom""))
  • Code coverage is suitable for any new functions/features (generally, 100% coverage for new code): devtools::test_coverage()

Notes on the checklist:

  • No new exported functions are added; construct_model.svyrep.design() delegates to the survey.design method, so set_cli_abort_call() and the package checks are inherited.
  • lintr reports no findings on any changed line.

`ard_emmeans_emmeans()` and `ard_emmeans_contrast()` rejected replicate-weight
designs with a class guard. Underneath that guard sat a second problem: both
selected the model data with

    data_in <- if (dplyr::last(class(data)) == "survey.design") ...

which happens to identify `survey.design2` -- whose last class element is
"survey.design" -- but does not identify a `svyrep.design` at all. Relaxing
only the guard would have passed the design object where a data frame was
expected. The test is now `inherits(data, c("survey.design",
"svyrep.design"))`, which is also less brittle for `survey.design` subclasses
generally.

`construct_model()` needed a `svyrep.design` method for the same
sibling-class reason; it delegates to the `survey.design` method, and the
model is fit by `survey::svyglm()`, which already handles replicate designs.

emmeans itself required no changes -- the estimated marginal means come back
with replicate-based standard errors, differing from the linearized design
and matching `emmeans::emmeans()` applied to the replicate model directly:

    linearized SEs: 28.98769  23.42657
    replicate  SEs: 32.48311  26.07184

Tests cover both functions, `construct_model()`, and a `data.frame` case to
confirm the changed class test did not alter the non-survey path.

Refs pharmaverse#355

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

1 participant