Add tests@208 support for newline@main #746

Melkiades · 2023-10-13T15:47:51Z

Fixes insightsengineering/tlg-catalog#48

…oche/rtables into add_tests@208_support_for_newline@main

shajoezhu · 2023-10-16T04:33:54Z

Thanks @Melkiades for the change, blocking this PR after the release

Melkiades · 2023-10-16T10:39:03Z

R/tt_toString.R

+        # We want topleft alignment that goes to the bottom!
+        tl <- c(rep("", nli - lentl), tl)


This is important imo @shajoezhu

for the align bottom of top_left

github-actions · 2023-10-16T10:40:10Z

Unit Tests Summary

      1 files     24 suites 2m 19s ⏱️
  186 tests   186 ✔️ 0 💤 0 ❌
1 408 runs 1 408 ✔️ 0 💤 0 ❌

Results for commit de4806a.

♻️ This comment has been updated with latest results.

github-actions · 2023-10-16T10:41:24Z

Code Coverage Summary

Filename                     Stmts    Miss  Cover    Missing
-------------------------  -------  ------  -------  ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
R/00tabletrees.R               662      46  93.05%   24, 94, 96, 371, 447-448, 451, 590, 681, 763-764, 854, 856-857, 880-881, 901, 1080-1083, 1214, 1298-1299, 1358-1361, 1394-1395, 1401-1406, 1449, 1455, 1544, 1636, 1647, 1649-1650, 1653-1654, 1682, 1708-1709
R/as_html.R                    102       4  96.08%   69, 135-137
R/colby_constructors.R         489      17  96.52%   63, 106-107, 155-156, 202-203, 328, 341, 1107, 1184, 1327, 1363, 1382, 1402, 1421, 1568
R/compare_rtables.R             78      11  85.90%   95-96, 99-100, 113-114, 131, 150-151, 181, 185
R/format_rcell.R                12       0  100.00%
R/indent.R                      13       2  84.62%   39-40
R/index_footnotes.R             52       0  100.00%
R/make_split_fun.R              97      15  84.54%   22-25, 48-49, 51-52, 103, 106, 252, 254-255, 259-260, 276, 366
R/make_subset_expr.R           108      12  88.89%   26-35, 93-98, 131, 200, 211, 218
R/simple_analysis.R              5       1  80.00%   48
R/split_funs.R                 431      52  87.94%   131, 135, 140-145, 149, 165-167, 327-330, 346-351, 421, 461, 475-476, 491, 547, 556-557, 559, 571, 611, 631, 790, 797, 822-823, 831-832, 834-835, 993-994, 1005-1008, 1071-1072, 1128-1129
R/summary.R                    187      20  89.30%   40, 84, 188, 195, 260-263, 273-274, 293-294, 399, 453-457, 485, 513
R/tree_accessors.R             802      74  90.77%   93, 210, 223, 240, 271, 281, 293, 385, 407-408, 662-666, 774, 788, 805, 846, 900-901, 933, 959, 991-995, 1042, 1104-1106, 1120, 1186, 1272-1273, 1291, 1305-1306, 1314, 1344, 1357-1360, 1378-1381, 1445, 1483, 1573, 1651, 1661, 1671, 1680, 1686, 1692-1695, 1973, 2267, 2361, 2443-2449, 2593, 2692, 2715-2743, 2755-2760
R/tt_afun_utils.R              345      25  92.75%   44, 150, 156, 164-173, 222, 233-234, 456, 463-464, 528-530, 549, 563-564
R/tt_compare_tables.R           65       4  93.85%   56, 170, 248, 251
R/tt_compatibility.R           442      48  89.14%   19, 137-138, 187, 191, 321-322, 325-328, 335, 338, 382, 498, 540, 569, 587, 611-612, 652, 665-667, 736, 761-764, 773, 825, 831, 838-839, 943, 949, 975-987, 1016-1017
R/tt_dotabulation.R            963      67  93.04%   47, 236, 238, 283, 305, 308-309, 356, 388-389, 417-418, 502, 627-629, 678, 681, 707-709, 718, 735-737, 742-743, 971, 974, 1001, 1112-1113, 1280-1284, 1381, 1475-1483, 1549-1552, 1563, 1567, 1572-1574, 1584, 1588, 1610, 1697-1711
R/tt_export.R                  442      38  91.40%   47, 63, 104-106, 137, 176-178, 235-248, 352-353, 365, 680, 689, 714-718, 886-889, 892, 923, 929
R/tt_from_df.R                   9       0  100.00%
R/tt_paginate.R                382      26  93.19%   47, 69, 100-105, 357, 458-459, 476-478, 621-622, 666-671, 735, 737, 746, 752, 755
R/tt_pos_and_access.R          519      37  92.87%   73, 75-76, 100, 152, 187-189, 234, 471, 473, 480, 486, 499, 508-509, 673, 684-686, 692-695, 723, 765-766, 776, 927-928, 976-1000, 1224, 1294
R/tt_showmethods.R             121      20  83.47%   47, 73-87, 138, 154-157, 163, 170, 172-174, 252-253
R/tt_sort.R                     76       3  96.05%   211-212, 218
R/tt_toString.R                324      23  92.90%   112, 305, 318, 327, 334, 336, 342-352, 435, 489, 495, 704-729
R/utils.R                       23       0  100.00%
R/validate_table_struct.R       75      11  85.33%   76-79, 88-89, 129, 137-138, 191-192
R/Viewer.R                      56       8  85.71%   50, 54, 64-67, 87, 117
TOTAL                         6880     564  91.80%

