Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unify plot and table namespaces #1194

Closed
wants to merge 28 commits into from
Closed

Unify plot and table namespaces #1194

wants to merge 28 commits into from

Conversation

m7pr
Copy link
Contributor

@m7pr m7pr commented Jun 12, 2024

Most of the plots had the same namespace name called myplot. The same for tables, that are called tables. It would be great to unify so they are all the same, so that their various features stored in input can be excluded from the logger::log_shiny_input_changes so that this feature/PR is easier to maintain in the future #1191

For now, to exclude all plot width and height changes from the logging one needs to run

plot_nss <- c("mmrm_plot", "myplot", "chart", "patient_timeline_plot", "therapy_plot", "vitals_plot")

elements <- c("plot_modal_width", "flex_width", "plot_modal_height", "flex_height")

exclude_inputs <- unlist(lapply(plot_nss, paste, elements, sep = "-"))

to create all plot input names.

When this is merged it's gonna be simplified to

elements <- c("plot_modal_width", "flex_width", "plot_modal_height", "flex_height")
exclude_inputs <- paste("tmcplot", elements, sep = "-")

which is easier to maintain.

One last advantage is the unification of all names so it's easier to write tests.

@m7pr m7pr added the core label Jun 12, 2024
@m7pr
Copy link
Contributor Author

m7pr commented Jun 12, 2024

We can even go one step further and create a new function in utils.R/zzz.R

tmcplot_with_settings <- function() teal.widgets::plot_with_settings_ui(id = ns("tmcplot"))

Copy link
Contributor

github-actions bot commented Jun 12, 2024

Unit Tests Summary

  1 files   70 suites   10s ⏱️
720 tests 150 ✅ 570 💤 0 ❌
856 runs  170 ✅ 686 💤 0 ❌

Results for commit 6bcfc46.

♻️ This comment has been updated with latest results.

Copy link
Contributor

Unit Test Performance Difference

