335 differential precision (chevron) #343

BFalquet · 2022-12-16T11:53:51Z

close #335
This is a proof of concept applied to lbt01

add precision arguments
put a blank in baseline CHG cell

thank you for your suggestions

github-actions · 2022-12-16T12:06:05Z

Unit Tests Summary

    1 files   19 suites 1m 5s ⏱️
  93 tests   64 ✔️ 29 💤 0 ❌
209 runs 137 ✔️ 72 💤 0 ❌

Results for commit 8fde04e.

♻️ This comment has been updated with latest results.

clarkliming · 2022-12-17T11:59:15Z

R/lbt01.R

+        n_fmt <- "xx"
+        mean_sd_fmt <- sprintf("xx%s (xx%s)", xn, xn)
+        median_fmt <- sprintf("xx%s", xn)
+        min_max_fmt <- "xx.x - xx.x"


for min_max format, I think we usually have one decimal points less than the mean/sd precision, e.g. for mean/sd we have xx.xx, for min-max, we have xx.x. What do you think @barnett11 ?

Yes I agree thanks @clarkliming -
3.1. Measures of location and dispersion (e.g., mean, median, variance, SD, SE, and corresponding confidence intervals) are displayed with 1 more decimal place of precision than the most precise collected data.
3.2. Percentiles (eg, Q1, Q3, 99th percentile) are displayed with 1 more decimal place of precision than the most precise collected data.
3.3. When data are summarized in the same unit as collected, the minimum and the maximum are displayed with the same number of decimal places of precision as the most precise collected data.

clarkliming · 2022-12-19T08:07:16Z

R/lbt01.R

@@ -94,7 +103,50 @@ lbt01_1_lyt <- function(armvar,
      varlabels = summaryvars_lbls,
      nested = TRUE
    ) %>%
