insightsengineering / teal.data

Data model for teal applications
https://insightsengineering.github.io/teal.data/
Other
8 stars 7 forks source link

minor improvements to label functions #299

Closed chlebowa closed 6 months ago

chlebowa commented 6 months ago

Adds unit tests for col_labels<-. Slightly changes bodies of col_labels<- and col_relabel so that they are more similar. Improved argument checks.

github-actions[bot] commented 6 months ago

badge

Code Coverage Summary

Filename                         Stmts    Miss  Cover    Missing
-----------------------------  -------  ------  -------  --------------------
R/cdisc_data.R                       1       0  100.00%
R/deprecated.R                      57      57  0.00%    19-344
R/dummy_function.R                   2       2  0.00%    14-15
R/formatters_var_labels.R           38       0  100.00%
R/join_key.R                        38       0  100.00%
R/join_keys-c.R                     12       0  100.00%
R/join_keys-extract.R              128       0  100.00%
R/join_keys-names.R                 15       0  100.00%
R/join_keys-parents.R               30       0  100.00%
R/join_keys-print.R                 45       0  100.00%
R/join_keys-utils.R                 73       3  95.89%   35-38
R/join_keys.R                       21       0  100.00%
R/teal_data-class.R                 25       1  96.00%   69
R/teal_data-datanames.R             10       0  100.00%
R/teal_data-get_code.R              14       0  100.00%
R/teal_data-show.R                   4       4  0.00%    14-19
R/teal_data.R                       30      16  46.67%   34, 37-43, 53-59, 62
R/testhat-helpers.R                 26       0  100.00%
R/topological_sort.R                32       0  100.00%
R/utils-get_code_dependency.R      184       1  99.46%   275
R/verify.R                          42      11  73.81%   63, 93-97, 100-104
TOTAL                              827      95  88.51%

Diff against main

Filename                     Stmts    Miss  Cover
-------------------------  -------  ------  -------
R/formatters_var_labels.R       +2     -11  +30.56%
TOTAL                           +2     -11  +1.36%

Results for commit: 32e305dc7c87e9918cbf8a848b120d987ad4adf1

Minimum allowed coverage is 80%

:recycle: This comment has been updated with latest results

github-actions[bot] commented 6 months ago

Unit Tests Summary

  1 files   14 suites   1s :stopwatch: 184 tests 182 :white_check_mark: 2 :zzz: 0 :x: 255 runs  253 :white_check_mark: 2 :zzz: 0 :x:

Results for commit 32e305dc.

:recycle: This comment has been updated with latest results.

github-actions[bot] commented 6 months ago

Unit Test Performance Difference

Additional test case details | Test Suite | $Status$ | Time on `main` | $±Time$ | Test Case | |:-----|:----:|:----:|:----:|:-----| | formatters_var_labels | 👶 | | $+0.00$ | col_labels_matches_labels_to_variables_by_names_of_values_argument | | formatters_var_labels | 👶 | | $+0.01$ | col_labels_sets_variable_labels_when_passed_named_character_vector | | formatters_var_labels | 👶 | | $+0.00$ | col_labels_sets_variable_labels_when_passed_unnamed_character_vector | | formatters_var_labels | 👶 | | $+0.01$ | col_labels_value_accepts_character_vector | | formatters_var_labels | 👶 | | $+0.00$ | col_labels_value_accepts_named_vector | | formatters_var_labels | 👶 | | $+0.01$ | col_labels_value_must_be_same_length_as_x | | formatters_var_labels | 👶 | | $+0.01$ | col_labels_value_names_must_be_same_as_variable_names |

Results for commit 8cb6267dc6c455cefa3574f3e00a96c8add6943d

♻️ This comment has been updated with latest results.

chlebowa commented 6 months ago

The stop("labels for variables ... are not character strings") is old logic that I hadn't changed previously. But this is silly, this should be checked by the setter, shouldn't it?

gogonzo commented 6 months ago

The stop("labels for variables ... are not character strings") is old logic that I hadn't changed previously. But this is silly, this should be checked by the setter, shouldn't it?

Hehe yes, I think it doesn't make a bigger sense neither. Why would we check on "get". Probably only because someone/something changed label directly by attr(<col>, "label"). I think if somebody/something does it we shouldn't really validate the "class" on get. I allow to remove this line if you agree. If not or if you're unsure please cover with a test

chlebowa commented 6 months ago

Rearranged a bit.