feat: added a formatting function for the output of `data_extract_srv` #60

kpagacz · 2022-05-13T15:10:49Z

Related to insightsengineering/teal.reporter#8

github-actions · 2022-05-13T15:16:39Z

Code Coverage Summary

Filename                          Stmts    Miss  Cover    Missing
------------------------------  -------  ------  -------  ---------------------------------------------------------------------
R/all_choices.R                       1       0  100.00%
R/call_utils.R                      156     124  20.51%   14-23, 64, 66, 68, 107-431
R/check_selector.R                   31       0  100.00%
R/choices_labeled.R                 202      61  69.80%   60, 71, 76, 87, 103, 221-225, 229-234, 264-277, 390-391, 393, 425-473
R/choices_selected.R                 81      11  86.42%   201-229, 260
R/column_functions.R                  3       3  0.00%    13-16
R/data_extract_datanames.R           32       8  75.00%   9-13, 64-66
R/data_extract_filter_module.R       91      32  64.84%   68-81, 83, 86-87, 89-105, 142
R/data_extract_module.R             205      20  90.24%   3, 121, 126, 143, 146-151, 153, 172-175, 222, 353, 358, 390, 396
R/data_extract_read_module.R        121      13  89.26%   27, 31-34, 101-106, 115, 132
R/data_extract_select_module.R       32      18  43.75%   31-48
R/data_extract_single_module.R       53       2  96.23%   29, 42
R/data_extract_spec.R                32       0  100.00%
R/data_merge_module.R                44      19  56.82%   102-123
R/filter_spec.R                     186       1  99.46%   373
R/format_data_extract.R              16       1  93.75%   53
R/get_dplyr_call.R                  298       0  100.00%
R/get_merge_call.R                  283      32  88.69%   29-36, 47, 210-219, 362, 369, 385-397, 460
R/include_css_js.R                    5       0  100.00%
R/input_checks.R                     11       2  81.82%   18-19
R/merge_data_utils.R                  2       0  100.00%
R/merge_datasets.R                  154       7  95.45%   71, 123, 251-255
R/resolve_delayed.R                 104      44  57.69%   220-303
R/select_spec.R                      49       8  83.67%   149, 213-220
R/utils.R                            15      11  26.67%   26-39
R/zzz.R                               2       2  0.00%    2-3
TOTAL                              2209     419  81.03%

Results for commit: 988dc88

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

github-actions · 2022-05-13T15:16:56Z

Unit Tests Summary

    1 files   20 suites 33s ⏱️
146 tests 145 ✔️ 1 💤 0 ❌
543 runs 542 ✔️ 1 💤 0 ❌

Results for commit c4ec672.

♻️ This comment has been updated with latest results.

nikolas-burkoff

I would add NEWS item for this
What about reshape? (Does that matter given des are likely to be replaced soon...)

vignettes/data-extract.Rmd

R/format_data_extract.R

nikolas-burkoff · 2022-05-16T08:23:27Z

R/format_data_extract.R

