Skip to content
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

Create analyze_num_patients and correct ae* tables #776

Merged
merged 35 commits into from
Dec 23, 2022

Conversation

Melkiades
Copy link
Contributor

@Melkiades Melkiades commented Dec 16, 2022

Fixes #748

Note: this was done primarily for pagination issues (new tests target this precisely, see test_summarize_num_patients). Concurrently, also sorting is tested.

This PR must be followed by a couple of fixes and enhancements, listed here:

The last point involves (currently) the following templates:

  • aet02
  • aet02_smq
  • aet_03
  • aet_06_smq
  • aet_09
  • aet_09_smq
  • mth01
  • cmt01 (indentation is shifted to the right in second split -> different from other tables, to check)
  • aet07
  • aet06 (shifted indentation as in cmt01 but only for variant 1. Variant 3 and 5 have normal indentation)

All of the above-mentioned have in some way or another a possibly breaking pagination due to using summarize_row_groups before the split so that it would end wrongfully on all the pages.

@ayogasekaram fyi this PR contains also an enhancement for the function to_string_matrix with a handy way to copy-paste the result matrix (this is intended for a more general and practical fix of #784 and future uses.

@Melkiades Melkiades marked this pull request as draft December 16, 2022 12:14
@Melkiades Melkiades marked this pull request as ready for review December 21, 2022 11:19
@github-actions
Copy link
Contributor

github-actions bot commented Dec 21, 2022

Unit Tests Summary

       1 files     127 suites   2m 35s ⏱️
   873 tests    873 ✔️ 0 💤 0
1 323 runs  1 323 ✔️ 0 💤 0

Results for commit fffa171.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@ayogasekaram ayogasekaram left a comment

Choose a reason for hiding this comment

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

lgtm @Melkiades

@Melkiades Melkiades merged commit 2cbb6e4 into main Dec 23, 2022
@Melkiades Melkiades deleted the 748_create_analyze_num_patients@main branch December 23, 2022 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

creating analyze function for summarize_num_patients
2 participants