Skip to content

Conversation

@averissimo
Copy link
Contributor

@averissimo averissimo commented Oct 3, 2025

Pull Request

Companion to

Open questions

To @donyunardi and @kumamiao

  • Should we always keep in reporter? (see screenshots below for the sections I mention)
    • Data pre-processing
    • Filtering

Context:

  • The previous version of the reporter had a section that described the active filters.
    • This will be lost if we remove all code.
image

Previous filter state

image

Changes description

  • Data pre-processing and Filtering are always kept in report
    • Even when de-selecting option to remove R code
image

@github-actions
Copy link
Contributor

github-actions bot commented Oct 3, 2025

badge

Code Coverage Summary

Filename                          Stmts    Miss  Cover    Missing
------------------------------  -------  ------  -------  --------------------------------------------------------------------------------------------------------------------------------------
R/after.R                            63      63  0.00%    15-89
R/checkmate.R                        24       0  100.00%
R/dummy_functions.R                  61       2  96.72%   44, 46
R/include_css_js.R                   11       0  100.00%
R/init.R                            152       1  99.34%   299
R/landing_popup_module.R             34      10  70.59%   44-53
R/module_bookmark_manager.R         153     117  23.53%   54-58, 78-133, 138-139, 151, 198, 233-310
R/module_data_summary.R             177       8  95.48%   40, 50, 205, 236-240
R/module_filter_data.R               64       0  100.00%
R/module_filter_manager.R           229      50  78.17%   72-81, 89-94, 107-111, 116-117, 290-313, 339, 366, 378, 385-386
R/module_init_data.R                 91       6  93.41%   38-43
R/module_nested_tabs.R              371      37  90.03%   163, 267-282, 302-306, 324, 361, 479-482, 486-489, 493-496, 541
R/module_session_info.R              18       0  100.00%
R/module_snapshot_manager.R         271     194  28.41%   103-112, 120-144, 163-164, 181-210, 214-229, 231-238, 245-275, 279, 283-287, 289-295, 298-311, 314-322, 352-366, 369-380, 383-397, 410
R/module_source_code.R               69       7  89.86%   54, 58, 107-111
R/module_teal_data.R                149      76  48.99%   43-149
R/module_teal_lockfile.R            131      53  59.54%   45-57, 60-62, 76, 86-88, 100-102, 110-119, 122, 124, 126-127, 142-146, 161-162, 177-186
R/module_teal_reporter.R            114      16  85.96%   60, 77-78, 81, 98, 125, 139, 141, 153, 155, 157, 202-206
R/module_teal_with_splash.R          33      33  0.00%    24-61
R/module_teal.R                     204      24  88.24%   130, 141-142, 182, 200-216, 218, 254-255
R/module_transform_data.R           116       6  94.83%   46, 130-134
R/modules.R                         291      51  82.47%   170-174, 229-232, 356-376, 384, 390, 567-573, 586-594, 609-624
R/reporter_previewer_module.R        41      41  0.00%    22-85
R/show_rcode_modal.R                 31      31  0.00%    17-49
R/tdata.R                            14      14  0.00%    19-61
R/teal_data_module-eval_code.R       23       0  100.00%
R/teal_data_module-within.R           7       0  100.00%
R/teal_data_module.R                 20       0  100.00%
R/teal_data_utils.R                  46       0  100.00%
R/teal_modifiers.R                   57       0  100.00%
R/teal_slices-store.R                29       0  100.00%
R/teal_slices.R                      63       0  100.00%
R/teal_transform_module.R            45       0  100.00%
R/TealAppDriver.R                   343     343  0.00%    50-711
R/utils.R                           291      48  83.51%   402-451, 539-548
R/validate_inputs.R                  32       0  100.00%
R/validations.R                      58      37  36.21%   114-392
R/zzz.R                              19      15  21.05%   4-22
TOTAL                              3945    1283  67.48%

Diff against main

Filename                Stmts    Miss  Cover
--------------------  -------  ------  --------
R/module_init_data.R       +7       0  +0.55%
R/teal_data_utils.R        +3       0  +100.00%
TOTAL                     +10       0  +0.08%

Results for commit: c8f555b

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

@github-actions
Copy link
Contributor

github-actions bot commented Oct 3, 2025

Unit Tests Summary

  1 files   27 suites   2m 20s ⏱️
310 tests 250 ✅ 60 💤 0 ❌
512 runs  452 ✅ 60 💤 0 ❌

Results for commit c8f555b.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 3, 2025

Unit Test Performance Difference

