239 utilize logger::log_shiny_input_change

[m7pr]

Close #239

The example loading for tm_a_pca on a startup shows how the input change logging is made.

We are connected to package namespace (namespace = "teal.modules.general")
that is created on a startup with teal.logger::register_logger(namespace = "teal.modules.general")

This function needs to be called in the server function, so there is no way this can be run during the package startup.

Note: Needed to bring logger to Imports.

[m7pr]

@pawelru kindly asking you for the review, as you are the logging expert these days :)
I needed to move logger to Imports. Do you think there is a way to extend teal.logger::register_logger to utilize this so logger can stay in Suggests? logger::log_shiny_input_changes needs to be called in server function.

[github-actions]

Code Coverage Summary

Filename                      Stmts    Miss  Cover    Missing
--------------------------  -------  ------  -------  ------------------------------------
R/tm_a_pca.R                    827     827  0.00%    108-1068
R/tm_a_regression.R             773     773  0.00%    153-1031
R/tm_data_table.R               185     185  0.00%    93-332
R/tm_file_viewer.R              173     173  0.00%    44-252
R/tm_front_page.R               133     122  8.27%    70-228
R/tm_g_association.R            330     330  0.00%    135-537
R/tm_g_bivariate.R              672     410  38.99%   303-769, 810, 921, 938, 956, 967-989
R/tm_g_distribution.R          1050    1050  0.00%    122-1311
R/tm_g_response.R               351     351  0.00%    154-578
R/tm_g_scatterplot.R            722     722  0.00%    230-1053
R/tm_g_scatterplotmatrix.R      278     259  6.83%    165-472, 533, 547
R/tm_missing_data.R            1069    1069  0.00%    92-1317
R/tm_outliers.R                 985     985  0.00%    134-1263
R/tm_t_crosstable.R             251     251  0.00%    141-440
R/tm_variable_browser.R         830     825  0.60%    79-1071, 1109-1293
R/utils.R                        99      96  3.03%    82-267
R/zzz.R                           2       2  0.00%    2-3
TOTAL                          8730    8430  3.44%

Diff against main

Filename                      Stmts    Miss  Cover
--------------------------  -------  ------  --------
R/tm_a_pca.R                     +1      +1  +100.00%
R/tm_a_regression.R              +1      +1  +100.00%
R/tm_data_table.R                +1      +1  +100.00%
R/tm_file_viewer.R               +1      +1  +100.00%
R/tm_front_page.R                +1      +1  -0.06%
R/tm_g_association.R             +1      +1  +100.00%
R/tm_g_bivariate.R               +1      +1  -0.06%
R/tm_g_distribution.R            +1      +1  +100.00%
R/tm_g_response.R                +1      +1  +100.00%
R/tm_g_scatterplot.R             +1      +1  +100.00%
R/tm_g_scatterplotmatrix.R       +1      +1  -0.02%
R/tm_missing_data.R              +1      +1  +100.00%
R/tm_outliers.R                  +1      +1  +100.00%
R/tm_t_crosstable.R              +1      +1  +100.00%
R/tm_variable_browser.R          +1      +1  -0.00%
TOTAL                           +15     +15  -0.01%

Results for commit: 75be73a

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

[github-actions]

Unit Tests Summary

  1 files   22 suites   11m 54s ⏱️
147 tests 147 ✅ 0 💤 0 ❌
478 runs  478 ✅ 0 💤 0 ❌

Results for commit 75be73a.

♻️ This comment has been updated with latest results.

[github-actions]

Unit Test Performance Difference

Test Suite	$Status$	Time on main	$±Time$	$±Tests$	$±Skipped$	$±Failures$	$±Errors$
shinytest2-tm_a_pca	💔	$96.26$	$+4.61$	$0$	$0$	$0$	$0$
shinytest2-tm_a_regression	💔	$42.32$	$+1.89$	$0$	$0$	$0$	$0$
shinytest2-tm_g_association	💔	$28.08$	$+1.25$	$0$	$0$	$0$	$0$
shinytest2-tm_g_bivariate	💔	$43.77$	$+2.38$	$0$	$0$	$0$	$0$
shinytest2-tm_g_distribution	💔	$55.23$	$+8.80$	$0$	$0$	$0$	$0$
shinytest2-tm_g_response	💔	$27.81$	$+1.02$	$0$	$0$	$0$	$0$
shinytest2-tm_g_scatterplot	💔	$73.36$	$+8.46$	$0$	$0$	$0$	$0$
shinytest2-tm_g_scatterplotmatrix	💔	$22.54$	$+1.38$	$0$	$0$	$0$	$0$
shinytest2-tm_misssing_data	💔	$31.65$	$+6.46$	$0$	$0$	$0$	$0$
shinytest2-tm_outliers	💔	$83.45$	$+28.21$	$0$	$0$	$0$	$0$
shinytest2-tm_t_crosstable	💔	$25.87$	$+1.68$	$0$	$0$	$0$	$0$
shinytest2-tm_variable_browser	💔	$48.40$	$+13.71$	$0$	$0$	$0$	$0$

Additional test case details