Diff against main

Filename                     Stmts    Miss  Cover
-------------------------  -------  ------  --------
R/00tabletrees.R               +38      +5  -0.38%
R/as_html.R                    +14      -3  +4.03%
R/colby_constructors.R          +7       0  +0.05%
R/index_footnotes.R             +2       0  +100.00%
R/make_split_fun.R             +97     +15  +84.54%
R/make_subset_expr.R            +2       0  +0.21%
R/split_funs.R                 +10      -3  +1.00%
R/summary.R                     +4      +4  -1.95%
R/tree_accessors.R              +9      +6  -0.65%
R/tt_compatibility.R           +29      -2  +1.25%
R/tt_dotabulation.R           +226     +23  -0.99%
R/tt_export.R                 +213     -35  +23.28%
R/tt_paginate.R                 -1     +11  -2.89%
R/tt_pos_and_access.R           -1       0  -0.01%
R/tt_showmethods.R               0      -1  +0.83%
R/tt_sort.R                     -5      -3  +3.46%
R/tt_toString.R                +19      +1  +0.11%
R/utils.R                      +12      -1  +9.09%
R/validate_table_struct.R      +75     +11  +85.33%
R/Viewer.R                       0      -1  +1.79%
TOTAL                         +750     +27  +0.56%

Results for commit: de4806a

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

Melkiades · 2023-10-16T15:52:29Z

R/tt_export.R

-          spans <- spans[-whsnc, , drop = FALSE]
-          body <- body[-whsnc, , drop = FALSE]
-          mpf_aligns <- mpf_aligns[-whsnc, , drop = FALSE]
+        row_to_pop <- whsnc - 1


this exported was changed because now the extra line to pop is above and not below (top_left is bottom aligned)

Melkiades · 2023-10-16T15:53:13Z

tests/testthat/test-printing.R

