-
-
Notifications
You must be signed in to change notification settings - Fork 22
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.
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
Fix snapshot differences (it may need muting h_ppmeans()
)
#1072
Conversation
This is not used in BAC AFAIK 👍🏼 |
Code Coverage Summary
Diff against main
Results for commit: f9c5826 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
In TLG-C, See context: ...
) %>%
summarize_glm_count(
vars = "AVAL",
variables = list(arm = "ARM", offset = "lgTMATRSK", covariates = NULL),
conf_level = 0.95,
distribution = "poisson",
rate_mean_method = "emmeans",
var_labels = "Unadjusted exacerbation rate (per year)",
table_names = "unadj",
.stats = c("rate"),
.labels = c(rate = "Rate")
) %>%
summarize_glm_count(
vars = "AVAL",
variables = list(arm = "ARM", offset = "lgTMATRSK", covariates = c("REGION1")),
conf_level = 0.95,
distribution = "quasipoisson",
rate_mean_method = "ppmeans",
var_labels = "Adjusted (QP) exacerbation rate (per year)",
table_names = "adj",
.stats = c("rate", "rate_ci", "rate_ratio", "rate_ratio_ci", "pval"),
.labels = c(
rate = "Rate", rate_ci = "Rate CI", rate_ratio = "Rate Ratio",
rate_ratio_ci = "Rate Ratio CI", pval = "p value"
)
) |
@@ -186,6 +187,7 @@ testthat::test_that("h_ppmeans works with healthy input", { | |||
|
|||
testthat::expect_snapshot(fits2) | |||
|
|||
# XXX ppmeans fails snapshot diff in integration tests | |||
result <- h_ppmeans( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keeping this failing to see if set.seed helps and if also AVR is triggered by this change
Fixes #1058
The error comes ONLY from integration tests (mismatch of output rates and rate CI in
h_ppmeans()
). @ayogasekaram tested on docker image, @edelarua and I tested it (I think window-based rstudio and Ubuntu rstudio-server) and there are no differences in our snapshots, i.e. it is passing for all of us. Now I think this needs to come fromstats::predict
step inh_ppmeans
as also rates are different. It seems we tested everything (adding random seed does not change my local snapshots). Could it have something to do with the system libraries? what do you think @cicdguy? If we cannot fix it, and it is not a fundamental feature we might want to turn it off. Is this function used in the biomarker catalog @danielinteractive? To my knowledge, it is not used in the TLG-c, right @ayogasekaram @edelarua?Please refer to #1059 and integration tests for the changes that exclude the fit itself from having any changes local vs integration tests.