fix as_result_df for nested rbind
#818
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Code Coverage SummaryDiff against mainResults for commit: bd4b284 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
Unit Tests Summary 1 files 24 suites 1m 30s ⏱️ Results for commit bd4b284. ♻️ This comment has been updated with latest results. |
Unit Test Performance Difference
Additional test case details
Results for commit c1714cb ♻️ This comment has been updated with latest results. |
Melkiades
commented
Jan 29, 2024
Comment on lines
+215
to
+221
| init_tbl <- df_to_tt(mtcars) | ||
| end_tbl <- init_tbl %>% as_result_df(as_is = TRUE) %>% df_to_tt() | ||
|
|
||
| expect_equal( | ||
| matrix_form(init_tbl)$strings, | ||
| matrix_form(end_tbl)$strings | ||
| ) |
There was a problem hiding this comment.
hey @shajoezhu could you check this? Tell me if it is as you want it. The above example works when there is a complex table and non-unique rownames
There was a problem hiding this comment.
brilliant! i was wondering why it has non-unique rownames (what about appending the multi-level rownames, and make them unique?)
There was a problem hiding this comment.
rownames need to be unique but if you create a table using {rtables} it happens that they are not unique. If that is the case as_result_df makes a label_name column with the final row names and if that column is present df_to_tt uses it for the table
There was a problem hiding this comment.
i would suggest we take look into the tree structure, i feel that we need to go deeper, and adding more elements at the node level
if I do the following, i see it starts to appending new columsn
> df_to_tt(mtcars) %>% as_result_df()
avar_name row_name label_name row_num is_group_summary node_class mpg cyl disp hp drat wt qsec vs am gear carb
1 Mazda RX4 Mazda RX4 Mazda RX4 1 FALSE DataRow 21 6 160 110 3.9 2.62 16.46 0 1 4 4
2 Mazda RX4 Wag Mazda RX4 Wag Mazda RX4 Wag 2 FALSE DataRow 21 6 160 110 3.9 2.875 17.02 0 1 4 4
3 Datsun 710 Datsun 710 Datsun 710 3 FALSE DataRow 22.8 4 108 93 3.85 2.32 18.61 1 1 4 1
4 Hornet 4 Drive Hornet 4 Drive Hornet 4 Drive 4 FALSE DataRow 21.4 6 258 110 3.08 3.215 19.44 1 0 3 1
5 Hornet Sportabout Hornet Sportabout Hornet Sportabout 5 FALSE DataRow 18.7 8 360 175 3.15 3.44 17.02 0 0 3 2
6 Valiant Valiant Valiant 6 FALSE DataRow 18.1 6 225 105 2.76 3.46 20.22 1 0 3 1
7 Duster 360 Duster 360 Duster 360 7 FALSE DataRow 14.3 8 360 245 3.21 3.57 15.84 0 0 3 4
8 Merc 240D Merc 240D Merc 240D 8 FALSE DataRow 24.4 4 146.7 62 3.69 3.19 20 1 0 4 2
9 Merc 230 Merc 230 Merc 230 9 FALSE DataRow 22.8 4 140.8 95 3.92 3.15 22.9 1 0 4 2
10 Merc 280 Merc 280 Merc 280 10 FALSE DataRow 19.2 6 167.6 123 3.92 3.44 18.3 1 0 4 4
11 Merc 280C Merc 280C Merc 280C 11 FALSE DataRow 17.8 6 167.6 123 3.92 3.44 18.9 1 0 4 4
12 Merc 450SE Merc 450SE Merc 450SE 12 FALSE DataRow 16.4 8 275.8 180 3.07 4.07 17.4 0 0 3 3
13 Merc 450SL Merc 450SL Merc 450SL 13 FALSE DataRow 17.3 8 275.8 180 3.07 3.73 17.6 0 0 3 3
14 Merc 450SLC Merc 450SLC Merc 450SLC 14 FALSE DataRow 15.2 8 275.8 180 3.07 3.78 18 0 0 3 3
15 Cadillac Fleetwood Cadillac Fleetwood Cadillac Fleetwood 15 FALSE DataRow 10.4 8 472 205 2.93 5.25 17.98 0 0 3 4
16 Lincoln Continental Lincoln Continental Lincoln Continental 16 FALSE DataRow 10.4 8 460 215 3 5.424 17.82 0 0 3 4
17 Chrysler Imperial Chrysler Imperial Chrysler Imperial 17 FALSE DataRow 14.7 8 440 230 3.23 5.345 17.42 0 0 3 4
18 Fiat 128 Fiat 128 Fiat 128 18 FALSE DataRow 32.4 4 78.7 66 4.08 2.2 19.47 1 1 4 1
19 Honda Civic Honda Civic Honda Civic 19 FALSE DataRow 30.4 4 75.7 52 4.93 1.615 18.52 1 1 4 2
20 Toyota Corolla Toyota Corolla Toyota Corolla 20 FALSE DataRow 33.9 4 71.1 65 4.22 1.835 19.9 1 1 4 1
21 Toyota Corona Toyota Corona Toyota Corona 21 FALSE DataRow 21.5 4 120.1 97 3.7 2.465 20.01 1 0 3 1
22 Dodge Challenger Dodge Challenger Dodge Challenger 22 FALSE DataRow 15.5 8 318 150 2.76 3.52 16.87 0 0 3 2
23 AMC Javelin AMC Javelin AMC Javelin 23 FALSE DataRow 15.2 8 304 150 3.15 3.435 17.3 0 0 3 2
24 Camaro Z28 Camaro Z28 Camaro Z28 24 FALSE DataRow 13.3 8 350 245 3.73 3.84 15.41 0 0 3 4
25 Pontiac Firebird Pontiac Firebird Pontiac Firebird 25 FALSE DataRow 19.2 8 400 175 3.08 3.845 17.05 0 0 3 2
26 Fiat X1-9 Fiat X1-9 Fiat X1-9 26 FALSE DataRow 27.3 4 79 66 4.08 1.935 18.9 1 1 4 1
27 Porsche 914-2 Porsche 914-2 Porsche 914-2 27 FALSE DataRow 26 4 120.3 91 4.43 2.14 16.7 0 1 5 2
28 Lotus Europa Lotus Europa Lotus Europa 28 FALSE DataRow 30.4 4 95.1 113 3.77 1.513 16.9 1 1 5 2
29 Ford Pantera L Ford Pantera L Ford Pantera L 29 FALSE DataRow 15.8 8 351 264 4.22 3.17 14.5 0 1 5 4
30 Ferrari Dino Ferrari Dino Ferrari Dino 30 FALSE DataRow 19.7 6 145 175 3.62 2.77 15.5 0 1 5 6
31 Maserati Bora Maserati Bora Maserati Bora 31 FALSE DataRow 15 8 301 335 3.54 3.57 14.6 0 1 5 8
32 Volvo 142E Volvo 142E Volvo 142E 32 FALSE DataRow 21.4 4 121 109 4.11 2.78 18.6 1 1 4 2
however, if i start appending to this operation, it does not go back
df_to_tt(mtcars) %>% as_result_df() %>% df_to_tt() %>% as_result_df()
shajoezhu
approved these changes
Feb 21, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #815 and #809
Having multiple nested
rbindis usually a problem as it breaks the structure of the table completely. In other words, we should find a way to flattenrbind_rootsnodes, or remove them in {tern} (tabulate_survival_subgroupsuses them), or establish a way to represent these in the table tree.I would suggest going for the following:
rbindnesting from {tern} - e.g.tabulate_survival_subgroupstern#1181