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

allow more statistics in cfbt01 template #552

Merged
merged 15 commits into from
Jul 10, 2023
Merged

Conversation

clarkliming
Copy link
Contributor

close #551

I feel that cfbt01 is becoming slower....

@github-actions
Copy link
Contributor

github-actions bot commented Jun 21, 2023

🧪 $Test coverage: 97.70%$

Code Coverage Summary

Filename                     Stmts    Miss  Cover    Missing
-------------------------  -------  ------  -------  -------------------------------------------------
R/ael01_nollt.R                 17       1  94.12%   61
R/aet01_aesi.R                 149       1  99.33%   211
R/aet01.R                       94       1  98.94%   158
R/aet02.R                       60       0  100.00%
R/aet03.R                       77       0  100.00%
R/aet04.R                       89       0  100.00%
R/aet05_all.R                   11       0  100.00%
R/aet05.R                       36       1  97.22%   101
R/aet10.R                       43       0  100.00%
R/assertions.R                  99       6  93.94%   88-93
R/cfbt01.R                     104       0  100.00%
R/checks.R                      14       0  100.00%
R/chevron_tlg-S4class.R         18       0  100.00%
R/chevron_tlg-S4methods.R      103       0  100.00%
R/cmt01a.R                      77       0  100.00%
R/coxt01.R                      48       1  97.92%   126
R/dmt01.R                       27       0  100.00%
R/dst01.R                       95       0  100.00%
R/dtht01.R                     103       6  94.17%   48, 52-56
R/egt02.R                       37       0  100.00%
R/egt03.R                       61       1  98.36%   127
R/egt05_qtcat.R                 64       0  100.00%
R/ext01.R                       61       1  98.36%   40
R/fstg01.R                      42       1  97.62%   95
R/kmg01.R                       28       1  96.43%   69
R/lbt04.R                       69       0  100.00%
R/lbt05.R                       67       5  92.54%   125-130
R/lbt06.R                       63       3  95.24%   132-135
R/lbt07.R                       94       0  100.00%
R/lbt14.R                       57       0  100.00%
R/mht01.R                       72       0  100.00%
R/mng01.R                       97       1  98.97%   110
R/pdt01.R                       61       0  100.00%
R/pdt02.R                       69       0  100.00%
R/rmpt01.R                      65      11  83.08%   91-100, 143
R/rspt01.R                      73       3  95.89%   156-159
R/rtables_utils.R              246      17  93.09%   41, 98, 210, 230, 416, 431-433, 435, 453-459, 469
R/standard_rules.R              11       0  100.00%
R/ttet01.R                     129       3  97.67%   229-232
R/utils.R                       54       0  100.00%
R/vst02.R                       47       1  97.87%   107
TOTAL                         2831      65  97.70%

Diff against main

Filename             Stmts    Miss  Cover
-----------------  -------  ------  --------
R/cfbt01.R              +8       0  +100.00%
R/rtables_utils.R       -5       0  -0.14%
R/utils.R               +4       0  +100.00%
TOTAL                   +7       0  +0.01%

Results for commit: a4831a6f59cb441b7683a563330a74992ae1e34a

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

@github-actions
Copy link
Contributor

github-actions bot commented Jun 21, 2023

Unit Tests Summary

    1 files    49 suites   3m 14s ⏱️
214 tests 157 ✔️   57 💤 0
432 runs  299 ✔️ 133 💤 0

Results for commit 958c23a.

♻️ This comment has been updated with latest results.

R/rtables_utils.R Outdated Show resolved Hide resolved
BFalquet
BFalquet previously approved these changes Jun 28, 2023
Copy link
Contributor

@BFalquet BFalquet left a comment

Choose a reason for hiding this comment

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

Seems to work for me, although the function is a bit slow, (it was already slow before). Very small comments inside. Thank you for the work. Any comment @barnett11

R/rtables_utils.R Outdated Show resolved Hide resolved
R/cfbt01.R Outdated Show resolved Hide resolved
clarkliming and others added 3 commits June 29, 2023 10:15
Co-authored-by: b_falquet <64274616+BFalquet@users.noreply.github.com>
Signed-off-by: Liming <36079400+clarkliming@users.noreply.github.com>
Co-authored-by: b_falquet <64274616+BFalquet@users.noreply.github.com>
Signed-off-by: Liming <36079400+clarkliming@users.noreply.github.com>
Co-authored-by: b_falquet <64274616+BFalquet@users.noreply.github.com>
Signed-off-by: Liming <36079400+clarkliming@users.noreply.github.com>
@clarkliming
Copy link
Contributor Author

clarkliming commented Jun 29, 2023

blocked by insightsengineering/tern#983

update: this PR won't be merged in the up coming release so let's do without that PR

@barnett11
Copy link
Contributor

Extra summary statistics seems to be working well, but I'm getting an error if I try to select summaryvars to be just AVAL - a feature of this table should be to have the below,
"Allow user to specify whether display value at visit only, change from baseline only or both value at visit and change from baseline"

@clarkliming
Copy link
Contributor Author

@barnett11 it is now fixed, could you please have a look?

@barnett11
Copy link
Contributor

@barnett11 it is now fixed, could you please have a look?

Looks perfect thanks!

@BFalquet
Copy link
Contributor

BFalquet commented Jul 7, 2023

I ll wait for insightsengineering/tern#983 to review since it might impact us.

@clarkliming
Copy link
Contributor Author

I ll wait for insightsengineering/tern#983 to review since it might impact us.

let's proceed with this because that PR won't be merged in the next release (happenning in sprint 3; date confirmed)

Copy link
Contributor

@BFalquet BFalquet left a comment

Choose a reason for hiding this comment

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

Look good to me, I am pleased to see that the baseline and screening are back in the snapshot

R/cfbt01.R Show resolved Hide resolved
@clarkliming
Copy link
Contributor Author

Look good to me, I am pleased to see that the baseline and screening are back in the snapshot

baseline are removed in the snapshot, screening are not (can be removed through skip)

@clarkliming clarkliming merged commit c6bc06a into main Jul 10, 2023
24 checks passed
@clarkliming clarkliming deleted the 551_statistics_cfbt@main branch July 10, 2023 02:32
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.

allow more statistics in cfbt01
3 participants