@@ -147,7 +147,7 @@ test_that("newline in column names and possibly cell values work", {
    expect_identical(mf_nlheader(matform2),
                     4L)
    expect_identical(matform2$strings[1:4, 1, drop = TRUE],
-                     c("Ethnicity", "  Factor2", "", ""))
+                     c("", "", "Ethnicity", "  Factor2"))


now It is bottom aligned

Melkiades · 2023-10-16T15:54:07Z

tests/testthat/test-printing.R

+        analyze("BMRKR1", na_str = "asd\nasd") %>%  # \n error
+        build_table(DM_trick)
+
+    top_left(tbl) <- c("\na", "b\nd\n\n", "c\n\n") # last \n is eaten up, if in the middle error


first \n are kept ->> note that if you want a blank line you could even do "\n " as group of spaces are considered words now

edelarua · 2023-10-16T20:40:20Z

@Melkiades one case I have identified where the topleft bottom-up insertion isn't working for me:

lyt <- basic_table(show_colcounts = TRUE) %>%
    split_cols_by("ARM") %>%
    analyze("AGE") %>%
    append_topleft("Age")
build_table(lyt, rawdat2)

Age     ARM1      ARM2  
       (N=504)   (N=496)
————————————————————————
Mean    55.36     55.64

This works only if the labels_var argument is specified when splitting by ARM. i.e. This works:

lyt <- basic_table(show_colcounts = TRUE) %>%
    split_cols_by("ARM", labels_var = "arm_label") %>%
    analyze("AGE") %>%
    append_topleft("Age")
build_table(lyt, rawdat2)

         Arm       Arm  
          1         2   
Age    (N=504)   (N=496)
————————————————————————
Mean    55.36     55.64

I'm not sure if this is an issue with the topleft insertion or because of the newlines in arm_label variable, but let me know if I'm doing something wrong!

shajoezhu

lgtm! merge to allow downstream UAT

Melkiades · 2023-10-17T07:23:48Z

@Melkiades one case I have identified where the topleft bottom-up insertion isn't working for me:
lyt <- basic_table(show_colcounts = TRUE) %>%
    split_cols_by("ARM") %>%
    analyze("AGE") %>%
    append_topleft("Age")
build_table(lyt, rawdat2)
Age     ARM1      ARM2  
       (N=504)   (N=496)
————————————————————————
Mean    55.36     55.64 
This works only if the labels_var argument is specified when splitting by ARM. i.e. This works:
lyt <- basic_table(show_colcounts = TRUE) %>%
    split_cols_by("ARM", labels_var = "arm_label") %>%
    analyze("AGE") %>%
    append_topleft("Age")
build_table(lyt, rawdat2)
         Arm       Arm  
          1         2   
Age    (N=504)   (N=496)
————————————————————————
Mean    55.36     55.64 
I'm not sure if this is an issue with the topleft insertion or because of the newlines in arm_label variable, but let me know if I'm doing something wrong!

for me the first does not work

> rawdat2 <- makefakedat2()
> lyt <- basic_table(show_colcounts = TRUE) %>%
+   split_cols_by("ARM") %>%
+   analyze("AGE") %>%
+   append_topleft("Age")
> build_table(lyt, rawdat2)
Error in .checkvarsok(spl, df) : 
   variable(s) [AGE] not present in data. (AnalyzeVarSplit)

Melkiades added 3 commits October 12, 2023 11:07

Fixing Viewer and as_html for \n manual varlabels

08ed785

fixes

e3dfa6f

Tests for newline allowance

26327c6

Melkiades added the sme label Oct 13, 2023

Melkiades added 3 commits October 13, 2023 17:48

Merge branch 'main' into add_tests@208_support_for_newline@main

ae2c3d1

additions

2e4cae5

Merge branch 'add_tests@208_support_for_newline@main' of github.com:R…

ecc1bb8

…oche/rtables into add_tests@208_support_for_newline@main

Melkiades marked this pull request as ready for review October 15, 2023 16:37

shajoezhu marked this pull request as draft October 16, 2023 04:33

Melkiades marked this pull request as ready for review October 16, 2023 08:39

Melkiades added 2 commits October 16, 2023 11:49

bug fix for bottom alignment for topleft materials

7994851

fixing top_left material in case of (N=xx)

75eb2f6

Melkiades commented Oct 16, 2023

View reviewed changes

Melkiades added 2 commits October 16, 2023 13:07

relevant fix for topleft

5c67515

allowing bottom topleft

cf27faf

Melkiades commented Oct 16, 2023

View reviewed changes

Melkiades requested a review from edelarua October 16, 2023 19:57

empty

de4806a

shajoezhu enabled auto-merge (squash) October 17, 2023 02:39

shajoezhu approved these changes Oct 17, 2023

View reviewed changes

shajoezhu merged commit aecd4c1 into main Oct 17, 2023
17 checks passed

shajoezhu deleted the add_tests@208_support_for_newline@main branch October 17, 2023 02:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add tests@208 support for newline@main #746

Add tests@208 support for newline@main #746

Melkiades commented Oct 13, 2023 •

edited

Loading

shajoezhu commented Oct 16, 2023

Melkiades Oct 16, 2023

Melkiades Oct 16, 2023

github-actions bot commented Oct 16, 2023 •

edited

Loading

github-actions bot commented Oct 16, 2023 •

edited

Loading

Melkiades Oct 16, 2023

Melkiades Oct 16, 2023

Melkiades Oct 16, 2023

edelarua commented Oct 16, 2023

shajoezhu left a comment

Melkiades commented Oct 17, 2023

		# We want topleft alignment that goes to the bottom!
		tl <- c(rep("", nli - lentl), tl)

Add tests@208 support for newline@main #746

Add tests@208 support for newline@main #746

Conversation

Melkiades commented Oct 13, 2023 • edited Loading

shajoezhu commented Oct 16, 2023

Melkiades Oct 16, 2023

Choose a reason for hiding this comment

Melkiades Oct 16, 2023

Choose a reason for hiding this comment

github-actions bot commented Oct 16, 2023 • edited Loading

Unit Tests Summary

github-actions bot commented Oct 16, 2023 • edited Loading

Code Coverage Summary

Diff against main

Melkiades Oct 16, 2023

Choose a reason for hiding this comment

Melkiades Oct 16, 2023

Choose a reason for hiding this comment

Melkiades Oct 16, 2023

Choose a reason for hiding this comment

edelarua commented Oct 16, 2023

shajoezhu left a comment

Choose a reason for hiding this comment

Melkiades commented Oct 17, 2023

Melkiades commented Oct 13, 2023 •

edited

Loading

github-actions bot commented Oct 16, 2023 •

edited

Loading

github-actions bot commented Oct 16, 2023 •

edited

Loading