Skip to content

trigger test from yurovska/tern/tree/821_ancova_weights #192

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

Merged
merged 14 commits into from
Apr 7, 2025

Conversation

shajoezhu
Copy link
Contributor

No description provided.

Signed-off-by: Joe Zhu <[email protected]>
Copy link
Contributor

github-actions bot commented Mar 29, 2025

Unit Tests Summary

  1 files  112 suites   3m 32s ⏱️
249 tests 249 ✅ 0 💤 0 ❌
530 runs  530 ✅ 0 💤 0 ❌

Results for commit 85b053c.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Mar 29, 2025

Unit Test Performance Difference

Test Suite $Status$ Time on main $±Time$ $±Tests$ $±Skipped$ $±Failures$ $±Errors$
char-support 👶 $+2.92$ $+6$ $0$ $0$ $0$
Additional test case details
Test Suite $Status$ Time on main $±Time$ Test Case
char-support 👶 $+1.17$ CRITDIR_is_factor_and_one_PARAMCD_is_not_available_on_all_AVISITS
char-support 👶 $+0.90$ CRITDIR_is_not_factor_and_all_PARAMCD_available_on_all_AVISITS
char-support 👶 $+0.85$ CRITDIR_is_not_factor_and_one_PARAMCD_is_not_available_on_all_AVISITS
table_aovt01 👶 $+0.74$ AOVT01_variant_with_proportional_weights_is_produced_correctly

Results for commit 585b351

♻️ This comment has been updated with latest results.

@yurovska
Copy link

@shajoezhu Since I have no permission to commit in this branch, could you please add the following test in tests/testthat/test-table_aovt01.R:

testthat::test_that("AOVT01 variant with proportional weights is produced correctly", {
  adqs_multi <- dplyr::filter(adqs, AVISIT == "WEEK 1 DAY 8")
  n_per_arm <- table(adsl$ARM)

  result <- basic_table() %>%
    split_cols_by("ARMCD", ref_group = "ARM A", split_fun = ref_group_position("first")) %>%
    add_colcounts() %>%
    split_rows_by("PARAMCD") %>%
    summarize_ancova(
      vars = "CHG",
      variables = list(arm = "ARMCD", covariates = c("BASE", "STRATA1")),
      conf_level = 0.95, var_labels = "Adjusted mean",
      weights_emmeans = "proportional"
    ) %>%
    build_table(adqs_multi, alt_counts_df = adsl)

  res <- testthat::expect_silent(result)
  testthat::expect_snapshot(res)
})

@shajoezhu
Copy link
Contributor Author

thanks guys! @Melkiades @yurovska

@shajoezhu shajoezhu assigned shajoezhu and unassigned shajoezhu Apr 7, 2025
@shajoezhu shajoezhu added the sme label Apr 7, 2025
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

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

Files not reviewed (8)
  • .lintr: Language not supported
  • DESCRIPTION: Language not supported
  • tests/testthat/test-char-support.R: Language not supported
  • tests/testthat/test-listing_adal02.R: Language not supported
  • tests/testthat/test-table_aet04.R: Language not supported
  • tests/testthat/test-table_aovt01.R: Language not supported
  • tests/testthat/test-table_pkpt04.R: Language not supported
  • tests/testthat/test-table_pkpt05.R: Language not supported
Comments suppressed due to low confidence (2)

tests/testthat/_snaps/table_aovt01.md:73

  • The horizontal divider uses non-standard dash characters; consider using standard hyphen characters for clarity and consistency.
      ——————————————————————————————————————————————————————————————————————————

tests/testthat/_snaps/char-support.md:17

  • [nitpick] Ensure consistent naming for heart rate labels; consider using 'HR' consistently across all instances instead of mixing 'HR' and 'Heart Rate'.
                  HIGH Heart Rate       5 (3.7%)      3 (2.2%)        8 (6.1%)

Copy link
Contributor

@Melkiades Melkiades left a comment

Choose a reason for hiding this comment

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

Lgtm! Thanks for adding also the breaking change snapshot @shajoezhu. Could you only check this?

tests/testthat/_snaps/table_aovt01.md:73

The horizontal divider uses non-standard dash characters; consider using standard hyphen characters for clarity and consistency.

Copy link
Contributor

@Melkiades Melkiades left a comment

Choose a reason for hiding this comment

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

Actually good to go. I know we replace the hyphen with the generic "-" in the CRAN packages but here it is not necessary!

@shajoezhu shajoezhu merged commit 666f807 into main Apr 7, 2025
29 checks passed
@shajoezhu shajoezhu deleted the shajoezhu-patch-1 branch April 7, 2025 07:59
@github-actions github-actions bot locked and limited conversation to collaborators Apr 7, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants