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

reporter cleaning #430

Merged
merged 6 commits into from
Jul 27, 2022
Merged

reporter cleaning #430

merged 6 commits into from
Jul 27, 2022

Conversation

mhallal1
Copy link
Collaborator

part of insightsengineering/teal.reporter#88, insightsengineering/teal.reporter#120 and insightsengineering/teal.slice#56

  • Use UI and SRV from simple_reporter
  • Remove adding title for Filter state and Show R code

@mhallal1 mhallal1 added the core label Jul 22, 2022
@mhallal1 mhallal1 changed the title simple_reporter reporter cleaning Jul 22, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Jul 22, 2022

Code Coverage Summary

Filename                      Stmts    Miss  Cover    Missing
--------------------------  -------  ------  -------  ----------------------------------------------
R/tm_a_pca.R                    804     804  0.00%    70-1008
R/tm_a_regression.R             719     719  0.00%    91-902
R/tm_data_table.R               168     168  0.00%    147-375
R/tm_file_viewer.R              169     169  0.00%    38-237
R/tm_front_page.R               122     111  9.02%    61-197
R/tm_g_association.R            287     287  0.00%    88-430
R/tm_g_bivariate.R              616     456  25.97%   105-642, 690, 696, 700, 811, 828, 846, 866-888
R/tm_g_distribution.R           974     974  0.00%    107-1235
R/tm_g_response.R               323     323  0.00%    84-470
R/tm_g_scatterplot.R            602     602  0.00%    141-823
R/tm_g_scatterplotmatrix.R      260     242  6.92%    79-354, 407, 421
R/tm_missing_data.R            1033    1033  0.00%    56-1246
R/tm_outliers.R                 989     989  0.00%    71-1195
R/tm_t_crosstable.R             246     246  0.00%    80-368
R/tm_variable_browser.R         757     752  0.66%    46-1198
R/utils.R                        89      89  0.00%    74-269
R/zzz.R                           1       1  0.00%    2
TOTAL                          8159    7965  2.38%

Results for commit: 160436541eae5e69753dbb09c17bf939723ba932

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

@github-actions
Copy link
Contributor

github-actions bot commented Jul 22, 2022

Unit Tests Summary

  1 files    5 suites   0s ⏱️
16 tests 16 ✔️ 0 💤 0
49 runs  49 ✔️ 0 💤 0

Results for commit 35796f4.

♻️ This comment has been updated with latest results.

@nikolas-burkoff nikolas-burkoff self-assigned this Jul 26, 2022
Copy link
Contributor

@nikolas-burkoff nikolas-burkoff left a comment

Choose a reason for hiding this comment

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

Just a few minor comments but this is good to be merged

@@ -432,7 +427,6 @@ srv_variable_browser <- function(id, datasets, reporter, datasets_selected, ggpl
card <- teal.reporter::TealReportCard$new()
card$set_name("Variable Browser Plot")
card$append_text("Variable Browser Plot", "header2")
card$append_text("Filter State", "header3")
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm so we need to record whether the use filtered data button was turned on or not as otherwise we could have the situation where say "Filtered State: x" but the Use filtered state button is set to FALSE and then the graph includes unfiltered data. Maybe we have if (input$<<use_filter>>) card$append_fs(...)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good point, updated.

@@ -352,27 +347,22 @@ srv_t_crosstable <- function(id, datasets, reporter, label, x, y, basic_table_ar
card <- teal.reporter::TealReportCard$new()
card$set_name("Cross Table")
card$append_text("Cross Table", "header2")
card$append_text("Filter State", "header3")
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have an issue to add encoding information into tmg modules as I could imagine say waning to know which join we've selected here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@mhallal1 mhallal1 merged commit f12253f into main Jul 27, 2022
@mhallal1 mhallal1 deleted the 120_append_fs_title@main branch July 27, 2022 09:13
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.

2 participants