Test Suite	$Status$	Time on main	$±Time$	Test Case
shinytest2-tm_g_bivariate	💔	$24.93$	$+1.82$	e2e_tm_g_bivariate_Setting_encoding_inputs_produces_outputs_without_validation_errors.
shinytest2-tm_g_distribution	💔	$14.82$	$+5.18$	e2e_tm_g_distribution_Histogram_encoding_inputs_produce_output_without_validation_errors.
shinytest2-tm_g_distribution	💔	$9.07$	$+2.81$	e2e_tm_g_distribution_QQ_plot_encoding_inputs_produce_output_without_validation_errors.
shinytest2-tm_g_scatterplot	💔	$8.45$	$+1.66$	e2e_tm_g_scatterplot_Base_for_the_log_transformation_can_be_applied.
shinytest2-tm_g_scatterplot	💔	$7.04$	$+1.46$	e2e_tm_g_scatterplot_Get_validation_error_when_facetting_with_the_same_row_col_variable.
shinytest2-tm_g_scatterplot	💔	$42.47$	$+2.67$	e2e_tm_g_scatterplot_The_encoding_inputs_produce_output_without_validation_errors.
shinytest2-tm_g_scatterplot	💔	$9.11$	$+1.81$	e2e_tm_g_scatterplot_The_log_transform_is_only_possible_for_positive_numeric_vars.
shinytest2-tm_misssing_data	💔	$7.23$	$+1.15$	e2e_tm_missing_data_Check_default_settings_and_visibility_of_the_combinations_graph_and_encodings
shinytest2-tm_misssing_data	💔	$5.58$	$+1.31$	e2e_tm_missing_data_Validate_By_Variable_Levels_table_values
shinytest2-tm_misssing_data	💔	$6.82$	$+2.84$	e2e_tm_missing_data_Validate_functionality_and_UI_response_for_By_Variable_Levels_
shinytest2-tm_outliers	💔	$8.81$	$+3.47$	e2e_tm_outliers_Data_extract_spec_elements_are_initialized_with_the_default_values_specified_by_outlier_var_and_categorical_var_argument.
shinytest2-tm_outliers	💔	$8.65$	$+3.03$	e2e_tm_outliers_Default_radio_buttons_are_set_properly.
shinytest2-tm_outliers	💔	$5.97$	$+1.12$	e2e_tm_outliers_Method_parameters_are_set_properly.
shinytest2-tm_outliers	💔	$5.93$	$+1.02$	e2e_tm_outliers_Module_is_divided_into_3_tabs.
shinytest2-tm_outliers	💔	$12.53$	$+5.81$	e2e_tm_outliers_Outlier_definition_text_and_range_are_displayed_properly_depending_on_method.
shinytest2-tm_outliers	💔	$8.72$	$+2.98$	e2e_tm_outliers_Outlier_table_is_displayed_with_proper_content.
shinytest2-tm_outliers	💔	$9.90$	$+4.17$	e2e_tm_outliers_Outliers_summary_table_is_displayed_with_proper_content.
shinytest2-tm_outliers	💔	$7.41$	$+1.88$	e2e_tm_outliers_Plot_type_is_correctly_set_by_default_and_has_appropriate_possible_options.
shinytest2-tm_outliers	💔	$9.56$	$+3.94$	e2e_tm_outliers_Plot_type_is_hidden_when_Boxplot_tab_is_not_selected.
shinytest2-tm_variable_browser	💔	$8.53$	$+2.71$	e2e_tm_variable_browser_Selecting_treat_variable_as_factor_changes_the_table_headers.
shinytest2-tm_variable_browser	💔	$7.11$	$+1.70$	e2e_tm_variable_browser_changing_display_density_encoding_doesn_t_show_errors.
shinytest2-tm_variable_browser	💔	$8.07$	$+3.02$	e2e_tm_variable_browser_changing_outlier_definition_encoding_doesn_t_show_errors.
shinytest2-tm_variable_browser	💔	$12.01$	$+3.56$	e2e_tm_variable_browser_changing_plot_setting_encodings_doesn_t_show_errors.
shinytest2-tm_variable_browser	💔	$5.58$	$+1.07$	e2e_tm_variable_browser_content_is_displayed_correctly.
shinytest2-tm_variable_browser	💔	$7.10$	$+1.63$	e2e_tm_variable_browser_selection_of_categorical_variable_has_a_table_with_level_header.

Results for commit 65415c2

♻️ This comment has been updated with latest results.

[pawelru]

Do you think there is a way to extend teal.logger::register_logger to utilize this so logger can stay in Suggests? logger::log_shiny_input_changes needs to be called in server function.

There was a discussion about this here: insightsengineering/teal.logger#74. This idea collapses for some good reasons. You should not be afraid of promoting a package if it is really used. I'm personally not a big fan of hiding (true) dependencies. I think it creates more bad than good. What you have done is correct.

[m7pr]

Got it, thanks @pawelru for suggestions

[m7pr]

Thanks @pawelru - I did updated NEWS, verified content of pre-commit-config.yaml and part of DESCRIPTION for Config/verdepcheck in this PR and others as well. Would you accept?

insightsengineering/teal.goshawk#274
insightsengineering/teal.osprey#265
insightsengineering/teal.modules.hermes#382
insightsengineering/teal.modules.clinical#1182

[pawelru]

Thanks. It looks to me that changes in logger you proposed can go its own way. After adding the if statement this is independent. I think it's good to go and after that PR we can remove the if statement (and vbump dep)

[m7pr]

Created an issue, for the future, to keep in mind that the if statement might not be needed anymore at some point #758