+#' }
+#' @export
+#'
+format_data_extract <- function(data_extract) {


I could imagine this being called in a reactive context so maybe we should not have the assert and stop?

Then, the reactive caller should catch the error and rethrow a shiny::validate error. This function does not know about shiny.

True - though in practice that will never be done of course.

This function will be being called when the add_to_report button is pressed so there's unlikely to be some weird initialization issue where this function is being called maybe with DDL where data_extract is say NULL? I get very confused as to the sequence of events during load up. You could imagine adding a if(is.null(data_exrtract)) return NULL or some such thing to make people lives a little easier if needed

Added a guard statement for null

R/format_data_extract.R

tests/testthat/test-format_data_extract.R

nikolas-burkoff · 2022-05-16T08:33:36Z

tests/testthat/test-format_data_extract.R

+    select = select_spec(choices = c("Petal.Length", "Species"))
+  )
+
+  shiny::testServer(


Yeah, it's not ideal because data_extract_srv does not output the structure with all values filled but just a skeleton due to the use of js for some logic.

A proper test boots up a headless browser for this, which I am keen on doing.

Co-authored-by: Nikolas Burkoff <[email protected]>

kpagacz · 2022-05-17T13:34:45Z

I would add NEWS item for this

What about reshape? (Does that matter given des are likely to be replaced soon...)

NEWS entry added

nikolas-burkoff

:(

nikolas-burkoff · 2022-05-18T07:57:40Z

tests/testthat/test-format_data_extract.R

+})
+
+testthat::test_that("format_data_extract integrates with data_extract_srv and the filtered data object", {
+  skip_if_too_deep(4)


Is 4 our convention for these tests?

I don't think there's a convention. This file takes around 10% of the whole test-suite duration, and most of it can be attributed to this particular test. I think it's unreasonable to run it always.

happy to not always run it - in other places (tmh?)I think we use 5 not 4

nikolas-burkoff

Approving, will let you decide what to do with the browser test

kpagacz · 2022-05-18T12:37:01Z

Oh crap. It was on auto merge, grr. I will open a new PR to fix the headless browser test

feat: added a formatting function for the output of data_extract_srv

84f48e6

Related to insightsengineering/teal.reporter#8

kpagacz added the core label May 13, 2022

kpagacz added 3 commits May 13, 2022 18:22

fixes

46061f5

fixes cont

84137b1

pkgdown

103e909

nikolas-burkoff self-assigned this May 16, 2022

nikolas-burkoff reviewed May 16, 2022

View reviewed changes

kpagacz and others added 3 commits May 17, 2022 14:25

Apply suggestions from code review [skip ci]

78d7bee

Co-authored-by: Nikolas Burkoff <[email protected]>

added shinytest tests

9469fcc

lint and style fixes

2592a2d

added new entry and a guard statement

c4ec672

kpagacz enabled auto-merge (squash) May 17, 2022 13:37

nikolas-burkoff reviewed May 18, 2022

View reviewed changes

nikolas-burkoff approved these changes May 18, 2022

View reviewed changes

kpagacz merged commit 0e870d5 into main May 18, 2022

kpagacz deleted the 8_teal.reporter_filter_list_print2@main branch May 18, 2022 09:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: added a formatting function for the output of `data_extract_srv` #60

feat: added a formatting function for the output of `data_extract_srv` #60

kpagacz commented May 13, 2022

github-actions bot commented May 13, 2022 •

edited

Loading

github-actions bot commented May 13, 2022 •

edited

Loading

nikolas-burkoff left a comment

nikolas-burkoff May 16, 2022

kpagacz May 16, 2022

nikolas-burkoff May 16, 2022

kpagacz May 17, 2022

nikolas-burkoff May 16, 2022

kpagacz May 16, 2022

kpagacz commented May 17, 2022

nikolas-burkoff left a comment

nikolas-burkoff May 18, 2022

kpagacz May 18, 2022

nikolas-burkoff May 18, 2022

nikolas-burkoff left a comment

kpagacz commented May 18, 2022

feat: added a formatting function for the output of data_extract_srv #60

feat: added a formatting function for the output of data_extract_srv #60

Conversation

kpagacz commented May 13, 2022

github-actions bot commented May 13, 2022 • edited Loading

Code Coverage Summary

github-actions bot commented May 13, 2022 • edited Loading

Unit Tests Summary

nikolas-burkoff left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kpagacz commented May 17, 2022

nikolas-burkoff left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nikolas-burkoff left a comment

Choose a reason for hiding this comment

kpagacz commented May 18, 2022

feat: added a formatting function for the output of `data_extract_srv` #60

feat: added a formatting function for the output of `data_extract_srv` #60

github-actions bot commented May 13, 2022 •

edited

Loading

github-actions bot commented May 13, 2022 •

edited

Loading