Test suite performance difference
Test Suite $Status$ Time on main $±Time$ $±Tests$ $±Skipped$ $±Failures$ $±Errors$
shinytest2-tm_a_gee 💔 $129.20$ $+7.89$ $0$ $0$ $0$ $0$
shinytest2-tm_a_mmrm 💔 $858.42$ $+20.55$ $0$ $0$ $0$ $0$
shinytest2-tm_g_barchart_simple 💔 $226.97$ $+13.93$ $0$ $0$ $+1$ $0$
shinytest2-tm_g_ci 💔 $100.61$ $+5.50$ $0$ $0$ $0$ $0$
shinytest2-tm_g_forest_rsp 💔 $169.94$ $+5.47$ $-2$ $0$ $+1$ $+5$
shinytest2-tm_g_forest_tte 💔 $64.92$ $+3.45$ $0$ $0$ $+1$ $0$
shinytest2-tm_g_ipp 💔 $102.65$ $+6.26$ $0$ $0$ $0$ $0$
shinytest2-tm_g_km 💔 $259.97$ $+21.01$ $0$ $0$ $0$ $+2$
shinytest2-tm_g_lineplot 💔 $88.43$ $+3.50$ $0$ $0$ $+1$ $0$
shinytest2-tm_g_pp_adverse_events 💔 $129.35$ $+6.47$ $0$ $0$ $0$ $0$
shinytest2-tm_g_pp_patient_timeline 💔 $234.95$ $+1.21$ $+1$ $0$ $+14$ $0$
shinytest2-tm_g_pp_therapy 💔 $193.63$ $+7.58$ $0$ $0$ $+2$ $0$
shinytest2-tm_g_pp_vitals 💔 $79.03$ $+5.06$ $+1$ $0$ $+1$ $0$
shinytest2-tm_t_abnormality 💔 $65.45$ $+4.25$ $0$ $0$ $0$ $0$
shinytest2-tm_t_abnormality_by_worst_grade 💔 $74.30$ $+4.06$ $0$ $0$ $0$ $0$
shinytest2-tm_t_ancova 💔 $90.97$ $+5.87$ $0$ $0$ $0$ $0$
shinytest2-tm_t_binary_outcome 💔 $75.53$ $+5.67$ $0$ $0$ $0$ $0$
shinytest2-tm_t_coxreg 💔 $71.68$ $+4.99$ $0$ $0$ $0$ $0$
shinytest2-tm_t_events 💔 $58.33$ $+3.88$ $0$ $0$ $0$ $0$
shinytest2-tm_t_events_by_grade 💔 $88.91$ $+8.07$ $0$ $0$ $0$ $0$
shinytest2-tm_t_events_patyear 💔 $41.86$ $+3.25$ $0$ $0$ $0$ $0$
shinytest2-tm_t_events_summary 💔 $61.38$ $+6.02$ $0$ $0$ $0$ $0$
shinytest2-tm_t_exposure 💔 $79.47$ $+5.89$ $0$ $0$ $0$ $0$
shinytest2-tm_t_logistic 💔 $57.87$ $+4.36$ $0$ $0$ $0$ $0$
shinytest2-tm_t_mult_events 💔 $55.16$ $+5.05$ $0$ $0$ $0$ $0$
shinytest2-tm_t_pp_basic_info 💔 $42.41$ $+3.44$ $0$ $0$ $0$ $0$
shinytest2-tm_t_pp_laboratory 💔 $132.70$ $+26.37$ $0$ $0$ $0$ $0$
shinytest2-tm_t_pp_medical_history 💔 $64.52$ $+6.53$ $0$ $0$ $0$ $0$
shinytest2-tm_t_pp_prior_medication 💔 $80.61$ $+5.81$ $0$ $0$ $0$ $0$
shinytest2-tm_t_shift_by_arm 💔 $56.52$ $+6.39$ $0$ $0$ $0$ $0$
shinytest2-tm_t_shift_by_arm_by_worst 💔 $87.77$ $+8.51$ $0$ $0$ $0$ $0$
shinytest2-tm_t_shift_by_grade 💔 $80.45$ $+7.21$ $0$ $0$ $0$ $0$
shinytest2-tm_t_smq 💔 $55.80$ $+5.02$ $0$ $0$ $0$ $0$
shinytest2-tm_t_summary 💔 $38.26$ $+3.20$ $0$ $0$ $0$ $0$
shinytest2-tm_t_summary_by 💔 $77.50$ $+6.81$ $0$ $0$ $0$ $0$
shinytest2-tm_t_tte 💔 $64.81$ $+7.64$ $0$ $0$ $0$ $0$
Additional test case details
Test Suite $Status$ Time on main $±Time$ Test Case
shinytest2-tm_a_mmrm 💔 $32.51$ $+1.18$ e2e_tm_a_mmrm_Validate_output_on_different_selection_on_method_g_mmrm_diagnostic.
shinytest2-tm_a_mmrm 💔 $53.09$ $+2.08$ e2e_tm_a_mmrm_Validate_output_on_different_selection_on_method_t_mmrm_lsmeans.
shinytest2-tm_g_barchart_simple 💔 $9.24$ $+1.04$ e2e_tm_g_barchart_simple_Changing_facet_scale_x_changes_the_plot_and_does_not_throw_validation_errors.
shinytest2-tm_g_barchart_simple 💔 $7.96$ $+1.01$ e2e_tm_g_barchart_simple_Deselection_of_x_throws_validation_error.
shinytest2-tm_g_barchart_simple 💔 $11.64$ $+1.08$ e2e_tm_g_barchart_simple_Selection_of_fill_dataset_changes_the_element_and_does_not_throw_validation_errors.
shinytest2-tm_g_km 💔 $9.02$ $+1.06$ e2e_tm_g_km_Changing_yval_changes_the_plot_and_does_not_throw_validation_errors.
shinytest2-tm_g_pp_adverse_events 💔 $8.77$ $+2.99$ e2e_tm_g_pp_adverse_events_Module_initializes_in_teal_without_any_errors_and_produces_the_plot_and_table.
shinytest2-tm_t_events_by_grade 💔 $9.18$ $+1.06$ e2e_tm_t_events_by_grade_Selecting_hlt_changes_the_table_and_does_not_throw_validation_errors.
shinytest2-tm_t_events_by_grade 💔 $9.23$ $+1.08$ e2e_tm_t_events_by_grade_Selecting_llt_changes_the_table_and_does_not_throw_validation_errors.
shinytest2-tm_t_events_summary 💔 $7.57$ $+1.19$ e2e_tm_t_events_summary_Deselection_of_flag_var_aesi_changes_the_table_and_does_not_throw_validation_errors.
shinytest2-tm_t_pp_laboratory 💔 $8.97$ $+3.19$ e2e_tm_t_pp_laboratory_Deselection_of_aval_var_throws_validation_error.
shinytest2-tm_t_pp_laboratory 💔 $9.03$ $+2.90$ e2e_tm_t_pp_laboratory_Deselection_of_avalu_throws_validation_error.
shinytest2-tm_t_pp_laboratory 💔 $9.03$ $+2.96$ e2e_tm_t_pp_laboratory_Deselection_of_param_throws_validation_error.
shinytest2-tm_t_pp_laboratory 💔 $9.02$ $+3.22$ e2e_tm_t_pp_laboratory_Deselection_of_paramcd_throws_validation_error.
shinytest2-tm_t_pp_laboratory 💔 $9.10$ $+2.85$ e2e_tm_t_pp_laboratory_Deselection_of_patient_id_throws_validation_error.
shinytest2-tm_t_pp_laboratory 💔 $9.01$ $+3.33$ e2e_tm_t_pp_laboratory_Deselection_of_timepoints_throws_validation_error.
shinytest2-tm_t_pp_medical_history 💔 $13.04$ $+1.16$ e2e_tm_t_pp_medical_history_Selecting_mhbodsys_changes_the_table_and_does_not_throw_validation_errors.
shinytest2-tm_t_shift_by_arm 💔 $6.42$ $+1.02$ e2e_tm_t_shift_by_arm_Starts_with_specified_label_arm_varparamcd_visit_var_useNA_treatment_flag_var_add_total.
shinytest2-tm_t_shift_by_grade 💔 $9.24$ $+1.11$ e2e_tm_t_shift_by_grade_Deselection_of_paramcd_throws_validation_error.
shinytest2-tm_t_shift_by_grade 💔 $10.96$ $+1.04$ e2e_tm_t_shift_by_grade_Selecting_paramcd_changes_the_table_and_does_not_throw_validation_errors.
shinytest2-tm_t_summary_by 💔 $14.83$ $+1.30$ e2e_tm_t_summary_by_Deselection_of_arm_var_throws_validation_error.
shinytest2-tm_t_summary_by 💔 $23.70$ $+2.03$ e2e_tm_t_summary_by_Selecting_arm_var_changes_the_table_and_does_not_throw_validation_errors.
shinytest2-tm_t_tte 💔 $8.89$ $+3.30$ e2e_tm_t_tte_Module_initializes_in_teal_without_errors_and_produces_table_output.