-    summarize_colvars() %>%
+    analyze_colvars(


we may need to abstract this function into tern some time later so that other templates can also use this

clarkliming · 2022-12-19T08:08:12Z

@barnett11 could you please help review this output, where change from baseline for baseline is cleaned up, and precisions are supported

barnett11

Am getting some odd results,
Precision of 2 - is that for the stats or the incoming value? So if I select 2, I should see mean to 3dp? And Min-Max to 2? Am not seeing it currently

Also issue if set to 0 dp,

The Min-Max is causing warnings

Should precision values be based on PARAMCD as easier to write?

clarkliming · 2022-12-20T08:14:05Z

I agree that we can use PARAMCD to input the precisions, so we will need split_rows_by(var = "PARAMCD", labels_var = "PARAM"). This can be much easier for precisions.

BFalquet · 2023-01-04T08:00:25Z

New changes;

the precision are now given for each value of "PARAMCD" to make the writing more concise-
the precision provided correspond to the numbers of digits presented in min/max/median. mean and sd have automatically one more digit of precision
new helper functions are used to create the format with an arbitrary precision
number of digits is not limited to 3 anymore.
test of the helper functions have been added.

thank you for your review

github-actions · 2023-01-04T13:21:20Z

🧪 $Test coverage: 89.58%$

Code Coverage Summary

Filename                     Stmts    Miss  Cover    Missing
-------------------------  -------  ------  -------  ---------------------------------
R/aet01.R                      330       6  98.18%   38, 240, 253, 316, 322, 554
R/aet02.R                      222       2  99.10%   136, 489
R/aet03.R                       85       0  100.00%
R/aet04.R                      104       6  94.23%   81-86
R/assertions.R                  36       0  100.00%
R/checks.R                      14       0  100.00%
R/chevron_tlg-S4class.R         21       0  100.00%
R/chevron_tlg-S4methods.R       42      16  61.90%   47-55, 90-98, 218-290
R/cmt01a.R                     191       0  100.00%
R/cmt02_pt.R                    52       0  100.00%
R/dmt01.R                       40       0  100.00%
R/dst01.R                      287       0  100.00%
R/dtht01.R                     104       0  100.00%
R/egt01.R                       46       0  100.00%
R/egt02.R                       60       0  100.00%
R/egt03.R                      129      85  34.11%   32-52, 111, 125-232, 291, 305-339
R/ext01.R                       81       8  90.12%   238-241, 245-248
R/gen_args.R                     1       1  0.00%    27
R/lbt01.R                       84       0  100.00%
R/lbt04.R                       52       1  98.08%   127
R/mht01.R                       72       2  97.22%   33-34
R/mng01.R                       93       9  90.32%   113, 117-120, 133-134, 139, 182
R/pdt01.R                       61      38  37.70%   35-52, 108-150
R/utils.R                      138      83  39.86%   70, 83-185
R/vst01.R                       48       0  100.00%
R/vst02.R                      102       3  97.06%   42, 120, 257
TOTAL                         2495     260  89.58%

Results for commit: c60c4208a9da4f5448009825abbbf9a3ab2ab493

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

R/utils.R

R/lbt01.R

R/utils.R

R/lbt01.R

R/utils.R

clarkliming · 2023-01-11T01:20:53Z

I think this output looks great. @barnett11 can you have a look?

barnett11

I still need to review the precision implementation, but wanted to quickly raise default behaviour issue. If no values are collected (n=0) then in CENSORED we see the below,

but currently in chevron we get warnings and the below,

clarkliming · 2023-01-11T14:18:54Z

I still need to review the precision implementation, but wanted to quickly raise default behaviour issue. If no values are collected (n=0) then in CENSORED we see the below, but currently in chevron we get warnings and the below,

I believe this is a common issue, and we can open new issues to track that. What do you think about this issue @shajoezhu ?

close #341 other todo in code: 1. change from baseline for baseline, tackled in #343 2. check grades in mapping, tackled in #324

barnett11 · 2023-01-12T12:26:32Z

This is looking really great thanks @BFalquet and @clarkliming . An observation and a suggestion,
Observation:
I'm not sure if it's an application of the precision, but for my data I have a value of 6.5. If I run it through lbt01_1 with precision of 0 it is being viewed in Min/Max as value 6, but I would expect 7?

Suggestion:
CENSORED currently supplies recommended precisions for the majority of standard Labs we observe in our studies - given the variability we see across tests is this something we could implement also in citril.metadata?

barnett11 · 2023-01-12T12:47:31Z

This is looking really great thanks @BFalquet and @clarkliming . An observation and a suggestion, Observation: I'm not sure if it's an application of the precision, but for my data I have a value of 6.5. If I run it through lbt01_1 with precision of 0 it is being viewed in Min/Max as value 6, but I would expect 7?

Suggestion: CENSORED currently supplies recommended precisions for the majority of standard Labs we observe in our studies - given the variability we see across tests is this something we could implement also in citril.metadata?

I've just realised this is due to the different nature of roundings between R and SAS - do we have/want a convention for this?

clarkliming · 2023-01-13T02:05:38Z

This is looking really great thanks @BFalquet and @clarkliming . An observation and a suggestion, Observation: I'm not sure if it's an application of the precision, but for my data I have a value of 6.5. If I run it through lbt01_1 with precision of 0 it is being viewed in Min/Max as value 6, but I would expect 7?
Suggestion: CENSORED currently supplies recommended precisions for the majority of standard Labs we observe in our studies - given the variability we see across tests is this something we could implement also in citril.metadata?

I've just realised this is due to the different nature of roundings between R and SAS - do we have/want a convention for this?

yes SAS and R have different conventions of rounding, R uses bankers rounding while SAS does not. This bankers rounding is selected as standard, see IEEE 754; I think we should not change this

Signed-off-by: Liming <[email protected]>

clarkliming

I think we can merge this for now, and open new issues of test; in addition, the spotted issue of rounding and NA when 0 records available can be tracked in other issue

shajoezhu · 2023-01-13T04:38:59Z

I still need to review the precision implementation, but wanted to quickly raise default behaviour issue. If no values are collected (n=0) then in CENSORED we see the below, but currently in chevron we get warnings and the below,

I believe this is a common issue, and we can open new issues to track that. What do you think about this issue @shajoezhu ?

I think this will be common, perhaps we can create improve this in tern functions, seems like a formatting issue?

…on@main' into 335-differential-precision-chevron@main

barnett11

Precision approach works well and looks good - issues identified have been passed to other issues thanks @BFalquet

benoit and others added 3 commits December 12, 2022 16:22

initial trial

5e517f7

add precision argument

6c566a8

Merge branch 'main' into 335-differential-precision-chevron@main

ec32dac

BFalquet requested a review from clarkliming December 16, 2022 11:58

revert to older ggplot version

d282d10

clarkliming reviewed Dec 17, 2022

View reviewed changes

Merge branch 'main' into 335-differential-precision-chevron@main

d3c7326

clarkliming reviewed Dec 19, 2022

View reviewed changes

barnett11 requested changes Dec 19, 2022

View reviewed changes

use format trick

5c7f33c

BFalquet marked this pull request as draft January 3, 2023 18:25

add tests

539494d

BFalquet marked this pull request as ready for review January 4, 2023 07:56

correct tests and helper

c29f1a9

update roxygen

08f99a6

BFalquet requested review from clarkliming and barnett11 January 4, 2023 18:15

clarkliming reviewed Jan 5, 2023

View reviewed changes

R/utils.R Outdated Show resolved Hide resolved

clarkliming reviewed Jan 5, 2023

View reviewed changes

R/lbt01.R Outdated Show resolved Hide resolved

liming's suggestions

4c22ffe

clarkliming reviewed Jan 9, 2023

View reviewed changes

R/utils.R Outdated Show resolved Hide resolved

clarkliming mentioned this pull request Jan 10, 2023

clean up todo #353

Merged

simplify

94b24c7

clarkliming reviewed Jan 10, 2023

View reviewed changes

R/lbt01.R Outdated Show resolved Hide resolved

return a function

adbb569

clarkliming reviewed Jan 10, 2023

View reviewed changes

R/utils.R Outdated Show resolved Hide resolved

return NULL if format is NA

6847409

clarkliming assigned clarkliming and barnett11 and unassigned barnett11 Jan 11, 2023

barnett11 requested changes Jan 11, 2023

View reviewed changes

clarkliming added a commit that referenced this pull request Jan 12, 2023

clean up todo (#353)

8efb3c6

close #341 other todo in code: 1. change from baseline for baseline, tackled in #343 2. check grades in mapping, tackled in #324

adapt to changes in rtables

7f531ee

Merge branch 'main' into 335-differential-precision-chevron@main

9f46f50

Signed-off-by: Liming <[email protected]>

clarkliming approved these changes Jan 13, 2023

View reviewed changes

clarkliming mentioned this pull request Jan 13, 2023

0 count value summary leads to NA of mean and sd, Inf in min and max insightsengineering/tern#796

Closed

Merge remote-tracking branch 'origin/335-differential-precision-chevr…

8fde04e

…on@main' into 335-differential-precision-chevron@main

barnett11 self-requested a review January 13, 2023 12:02

barnett11 approved these changes Jan 13, 2023

View reviewed changes

BFalquet merged commit 5960712 into main Jan 13, 2023

BFalquet deleted the 335-differential-precision-chevron@main branch January 13, 2023 12:04

clarkliming mentioned this pull request Feb 9, 2023

[Feature Request]: different factor levels for different split contexts insightsengineering/tern#824

Open

3 tasks

clarkliming mentioned this pull request Jun 21, 2023

default precisions for s_summary.numeric insightsengineering/tern#979

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

335 differential precision (chevron) #343

335 differential precision (chevron) #343

BFalquet commented Dec 16, 2022

github-actions bot commented Dec 16, 2022 •

edited

Loading

clarkliming Dec 17, 2022

barnett11 Dec 19, 2022

clarkliming Dec 19, 2022

clarkliming commented Dec 19, 2022

barnett11 left a comment •

edited

Loading

clarkliming commented Dec 20, 2022

BFalquet commented Jan 4, 2023

github-actions bot commented Jan 4, 2023 •

edited

Loading

clarkliming commented Jan 11, 2023

barnett11 left a comment •

edited by BFalquet

Loading

clarkliming commented Jan 11, 2023 •

edited by BFalquet

Loading

barnett11 commented Jan 12, 2023 •

edited by BFalquet

Loading

barnett11 commented Jan 12, 2023 •

edited by BFalquet

Loading

clarkliming commented Jan 13, 2023 •

edited by BFalquet

Loading

clarkliming left a comment

shajoezhu commented Jan 13, 2023 •

edited by BFalquet

Loading

barnett11 left a comment

335 differential precision (chevron) #343

335 differential precision (chevron) #343

Conversation

BFalquet commented Dec 16, 2022

github-actions bot commented Dec 16, 2022 • edited Loading

Unit Tests Summary

clarkliming Dec 17, 2022

Choose a reason for hiding this comment

barnett11 Dec 19, 2022

Choose a reason for hiding this comment

clarkliming Dec 19, 2022

Choose a reason for hiding this comment

clarkliming commented Dec 19, 2022

barnett11 left a comment • edited Loading

Choose a reason for hiding this comment

clarkliming commented Dec 20, 2022

BFalquet commented Jan 4, 2023

github-actions bot commented Jan 4, 2023 • edited Loading

Code Coverage Summary

clarkliming commented Jan 11, 2023

barnett11 left a comment • edited by BFalquet Loading

Choose a reason for hiding this comment

clarkliming commented Jan 11, 2023 • edited by BFalquet Loading

barnett11 commented Jan 12, 2023 • edited by BFalquet Loading

barnett11 commented Jan 12, 2023 • edited by BFalquet Loading

clarkliming commented Jan 13, 2023 • edited by BFalquet Loading

clarkliming left a comment

Choose a reason for hiding this comment

shajoezhu commented Jan 13, 2023 • edited by BFalquet Loading

barnett11 left a comment

Choose a reason for hiding this comment

github-actions bot commented Dec 16, 2022 •

edited

Loading

barnett11 left a comment •

edited

Loading

github-actions bot commented Jan 4, 2023 •

edited

Loading

barnett11 left a comment •

edited by BFalquet

Loading

clarkliming commented Jan 11, 2023 •

edited by BFalquet

Loading

barnett11 commented Jan 12, 2023 •

edited by BFalquet

Loading

barnett11 commented Jan 12, 2023 •

edited by BFalquet

Loading

clarkliming commented Jan 13, 2023 •

edited by BFalquet

Loading

shajoezhu commented Jan 13, 2023 •

edited by BFalquet

Loading