Skip to content

Conversation

@llrs-roche
Copy link
Contributor

Pull Request

Fixes #786

This PR load the packages needed before the code is evaluated inside the qenv.
This makes the modules' code more reproducible and solve these errors:

image

We (@m7pr + me) decided to still add the prefixes on functions to avoid namespace conflicts.

@llrs-roche llrs-roche requested a review from m7pr February 17, 2025 10:37
@m7pr
Copy link
Contributor

m7pr commented Feb 17, 2025

This change fixes #786 + adds direct library calls, so that Show R Code panel actually includes them.
So the code from Show R Code was not reproducible before.

We decided to pay attention to prefixes, to avoid name conflicts. Hovewer we decided not to prefix operators + and %>% - those need direct library(package) call. magrittr is not used as we import pipe from dplyr.

@llrs-roche llrs-roche marked this pull request as ready for review February 17, 2025 16:36
@llrs-roche
Copy link
Contributor Author

I think it is ready for review, I'll fix the SuperLinter issues with the review comments.

@llrs-roche llrs-roche requested a review from m7pr February 17, 2025 16:37
@github-actions
Copy link
Contributor

github-actions bot commented Feb 17, 2025

badge

Code Coverage Summary

Filename                      Stmts    Miss  Cover    Missing
--------------------------  -------  ------  -------  ---------------------------------------
R/tm_a_pca.R                    888     888  0.00%    136-1157
R/tm_a_regression.R             777     777  0.00%    175-1057
R/tm_data_table.R               211     211  0.00%    100-359
R/tm_file_viewer.R              173     173  0.00%    47-255
R/tm_front_page.R               144     133  7.64%    77-247
R/tm_g_association.R            346     346  0.00%    156-577
R/tm_g_bivariate.R              690     426  38.26%   328-815, 856, 967, 984, 1002, 1013-1035
R/tm_g_distribution.R          1123    1123  0.00%    153-1423
R/tm_g_response.R               369     369  0.00%    174-622
R/tm_g_scatterplot.R            731     731  0.00%    257-1092
R/tm_g_scatterplotmatrix.R      296     277  6.42%    195-528, 589, 603
R/tm_missing_data.R            1133    1133  0.00%    121-1431
R/tm_outliers.R                1044    1044  0.00%    160-1355
R/tm_t_crosstable.R             261     261  0.00%    160-469
R/tm_variable_browser.R         832     827  0.60%    89-1078, 1116-1299
R/utils.R                       151     135  10.60%   89-274, 304-340, 352-361, 366, 380-399
R/zzz.R                           2       2  0.00%    2-3
TOTAL                          9171    8856  3.43%

Diff against main

Filename                      Stmts    Miss  Cover
--------------------------  -------  ------  --------
R/tm_a_pca.R                     +3      +3  +100.00%
R/tm_a_regression.R              +4      +4  +100.00%
R/tm_data_table.R                +4      +4  +100.00%
R/tm_g_association.R             +5      +5  +100.00%
R/tm_g_bivariate.R               +4      +4  -0.22%
R/tm_g_distribution.R           +11     +11  +100.00%
R/tm_g_response.R                +4      +4  +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              +7      +7  +100.00%
R/tm_outliers.R                  +9      +9  +100.00%
R/tm_t_crosstable.R              +1      +1  +100.00%
TOTAL                           +54     +54  -0.02%

Results for commit: 3037961

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

@github-actions
Copy link
Contributor

github-actions bot commented Feb 17, 2025

Unit Tests Summary

  1 files   22 suites   13m 43s ⏱️
144 tests 144 ✅ 0 💤 0 ❌
475 runs  475 ✅ 0 💤 0 ❌

Results for commit 3037961.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 17, 2025

Unit Test Performance Difference

Test Suite $Status$ Time on main $±Time$ $±Tests$ $±Skipped$ $±Failures$ $±Errors$
shinytest2-tm_a_pca 💔 $117.36$ $+4.90$ $0$ $0$ $0$ $0$
shinytest2-tm_a_regression 💔 $53.60$ $+2.58$ $0$ $0$ $0$ $0$
shinytest2-tm_file_viewer 💔 $29.20$ $+1.06$ $0$ $0$ $0$ $0$
shinytest2-tm_front_page 💔 $20.72$ $+1.13$ $0$ $0$ $0$ $0$
shinytest2-tm_g_association 💔 $31.37$ $+2.18$ $0$ $0$ $0$ $0$
shinytest2-tm_g_bivariate 💔 $75.17$ $+1.93$ $0$ $0$ $0$ $0$
shinytest2-tm_g_distribution 💔 $58.06$ $+2.10$ $0$ $0$ $0$ $0$
shinytest2-tm_g_response 💔 $29.68$ $+1.59$ $0$ $0$ $0$ $0$
shinytest2-tm_g_scatterplot 💔 $73.91$ $+2.92$ $0$ $0$ $0$ $0$
shinytest2-tm_g_scatterplotmatrix 💔 $28.00$ $+1.64$ $0$ $0$ $0$ $0$
shinytest2-tm_misssing_data 💔 $47.49$ $+1.97$ $0$ $0$ $0$ $0$
shinytest2-tm_outliers 💔 $111.22$ $+6.96$ $0$ $0$ $0$ $0$
shinytest2-tm_t_crosstable 💔 $31.34$ $+1.80$ $0$ $0$ $0$ $0$
shinytest2-tm_variable_browser 💔 $56.53$ $+2.31$ $0$ $0$ $0$ $0$
Additional test case details
Test Suite $Status$ Time on main $±Time$ Test Case
shinytest2-tm_g_bivariate 💔 $25.88$ $+1.18$ e2e_tm_g_bivariate_Setting_encoding_inputs_produces_outputs_without_validation_errors.
shinytest2-tm_g_scatterplot 💔 $42.71$ $+1.22$ e2e_tm_g_scatterplot_The_encoding_inputs_produce_output_without_validation_errors.
shinytest2-tm_outliers 💔 $9.98$ $+1.07$ e2e_tm_outliers_Data_extract_spec_elements_are_initialized_with_the_default_values_specified_by_outlier_var_and_categorical_var_argument.
shinytest2-tm_outliers 💔 $25.33$ $+1.24$ e2e_tm_outliers_Outlier_definition_text_and_range_are_displayed_properly_depending_on_method.

Results for commit 934bf2a

♻️ This comment has been updated with latest results.

@m7pr
Copy link
Contributor

m7pr commented Feb 18, 2025

hey @llrs-roche , really great job with the modules on tmg! I will review each module separately! it might take some time : )

In the meantime, can you have a look at the SuperLinter fail?

Copy link
Contributor Author

@llrs-roche llrs-roche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressing the remaining comments.
I also found some other calls missing a prefix.

@llrs-roche llrs-roche merged commit c567990 into main Feb 20, 2025
28 of 29 checks passed
@llrs-roche llrs-roche deleted the 786_attached_packages@main branch February 20, 2025 08:21
@github-actions github-actions bot locked and limited conversation to collaborators Feb 20, 2025
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.

[Bug]: modules crash if package not attached

5 participants