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

Remove "default" decoration and use the same way of decoration in all modules #846

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

vedhav
Copy link
Contributor

@vedhav vedhav commented Feb 14, 2025

Closes #845

Changes:

  1. Removes the ability to have a "default" decoration applied to all output objects.
  2. Makes sure that all our modules follow the same decoration format: decorators = list(output_name = teal_transform_module(...))
  3. Improve the error message so the user knows that they have to provide the names from the available list of names.

@vedhav vedhav added the core label Feb 14, 2025
Copy link
Contributor

badge

Code Coverage Summary

Filename                      Stmts    Miss  Cover    Missing
--------------------------  -------  ------  -------  --------------------------------------
R/tm_a_pca.R                    885     885  0.00%    136-1155
R/tm_a_regression.R             773     773  0.00%    175-1052
R/tm_data_table.R               207     207  0.00%    100-356
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            341     341  0.00%    156-572
R/tm_g_bivariate.R              686     422  38.48%   328-810, 851, 962, 979, 997, 1008-1030
R/tm_g_distribution.R          1112    1112  0.00%    154-1411
R/tm_g_response.R               365     365  0.00%    174-617
R/tm_g_scatterplot.R            730     730  0.00%    257-1091
R/tm_g_scatterplotmatrix.R      295     276  6.44%    195-527, 588, 602
R/tm_missing_data.R            1126    1126  0.00%    122-1425
R/tm_outliers.R                1035    1035  0.00%    161-1347
R/tm_t_crosstable.R             260     260  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                          9117    8802  3.46%

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_g_association.R             -1      -1  +100.00%
R/tm_g_bivariate.R               -1       0  -0.09%
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/utils.R                       -15      -1  -7.48%
TOTAL                           -26     -11  -0.15%

Results for commit: 40d8f5c

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

Copy link
Contributor

Unit Tests Summary

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

Results for commit 40d8f5c.

Copy link
Contributor

Unit Tests Summary

  1 files   22 suites   12m 51s ⏱️
144 tests 107 ✅ 37 💤 0 ❌
474 runs  437 ✅ 37 💤 0 ❌

Results for commit 40d8f5c.

Copy link
Contributor

github-actions bot commented Feb 14, 2025

Unit Test Performance Difference

Test Suite $Status$ Time on main $±Time$ $±Tests$ $±Skipped$ $±Failures$ $±Errors$
shinytest2-tm_a_pca 💚 $116.36$ $-1.22$ $0$ $0$ $0$ $0$
Additional test case details
Test Suite $Status$ Time on main $±Time$ Test Case
examples 💀 $0.01$ $-0.01$ example_normalize_decorators.Rd

Results for commit db00a62

♻️ This comment has been updated with latest results.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Question]: Do we need the default in the decorators?
1 participant