Test Suite $Status$ Time on main $±Time$ $±Tests$ $±Skipped$ $±Failures$ $±Errors$
module_teal 💚 $109.31$ $-1.03$ $0$ $0$ $0$ $0$
shinytest2-show-rcode 💀 $0.29$ $-0.29$ $-3$ $-3$ $0$ $0$
shinytest2-teal_modifiers 💀 $0.54$ $-0.54$ $-7$ $-7$ $0$ $0$
Additional test case details
Test Suite $Status$ Time on main $±Time$ Test Case
shinytest2-reporter 👶 $+0.02$ unnamed
shinytest2-show-rcode 💀 $0.09$ $-0.09$ e2e_Module_with_Show_R_Code_has_code
shinytest2-show-rcode 💀 $0.09$ $-0.09$ e2e_Module_with_Show_R_Code_has_modal_with_two_dismiss_and_two_copy_to_clipboard_buttons
shinytest2-show-rcode 💀 $0.11$ $-0.11$ e2e_Module_with_Show_R_Code_initializes_with_visible_button
shinytest2-teal_data_module 👶 $+0.01$ unnamed
shinytest2-teal_modifiers 💀 $0.07$ $-0.07$ e2e_add_landing_modal_displays_landing_modal_on_app_startup
shinytest2-teal_modifiers 💀 $0.08$ $-0.08$ e2e_add_landing_modal_modal_can_be_dismissed
shinytest2-teal_modifiers 💀 $0.08$ $-0.08$ e2e_combined_modifiers_displays_all_customizations_when_chained_together
shinytest2-teal_modifiers 💀 $0.07$ $-0.07$ e2e_modify_footer_displays_custom_footer_in_the_app
shinytest2-teal_modifiers 💀 $0.08$ $-0.08$ e2e_modify_header_displays_custom_header_in_the_app
shinytest2-teal_modifiers 💀 $0.09$ $-0.09$ e2e_modify_title_sets_custom_title_in_the_page_title_head_title_displays_custom_favicon_in_the_app
shinytest2-teal_modifiers 💀 $0.08$ $-0.08$ e2e_modify_title_sets_custom_title_in_the_page_title_head_title_displays_custom_title_in_the_app

Results for commit c7cff71

♻️ This comment has been updated with latest results.

@m7pr m7pr requested review from Copilot and m7pr October 7, 2025 10:53
@m7pr m7pr self-assigned this Oct 7, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR modifies the teal.reporter to always preserve certain R code sections in reports, specifically "Data pre-processing" and "Filtering" sections, even when users choose to remove R code from reports. This ensures that important contextual information about data transformation and filtering states remains visible in generated reports.

Key changes:

  • Adds always_keep = TRUE parameter to code chunks for data filtering
  • Marks data preparation code chunks as always kept using the always_keep attribute
  • Updates code chunk merging logic to preserve the always_keep attribute when combining chunks

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
R/teal_data_utils.R Adds always_keep = TRUE to filtering code chunks and updates merge logic to preserve this attribute
R/module_init_data.R Marks all data preparation code chunks with always_keep = TRUE attribute

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@m7pr
Copy link
Contributor

m7pr commented Oct 7, 2025

This is how it looks for regression module, if you don't include the code (initial data setup code is still visible)

image

and the comparison on how it looks with the code included

image

Copy link
Contributor

@m7pr m7pr left a comment

Choose a reason for hiding this comment

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

From the code's perspective, all looks good

@gogonzo gogonzo self-requested a review October 7, 2025 12:03
@gogonzo gogonzo self-assigned this Oct 7, 2025
@kumamiao
Copy link
Contributor

kumamiao commented Oct 7, 2025

Thanks @m7pr for following up on this one. My comments on the inclusion/exclusion:

  1. Whether the app user chooses to include or not include the R code, we should always include the filter state (as we did previously), as this is critical information to inform the app user what is the analysis created. For example, if they did not add in the title/comment, they would not know what population the analysis was applied to. This was specifically requested before if I recall correctly.
image
  1. If user chooses to not include the R code, let's not show the code for Data pre-processing or Data Filtering. The filter state itself will be sufficient.

@gogonzo
Copy link
Contributor

gogonzo commented Oct 8, 2025

  1. If user chooses to not include the R code, let's not show the code for Data pre-processing or Data Filtering. The filter state itself will be sufficient.

What if transformator is applied with some custom filters? Filter-state won't be sufficient information then about data transformation.

This was specifically requested before if I recall correctly.

How important filter state is? Please give us more background what is the meaning of having information which IMO is incomplete. Let's discuss how final report should look like.

@m7pr
Copy link
Contributor

m7pr commented Oct 10, 2025

Hey @gogonzo we decided together with team today during a catch-up that if code is not showed we will add a small note below the Filter State, that says "further data manipulations and filtering could be applied with transformers for which you did not want to see the code". Stuff like that that suggests that there might be more manipulations. And when code is included, then this message is not shown.

The motivation for Filter State is that people tend to recognize/differentiate various report cards based on the Filter State.

@gogonzo
Copy link
Contributor

gogonzo commented Oct 14, 2025

We decided to do this other way. Thanks @averissimo

@gogonzo gogonzo closed this Oct 14, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Oct 14, 2025
@gogonzo gogonzo deleted the 400-skip_headers_when_no_code branch October 14, 2025 13:34
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]: Downloading report without code keeps Module's Code section

5 participants