Results for commit c87468a

♻️ This comment has been updated with latest results.

m7pr and others added 12 commits June 13, 2024 11:53
Merge branch '553_log_shiny_input_changes@main' into unify_plot_ns@main

# Conflicts:
#	R/tm_a_gee.R
#	R/tm_a_mmrm.R
#	R/tm_g_barchart_simple.R
#	R/tm_g_ci.R
#	R/tm_g_forest_rsp.R
#	R/tm_g_forest_tte.R
#	R/tm_g_ipp.R
#	R/tm_g_km.R
#	R/tm_g_lineplot.R
#	R/tm_g_pp_adverse_events.R
#	R/tm_g_pp_patient_timeline.R
#	R/tm_g_pp_therapy.R
#	R/tm_g_pp_vitals.R
#	R/tm_t_abnormality.R
#	R/tm_t_abnormality_by_worst_grade.R
#	R/tm_t_ancova.R
#	R/tm_t_binary_outcome.R
#	R/tm_t_coxreg.R
#	R/tm_t_events.R
#	R/tm_t_events_by_grade.R
#	R/tm_t_events_patyear.R
#	R/tm_t_events_summary.R
#	R/tm_t_exposure.R
#	R/tm_t_logistic.R
#	R/tm_t_mult_events.R
#	R/tm_t_pp_basic_info.R
#	R/tm_t_pp_laboratory.R
#	R/tm_t_pp_medical_history.R
#	R/tm_t_pp_prior_medication.R
#	R/tm_t_shift_by_arm.R
#	R/tm_t_shift_by_arm_by_worst.R
#	R/tm_t_shift_by_grade.R
#	R/tm_t_smq.R
#	R/tm_t_summary.R
#	R/tm_t_summary_by.R
#	R/tm_t_tte.R
#	R/utils.R
@m7pr m7pr changed the title Unify plot namespaces Unify plot and table namespaces Jun 13, 2024
@m7pr m7pr changed the base branch from main to 553_log_shiny_input_changes@main June 13, 2024 11:19
Base automatically changed from 553_log_shiny_input_changes@main to main June 19, 2024 11:24
@m7pr m7pr closed this Nov 25, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Nov 25, 2024
@insights-engineering-bot insights-engineering-bot deleted the unify_plot_ns@main branch December 1, 2024 03:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant