From 0a631058b4c8fb697e217e4e50d61b43e3ce55e6 Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Wed, 11 Sep 2024 15:22:50 +0000 Subject: [PATCH 01/16] use results as metrics, set abbreviation to metricid --- R/Report_FlagOverTime.R | 32 +++++++++++++++++++++++--------- R/Widget_FlagOverTime.R | 2 +- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/R/Report_FlagOverTime.R b/R/Report_FlagOverTime.R index 16aaca6ae..1a53501ce 100644 --- a/R/Report_FlagOverTime.R +++ b/R/Report_FlagOverTime.R @@ -13,7 +13,7 @@ #' @export Report_FlagOverTime <- function( dfResults, - dfMetrics, + dfMetrics = NULL, strGroupLevel = c("Site", "Study", "Country") ) { strGroupLevel <- rlang::arg_match(strGroupLevel) @@ -54,14 +54,28 @@ flag_changes <- function(dfResults) { } widen_results <- function(dfResults, dfMetrics, strGroupLevel) { - dfMetrics_join <- dfMetrics %>% - dplyr::mutate(GroupLevel = stringr::str_to_sentence(.data$GroupLevel)) %>% - dplyr::filter(.data$GroupLevel == strGroupLevel) %>% - dplyr::select( - "MetricID", - "Abbreviation", - "GroupLevel" - ) + if(!is.null(dfMetrics)) { + dfMetrics_join <- dfMetrics %>% + dplyr::mutate(GroupLevel = stringr::str_to_sentence(.data$GroupLevel)) %>% + dplyr::filter(.data$GroupLevel == strGroupLevel) %>% + dplyr::select( + "MetricID", + "Abbreviation", + "GroupLevel" + ) + } + else { + dfMetrics_join <- dfResults %>% + dplyr::mutate(GroupLevel = stringr::str_to_sentence(.data$GroupLevel), + Abbreviation = MetricID) %>% + dplyr::filter(.data$GroupLevel == strGroupLevel) %>% + dplyr::select( + "MetricID", + "Abbreviation", + "GroupLevel" + ) %>% + unique() + } dfFlagOverTime <- dfResults %>% dplyr::mutate(GroupLevel = stringr::str_to_sentence(.data$GroupLevel)) %>% dplyr::inner_join(dfMetrics_join, by = c("MetricID", "GroupLevel")) %>% diff --git a/R/Widget_FlagOverTime.R b/R/Widget_FlagOverTime.R index 28a67df4d..d4366567f 100644 --- a/R/Widget_FlagOverTime.R +++ b/R/Widget_FlagOverTime.R @@ -17,7 +17,7 @@ #' @export Widget_FlagOverTime <- function( dfResults, - dfMetrics, + dfMetrics = NULL, strGroupLevel = c("Site", "Study", "Country") ) { gtFlagOverTime <- Report_FlagOverTime( From f91e4ecdd328314772c8cc25560e31c8c13ba310 Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Wed, 11 Sep 2024 16:41:37 +0000 Subject: [PATCH 02/16] runs except `Report_MetricTable()` --- R/Visualize_Metric.R | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/R/Visualize_Metric.R b/R/Visualize_Metric.R index 3aa747066..cae797c60 100644 --- a/R/Visualize_Metric.R +++ b/R/Visualize_Metric.R @@ -48,7 +48,7 @@ Visualize_Metric <- function( dfResults$SnapshotDate <- as.Date(Sys.Date()) } - if (!"SnapshotDate" %in% colnames(dfBounds)) { + if (!"SnapshotDate" %in% colnames(dfBounds) & !is.null(dfBounds)) { dfBounds$SnapshotDate <- as.Date(Sys.Date()) } @@ -80,15 +80,22 @@ Visualize_Metric <- function( cli::cli_abort("Multiple MetricIDs found in dfResults, dfBounds or dfMetrics. Specify `MetricID` to subset. No charts will be generated.") return(NULL) } - # Prep chart inputs --------------------------------------------------------- - lMetric <- as.list(dfMetrics) - vThreshold <- ParseThreshold(lMetric$Threshold) + if(is.null(dfMetrics)){ + lMetric <- NULL + } else { + lMetric <- as.list(dfMetrics) + } + vThreshold <- ifelse(length(lMetric) != 0, ParseThreshold(lMetric$Threshold), c(-2, -1, 2, 3)) # Cross-sectional Charts using most recent snapshot ------------------------ lCharts <- list() dfResults_latest <- FilterByLatestSnapshotDate(dfResults, strSnapshotDate) - dfBounds_latest <- FilterByLatestSnapshotDate(dfBounds, strSnapshotDate) + if(is.null(dfBounds)) { + dfBounds_latest <- NULL + } else{ + dfBounds_latest <- FilterByLatestSnapshotDate(dfBounds, strSnapshotDate) + } if (nrow(dfResults_latest) == 0) { cli::cli_alert_warning("No data found for specified snapshot date: {strSnapshotDate}. No charts will be generated.") @@ -134,11 +141,11 @@ Visualize_Metric <- function( vThreshold = vThreshold ) - lCharts$metricTable <- Report_MetricTable( - dfResults = dfResults_latest, - dfGroups = dfGroups, - strGroupLevel = lMetric$GroupLevel - ) + # lCharts$metricTable <- Report_MetricTable( + # dfResults = dfResults_latest, + # dfGroups = dfGroups, + # strGroupLevel = lMetric$GroupLevel + # ) } # Continuous Charts ------------------------------------------------------- if (number_of_snapshots <= 1) { From d77a3ee716e713aafe7d0c6e0e98419bad225a22 Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Wed, 11 Sep 2024 16:58:57 +0000 Subject: [PATCH 03/16] can Report_MetricTable be renderd without group --- R/Visualize_Metric.R | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/R/Visualize_Metric.R b/R/Visualize_Metric.R index cae797c60..a40333aa2 100644 --- a/R/Visualize_Metric.R +++ b/R/Visualize_Metric.R @@ -140,12 +140,16 @@ Visualize_Metric <- function( strType = "Score", vThreshold = vThreshold ) - - # lCharts$metricTable <- Report_MetricTable( - # dfResults = dfResults_latest, - # dfGroups = dfGroups, - # strGroupLevel = lMetric$GroupLevel - # ) + if(!is.null(dfGroups)) { + lCharts$metricTable <- Report_MetricTable( + dfResults = dfResults_latest, + dfGroups = dfGroups, + strGroupLevel = unique(dfGroups$GroupLevel) + ) + } + else { + cli_inform("Group Metric Table was not rendered") + } } # Continuous Charts ------------------------------------------------------- if (number_of_snapshots <= 1) { From a9e9164aaec3a48aead8507810d18c090e7aaab9 Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Wed, 11 Sep 2024 22:36:27 +0000 Subject: [PATCH 04/16] undo initial accident --- R/Report_FlagOverTime.R | 60 +++++++++++++++----------------------- man/Widget_FlagOverTime.Rd | 2 +- 2 files changed, 24 insertions(+), 38 deletions(-) diff --git a/R/Report_FlagOverTime.R b/R/Report_FlagOverTime.R index 1a53501ce..af57a2fd2 100644 --- a/R/Report_FlagOverTime.R +++ b/R/Report_FlagOverTime.R @@ -12,9 +12,9 @@ #' @inherit gt-shared return #' @export Report_FlagOverTime <- function( - dfResults, - dfMetrics = NULL, - strGroupLevel = c("Site", "Study", "Country") + dfResults, + dfMetrics, + strGroupLevel = c("Site", "Study", "Country") ) { strGroupLevel <- rlang::arg_match(strGroupLevel) dfFlagOverTime <- dfResults %>% @@ -54,28 +54,14 @@ flag_changes <- function(dfResults) { } widen_results <- function(dfResults, dfMetrics, strGroupLevel) { - if(!is.null(dfMetrics)) { - dfMetrics_join <- dfMetrics %>% - dplyr::mutate(GroupLevel = stringr::str_to_sentence(.data$GroupLevel)) %>% - dplyr::filter(.data$GroupLevel == strGroupLevel) %>% - dplyr::select( - "MetricID", - "Abbreviation", - "GroupLevel" - ) - } - else { - dfMetrics_join <- dfResults %>% - dplyr::mutate(GroupLevel = stringr::str_to_sentence(.data$GroupLevel), - Abbreviation = MetricID) %>% - dplyr::filter(.data$GroupLevel == strGroupLevel) %>% - dplyr::select( - "MetricID", - "Abbreviation", - "GroupLevel" - ) %>% - unique() - } + dfMetrics_join <- dfMetrics %>% + dplyr::mutate(GroupLevel = stringr::str_to_sentence(.data$GroupLevel)) %>% + dplyr::filter(.data$GroupLevel == strGroupLevel) %>% + dplyr::select( + "MetricID", + "Abbreviation", + "GroupLevel" + ) dfFlagOverTime <- dfResults %>% dplyr::mutate(GroupLevel = stringr::str_to_sentence(.data$GroupLevel)) %>% dplyr::inner_join(dfMetrics_join, by = c("MetricID", "GroupLevel")) %>% @@ -108,17 +94,17 @@ fmt_flag_rag <- function(data, columns = gt::everything()) { # Cells ------------------------------------------------------------------------ fmt_sign_rag <- function( - data, - columns = gt::everything(), - rows = gt::everything()) { + data, + columns = gt::everything(), + rows = gt::everything()) { data_color_rag(data, columns = columns) %>% fmt_sign(columns = columns, rows = rows) } data_color_rag <- function( - data, - columns = gt::everything(), - rows = gt::everything()) { + data, + columns = gt::everything(), + rows = gt::everything()) { gt::data_color( data, columns = columns, @@ -128,9 +114,9 @@ data_color_rag <- function( } fmt_sign <- function( - data, - columns = gt::everything(), - rows = gt::everything()) { + data, + columns = gt::everything(), + rows = gt::everything()) { gt::fmt( data, columns = columns, @@ -160,9 +146,9 @@ n_to_rag <- function(x) { } fmt_present <- function( - data, - columns = gt::everything(), - rows = gt::everything()) { + data, + columns = gt::everything(), + rows = gt::everything()) { gt::fmt( data, columns = columns, diff --git a/man/Widget_FlagOverTime.Rd b/man/Widget_FlagOverTime.Rd index 0472c20c3..16472fd8a 100644 --- a/man/Widget_FlagOverTime.Rd +++ b/man/Widget_FlagOverTime.Rd @@ -6,7 +6,7 @@ \usage{ Widget_FlagOverTime( dfResults, - dfMetrics, + dfMetrics = NULL, strGroupLevel = c("Site", "Study", "Country") ) } From 3338861986b2858138b6ad011486e4412b7fd4fa Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Wed, 11 Sep 2024 22:45:47 +0000 Subject: [PATCH 05/16] undo unnecessary tabs --- R/Report_FlagOverTime.R | 30 +++++++++++++++--------------- R/Widget_FlagOverTime.R | 2 +- man/Widget_FlagOverTime.Rd | 2 +- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/R/Report_FlagOverTime.R b/R/Report_FlagOverTime.R index af57a2fd2..16aaca6ae 100644 --- a/R/Report_FlagOverTime.R +++ b/R/Report_FlagOverTime.R @@ -12,9 +12,9 @@ #' @inherit gt-shared return #' @export Report_FlagOverTime <- function( - dfResults, - dfMetrics, - strGroupLevel = c("Site", "Study", "Country") + dfResults, + dfMetrics, + strGroupLevel = c("Site", "Study", "Country") ) { strGroupLevel <- rlang::arg_match(strGroupLevel) dfFlagOverTime <- dfResults %>% @@ -94,17 +94,17 @@ fmt_flag_rag <- function(data, columns = gt::everything()) { # Cells ------------------------------------------------------------------------ fmt_sign_rag <- function( - data, - columns = gt::everything(), - rows = gt::everything()) { + data, + columns = gt::everything(), + rows = gt::everything()) { data_color_rag(data, columns = columns) %>% fmt_sign(columns = columns, rows = rows) } data_color_rag <- function( - data, - columns = gt::everything(), - rows = gt::everything()) { + data, + columns = gt::everything(), + rows = gt::everything()) { gt::data_color( data, columns = columns, @@ -114,9 +114,9 @@ data_color_rag <- function( } fmt_sign <- function( - data, - columns = gt::everything(), - rows = gt::everything()) { + data, + columns = gt::everything(), + rows = gt::everything()) { gt::fmt( data, columns = columns, @@ -146,9 +146,9 @@ n_to_rag <- function(x) { } fmt_present <- function( - data, - columns = gt::everything(), - rows = gt::everything()) { + data, + columns = gt::everything(), + rows = gt::everything()) { gt::fmt( data, columns = columns, diff --git a/R/Widget_FlagOverTime.R b/R/Widget_FlagOverTime.R index d4366567f..28a67df4d 100644 --- a/R/Widget_FlagOverTime.R +++ b/R/Widget_FlagOverTime.R @@ -17,7 +17,7 @@ #' @export Widget_FlagOverTime <- function( dfResults, - dfMetrics = NULL, + dfMetrics, strGroupLevel = c("Site", "Study", "Country") ) { gtFlagOverTime <- Report_FlagOverTime( diff --git a/man/Widget_FlagOverTime.Rd b/man/Widget_FlagOverTime.Rd index 16472fd8a..0472c20c3 100644 --- a/man/Widget_FlagOverTime.Rd +++ b/man/Widget_FlagOverTime.Rd @@ -6,7 +6,7 @@ \usage{ Widget_FlagOverTime( dfResults, - dfMetrics = NULL, + dfMetrics, strGroupLevel = c("Site", "Study", "Country") ) } From 4d8f6b4f034674b8203aa06401079b8b8a55c5f7 Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Wed, 11 Sep 2024 22:52:07 +0000 Subject: [PATCH 06/16] that's probably the best way to write it? --- R/Visualize_Metric.R | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/R/Visualize_Metric.R b/R/Visualize_Metric.R index a40333aa2..5a7a591fb 100644 --- a/R/Visualize_Metric.R +++ b/R/Visualize_Metric.R @@ -83,10 +83,11 @@ Visualize_Metric <- function( # Prep chart inputs --------------------------------------------------------- if(is.null(dfMetrics)){ lMetric <- NULL + vThreshold <- NULL } else { lMetric <- as.list(dfMetrics) + vThreshold <- ParseThreshold(lMetric$Threshold) } - vThreshold <- ifelse(length(lMetric) != 0, ParseThreshold(lMetric$Threshold), c(-2, -1, 2, 3)) # Cross-sectional Charts using most recent snapshot ------------------------ lCharts <- list() From 1e1163dc2149c8e0702d6c8e858c82ff18179ac4 Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Thu, 12 Sep 2024 14:52:34 +0000 Subject: [PATCH 07/16] need to add unit tests --- R/Report_MetricTable.R | 17 ++++++++++------- R/Visualize_Metric.R | 16 ++++++---------- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/R/Report_MetricTable.R b/R/Report_MetricTable.R index acc1be8eb..f39d89d06 100644 --- a/R/Report_MetricTable.R +++ b/R/Report_MetricTable.R @@ -28,17 +28,20 @@ #' @export Report_MetricTable <- function( dfResults, - dfGroups, + dfGroups = NULL, strGroupLevel = c("Site", "Country", "Study"), strGroupDetailsParams = NULL, vFlags = c(-2, -1, 1, 2) ) { + if(!is.null(dfGroups)) { + dfResults <- dfResults %>% + add_Groups_metadata( + dfGroups, + strGroupLevel, + strGroupDetailsParams + ) + } dfResults <- dfResults %>% - add_Groups_metadata( - dfGroups, - strGroupLevel, - strGroupDetailsParams - ) %>% dplyr::filter( .data$Flag %in% vFlags ) @@ -51,7 +54,7 @@ Report_MetricTable <- function( stop("Expecting `dfResults` to be filtered to one unique MetricID, but many detected.") } - if (rlang::arg_match(strGroupLevel) == "Site") { + if (rlang::arg_match(strGroupLevel) == "Site" & !is.null(dfGroups)) { dfResults$Group <- glue::glue("{dfResults$GroupID} ({dfResults$InvestigatorLastName})") } else { dfResults$Group <- dfResults$GroupID diff --git a/R/Visualize_Metric.R b/R/Visualize_Metric.R index 5a7a591fb..ed30abb97 100644 --- a/R/Visualize_Metric.R +++ b/R/Visualize_Metric.R @@ -141,16 +141,12 @@ Visualize_Metric <- function( strType = "Score", vThreshold = vThreshold ) - if(!is.null(dfGroups)) { - lCharts$metricTable <- Report_MetricTable( - dfResults = dfResults_latest, - dfGroups = dfGroups, - strGroupLevel = unique(dfGroups$GroupLevel) - ) - } - else { - cli_inform("Group Metric Table was not rendered") - } + + lCharts$metricTable <- Report_MetricTable( + dfResults = dfResults_latest, + dfGroups = dfGroups, + strGroupLevel = unique(dfGroups$GroupLevel) + ) } # Continuous Charts ------------------------------------------------------- if (number_of_snapshots <= 1) { From 01f4f931eeac5c0392cf9df2f43645a99e98addf Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Thu, 12 Sep 2024 15:56:09 +0000 Subject: [PATCH 08/16] get tests up --- R/Visualize_Metric.R | 14 +++++++++----- tests/testthat/test-Report_MetricTable.R | 7 +++++++ tests/testthat/test-Visualize_Metric.R | 16 ++++++++++++++++ 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/R/Visualize_Metric.R b/R/Visualize_Metric.R index ed30abb97..c9504d3a4 100644 --- a/R/Visualize_Metric.R +++ b/R/Visualize_Metric.R @@ -141,12 +141,16 @@ Visualize_Metric <- function( strType = "Score", vThreshold = vThreshold ) + if(!is.null(lMetric)) { + lCharts$metricTable <- Report_MetricTable( + dfResults = dfResults_latest, + dfGroups = dfGroups, + strGroupLevel = lMetric$GroupLevel + ) + } else { + lCharts$metricTable <- Report_MetricTable(dfResults_latest) + } - lCharts$metricTable <- Report_MetricTable( - dfResults = dfResults_latest, - dfGroups = dfGroups, - strGroupLevel = unique(dfGroups$GroupLevel) - ) } # Continuous Charts ------------------------------------------------------- if (number_of_snapshots <= 1) { diff --git a/tests/testthat/test-Report_MetricTable.R b/tests/testthat/test-Report_MetricTable.R index 50a59d43f..c1f05a5c4 100644 --- a/tests/testthat/test-Report_MetricTable.R +++ b/tests/testthat/test-Report_MetricTable.R @@ -32,3 +32,10 @@ test_that("Score rounding works correctly", { test_that("Errors out when multiple MetricIDs passed in", { expect_error(Report_MetricTable(reportingResults, reportingGroups)) }) + +test_that("Runs with just results with NULL group argument", { + reportingResults_filt <- reportingResults %>% + dplyr::filter(MetricID == unique(reportingResults$MetricID)[1]) + result <- Report_MetricTable(reportingResults_filt) + expect_s3_class(result, "kableExtra") +}) diff --git a/tests/testthat/test-Visualize_Metric.R b/tests/testthat/test-Visualize_Metric.R index c52972223..a07f84192 100644 --- a/tests/testthat/test-Visualize_Metric.R +++ b/tests/testthat/test-Visualize_Metric.R @@ -33,3 +33,19 @@ test_that("Visualize_Metric handles multiple snapshots", { expect_true("timeSeriesContinuousMetricJS" %in% names(charts)) expect_true("timeSeriesContinuousNumeratorJS" %in% names(charts)) }) + +test_that("Visualize_Metric can run on just results", { + charts <- Visualize_Metric(filter(reportingResults, MetricID == "kri0001")) + + # Test if the list contains expected chart names + expect_true("scatterJS" %in% names(charts)) + expect_true("scatter" %in% names(charts)) + expect_true("barMetricJS" %in% names(charts)) + expect_true("barScoreJS" %in% names(charts)) + expect_true("barMetric" %in% names(charts)) + expect_true("barScore" %in% names(charts)) + expect_true("timeSeriesContinuousScoreJS" %in% names(charts)) + expect_true("timeSeriesContinuousMetricJS" %in% names(charts)) + expect_true("timeSeriesContinuousNumeratorJS" %in% names(charts)) + expect_true("metricTable" %in% names(charts)) +}) From f93f655b2d813f0e5d7e565deb0dfba17178d484 Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Thu, 12 Sep 2024 16:03:52 +0000 Subject: [PATCH 09/16] run devtools doc --- man/Report_MetricTable.Rd | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/man/Report_MetricTable.Rd b/man/Report_MetricTable.Rd index 1645fdb76..d7d34fdb1 100644 --- a/man/Report_MetricTable.Rd +++ b/man/Report_MetricTable.Rd @@ -6,7 +6,7 @@ \usage{ Report_MetricTable( dfResults, - dfGroups, + dfGroups = NULL, strGroupLevel = c("Site", "Country", "Study"), strGroupDetailsParams = NULL, vFlags = c(-2, -1, 1, 2) From 740cab1b765b1423a26c2a6e6cacaaaf0c560880 Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Thu, 12 Sep 2024 16:34:17 +0000 Subject: [PATCH 10/16] account for new stopifnot() --- R/Visualize_Metric.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/R/Visualize_Metric.R b/R/Visualize_Metric.R index c9504d3a4..acd9ac592 100644 --- a/R/Visualize_Metric.R +++ b/R/Visualize_Metric.R @@ -82,7 +82,7 @@ Visualize_Metric <- function( } # Prep chart inputs --------------------------------------------------------- if(is.null(dfMetrics)){ - lMetric <- NULL + lMetric <- list() vThreshold <- NULL } else { lMetric <- as.list(dfMetrics) @@ -141,7 +141,7 @@ Visualize_Metric <- function( strType = "Score", vThreshold = vThreshold ) - if(!is.null(lMetric)) { + if(length(lMetric) != 0) { lCharts$metricTable <- Report_MetricTable( dfResults = dfResults_latest, dfGroups = dfGroups, From 4c1c514b194e199b51f0a16423e9cfa0dadcd061 Mon Sep 17 00:00:00 2001 From: Spencer Childress Date: Tue, 17 Sep 2024 09:49:34 -0400 Subject: [PATCH 11/16] explicitly handle `lMetric` data type on the widget side --- R/Report_MetricTable.R | 2 +- R/Widget_BarChart.R | 4 ++-- R/Widget_ScatterPlot.R | 5 +++-- R/Widget_TimeSeries.R | 4 ++-- inst/htmlwidgets/Widget_BarChart.js | 5 +++++ inst/htmlwidgets/Widget_ScatterPlot.js | 5 +++++ inst/htmlwidgets/Widget_TimeSeries.js | 5 +++++ 7 files changed, 23 insertions(+), 7 deletions(-) diff --git a/R/Report_MetricTable.R b/R/Report_MetricTable.R index f39d89d06..f95babea2 100644 --- a/R/Report_MetricTable.R +++ b/R/Report_MetricTable.R @@ -54,7 +54,7 @@ Report_MetricTable <- function( stop("Expecting `dfResults` to be filtered to one unique MetricID, but many detected.") } - if (rlang::arg_match(strGroupLevel) == "Site" & !is.null(dfGroups)) { + if (rlang::arg_match(strGroupLevel) == "Site" && !is.null(dfGroups) && !is.null(dfGroups$InvestigatorLastName)) { dfResults$Group <- glue::glue("{dfResults$GroupID} ({dfResults$InvestigatorLastName})") } else { dfResults$Group <- dfResults$GroupID diff --git a/R/Widget_BarChart.R b/R/Widget_BarChart.R index e30c06325..914f85fa0 100644 --- a/R/Widget_BarChart.R +++ b/R/Widget_BarChart.R @@ -33,7 +33,7 @@ Widget_BarChart <- function( dfResults, - lMetric = list(), # TODO: coerce list to object instead of array with jsonlite::toJSON() + lMetric = NULL, dfGroups = NULL, vThreshold = NULL, strOutcome = "Score", @@ -43,7 +43,7 @@ Widget_BarChart <- function( ) { stopifnot( "dfResults is not a data.frame" = is.data.frame(dfResults), - "lMetric must be a list, but not a data.frame" = is.list(lMetric) && !is.data.frame(lMetric), + "lMetric must be a list, but not a data.frame" = is.null(lMetric) || (is.list(lMetric) && !is.data.frame(lMetric)), "dfGroups is not a data.frame" = is.null(dfGroups) || is.data.frame(dfGroups), "strOutcome must be length 1" = length(strOutcome) == 1, "strOutcome is not a character" = is.character(strOutcome), diff --git a/R/Widget_ScatterPlot.R b/R/Widget_ScatterPlot.R index cc6e05325..22dd465ac 100644 --- a/R/Widget_ScatterPlot.R +++ b/R/Widget_ScatterPlot.R @@ -33,7 +33,7 @@ Widget_ScatterPlot <- function( dfResults, - lMetric = list(), # TODO: coerce list to object instead of array with jsonlite::toJSON() + lMetric = NULL, dfGroups = NULL, dfBounds = NULL, bAddGroupSelect = TRUE, @@ -42,13 +42,14 @@ Widget_ScatterPlot <- function( ) { stopifnot( "dfResults is not a data.frame" = is.data.frame(dfResults), - "lMetric must be a list, but not a data.frame" = is.list(lMetric) && !is.data.frame(lMetric), + "lMetric must be a list, but not a data.frame" = is.null(lMetric) || (is.list(lMetric) && !is.data.frame(lMetric)), "dfGroups is not a data.frame" = is.null(dfGroups) || is.data.frame(dfGroups), "dfBounds is not a data.frame" = is.null(dfBounds) || is.data.frame(dfBounds), "bAddGroupSelect is not a logical" = is.logical(bAddGroupSelect), "strShinyGroupSelectID is not a character" = is.character(strShinyGroupSelectID), "bDebug is not a logical" = is.logical(bDebug) ) + # define widget inputs input <- list( dfResults = dfResults, diff --git a/R/Widget_TimeSeries.R b/R/Widget_TimeSeries.R index ab33b7e0e..76df01854 100644 --- a/R/Widget_TimeSeries.R +++ b/R/Widget_TimeSeries.R @@ -32,7 +32,7 @@ Widget_TimeSeries <- function( dfResults, - lMetric = list(), + lMetric = NULL, dfGroups = NULL, vThreshold = NULL, strOutcome = "Score", @@ -42,7 +42,7 @@ Widget_TimeSeries <- function( ) { stopifnot( "dfResults is not a data.frame" = is.data.frame(dfResults), - "lMetric must be a list, but not a data.frame" = is.list(lMetric) & !is.data.frame(lMetric), + "lMetric must be a list, but not a data.frame" = is.null(lMetric) || (is.list(lMetric) & !is.data.frame(lMetric)), "dfGroups is not a data.frame" = is.null(dfGroups) || is.data.frame(dfGroups), "strOutcome must be length 1" = length(strOutcome) == 1, "strOutcome is not a character" = is.character(strOutcome), diff --git a/inst/htmlwidgets/Widget_BarChart.js b/inst/htmlwidgets/Widget_BarChart.js index 93f284fc4..31fc464c6 100644 --- a/inst/htmlwidgets/Widget_BarChart.js +++ b/inst/htmlwidgets/Widget_BarChart.js @@ -7,6 +7,11 @@ HTMLWidgets.widget({ if (input.bDebug) console.log(input); + // Coerce `input.lMetric` to an object if it is not already. + if (Object.prototype.toString.call(input.lMetric) !== '[object Object]') { + input.lMetric = {}; + }; + // Assign a unique ID to the element. el.id = `barChart--${input.lMetric.MetricID}_${input.strOutcome}`; diff --git a/inst/htmlwidgets/Widget_ScatterPlot.js b/inst/htmlwidgets/Widget_ScatterPlot.js index 1f4945ea4..db4a0fc16 100644 --- a/inst/htmlwidgets/Widget_ScatterPlot.js +++ b/inst/htmlwidgets/Widget_ScatterPlot.js @@ -7,6 +7,11 @@ HTMLWidgets.widget({ if (input.bDebug) console.log(input); + // Coerce `input.lMetric` to an object if it is not already. + if (Object.prototype.toString.call(input.lMetric) !== '[object Object]') { + input.lMetric = {}; + }; + // Assign a unique ID to the element. el.id = `scatterPlot--${input.lMetric.MetricID}`; diff --git a/inst/htmlwidgets/Widget_TimeSeries.js b/inst/htmlwidgets/Widget_TimeSeries.js index 323e1591c..bfac2c2e0 100644 --- a/inst/htmlwidgets/Widget_TimeSeries.js +++ b/inst/htmlwidgets/Widget_TimeSeries.js @@ -7,6 +7,11 @@ HTMLWidgets.widget({ if (input.bDebug) console.log(input); + // Coerce `input.lMetric` to an object if it is not already. + if (Object.prototype.toString.call(input.lMetric) !== '[object Object]') { + input.lMetric = {}; + }; + // Assign a unique ID to the element. el.id = `timeSeries--${input.lMetric.MetricID}_${input.strOutcome}`; From b399e9975c4b6b73774b3122fb39e4c37bbdbcda Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Tue, 17 Sep 2024 15:23:27 +0000 Subject: [PATCH 12/16] probably going to want to check cyclomatic complexity one day --- R/Visualize_Metric.R | 14 ++++++++++++++ tests/testthat/test-Visualize_Metric.R | 16 ++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/R/Visualize_Metric.R b/R/Visualize_Metric.R index acd9ac592..0099256b4 100644 --- a/R/Visualize_Metric.R +++ b/R/Visualize_Metric.R @@ -67,7 +67,20 @@ Visualize_Metric <- function( return(NULL) } else { dfResults <- dfResults %>% filter(.data$MetricID == strMetricID) + } + } + if (!is.null(strMetricID)) { + if (!(strMetricID %in% unique(dfBounds$MetricID))) { + cli::cli_inform("MetricID not found in dfBounds. Please double check input data if intentional.") + } else { dfBounds <- dfBounds %>% filter(.data$MetricID == strMetricID) + } + } + + if (!is.null(strMetricID)) { + if (!(strMetricID %in% unique(dfMetrics$MetricID))) { + cli::cli_inform("MetricID not found in dfMetrics. Please double check input data if intentional.") + } else { dfMetrics <- dfMetrics %>% filter(.data$MetricID == strMetricID) } } @@ -80,6 +93,7 @@ Visualize_Metric <- function( cli::cli_abort("Multiple MetricIDs found in dfResults, dfBounds or dfMetrics. Specify `MetricID` to subset. No charts will be generated.") return(NULL) } + # Prep chart inputs --------------------------------------------------------- if(is.null(dfMetrics)){ lMetric <- list() diff --git a/tests/testthat/test-Visualize_Metric.R b/tests/testthat/test-Visualize_Metric.R index a07f84192..88d95e70e 100644 --- a/tests/testthat/test-Visualize_Metric.R +++ b/tests/testthat/test-Visualize_Metric.R @@ -49,3 +49,19 @@ test_that("Visualize_Metric can run on just results", { expect_true("timeSeriesContinuousNumeratorJS" %in% names(charts)) expect_true("metricTable" %in% names(charts)) }) + +test_that("Visualize_Metric can run on just results and MetricID", { + charts <- Visualize_Metric(reportingResults, strMetricID = "kri0001") + + # Test if the list contains expected chart names + expect_true("scatterJS" %in% names(charts)) + expect_true("scatter" %in% names(charts)) + expect_true("barMetricJS" %in% names(charts)) + expect_true("barScoreJS" %in% names(charts)) + expect_true("barMetric" %in% names(charts)) + expect_true("barScore" %in% names(charts)) + expect_true("timeSeriesContinuousScoreJS" %in% names(charts)) + expect_true("timeSeriesContinuousMetricJS" %in% names(charts)) + expect_true("timeSeriesContinuousNumeratorJS" %in% names(charts)) + expect_true("metricTable" %in% names(charts)) +}) From 64b48b79fd8d520a4e7114d06beeadc1392a6734 Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Tue, 17 Sep 2024 15:31:07 +0000 Subject: [PATCH 13/16] resolve conflict --- R/Report_MetricTable.R | 50 ++++++++++-------------------------------- 1 file changed, 12 insertions(+), 38 deletions(-) diff --git a/R/Report_MetricTable.R b/R/Report_MetricTable.R index f95babea2..03121eff0 100644 --- a/R/Report_MetricTable.R +++ b/R/Report_MetricTable.R @@ -27,12 +27,13 @@ #' #' @export Report_MetricTable <- function( - dfResults, - dfGroups = NULL, - strGroupLevel = c("Site", "Country", "Study"), - strGroupDetailsParams = NULL, - vFlags = c(-2, -1, 1, 2) + dfResults, + dfGroups = NULL, + strGroupLevel = c("Site", "Country", "Study"), + strGroupDetailsParams = NULL, + vFlags = c(-2, -1, 1, 2) ) { + # Check for if dfGroups was provided and process group metadata if available if(!is.null(dfGroups)) { dfResults <- dfResults %>% add_Groups_metadata( @@ -41,48 +42,21 @@ Report_MetricTable <- function( strGroupDetailsParams ) } + dfResults <- dfResults %>% dplyr::filter( .data$Flag %in% vFlags ) + MetricTable <- MakeMetricTable( + dfResults, dfGroups, strGroupLevel, strGroupDetailsParams, vFlags + ) + if (!nrow(dfResults)) { return("Nothing flagged for this KRI.") } - if (length(unique(dfResults$MetricID)) > 1) { - stop("Expecting `dfResults` to be filtered to one unique MetricID, but many detected.") - } - - if (rlang::arg_match(strGroupLevel) == "Site" && !is.null(dfGroups) && !is.null(dfGroups$InvestigatorLastName)) { - dfResults$Group <- glue::glue("{dfResults$GroupID} ({dfResults$InvestigatorLastName})") - } else { - dfResults$Group <- dfResults$GroupID - } - - SummaryTable <- dfResults %>% - dplyr::arrange( - desc(abs(.data$Flag)), - desc(abs(.data$Score)) - ) %>% - dplyr::mutate( - Flag = Report_FormatFlag(.data$Flag), - dplyr::across( - dplyr::where(is.numeric), - ~ round(.x, 2) - ) - ) %>% - dplyr::select( - dplyr::any_of(c( - "Group", - "Enrolled" = "ParticipantCount", - "Numerator", - "Denominator", - "Metric", - "Score", - "Flag" - )) - ) %>% + SummaryTable <- MetricTable %>% kableExtra::kbl(format = "html", escape = FALSE) %>% kableExtra::kable_styling("striped", full_width = FALSE) From d86b0fbf1a3e1e451a7a89ca3e7e22461fb6168e Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Tue, 17 Sep 2024 16:13:42 +0000 Subject: [PATCH 14/16] accomodate for latest updates --- R/Report_MetricTable.R | 15 --------------- R/Visualize_Metric.R | 4 ++-- R/util-MakeMetricTable.R | 19 ++++++++++++------- 3 files changed, 14 insertions(+), 24 deletions(-) diff --git a/R/Report_MetricTable.R b/R/Report_MetricTable.R index 03121eff0..d4784cf0d 100644 --- a/R/Report_MetricTable.R +++ b/R/Report_MetricTable.R @@ -33,21 +33,6 @@ Report_MetricTable <- function( strGroupDetailsParams = NULL, vFlags = c(-2, -1, 1, 2) ) { - # Check for if dfGroups was provided and process group metadata if available - if(!is.null(dfGroups)) { - dfResults <- dfResults %>% - add_Groups_metadata( - dfGroups, - strGroupLevel, - strGroupDetailsParams - ) - } - - dfResults <- dfResults %>% - dplyr::filter( - .data$Flag %in% vFlags - ) - MetricTable <- MakeMetricTable( dfResults, dfGroups, strGroupLevel, strGroupDetailsParams, vFlags ) diff --git a/R/Visualize_Metric.R b/R/Visualize_Metric.R index 0099256b4..1f001a79a 100644 --- a/R/Visualize_Metric.R +++ b/R/Visualize_Metric.R @@ -96,7 +96,7 @@ Visualize_Metric <- function( # Prep chart inputs --------------------------------------------------------- if(is.null(dfMetrics)){ - lMetric <- list() + lMetric <- NULL vThreshold <- NULL } else { lMetric <- as.list(dfMetrics) @@ -155,7 +155,7 @@ Visualize_Metric <- function( strType = "Score", vThreshold = vThreshold ) - if(length(lMetric) != 0) { + if(!is.null(lMetric)) { lCharts$metricTable <- Report_MetricTable( dfResults = dfResults_latest, dfGroups = dfGroups, diff --git a/R/util-MakeMetricTable.R b/R/util-MakeMetricTable.R index 4d095349b..811bdfae3 100644 --- a/R/util-MakeMetricTable.R +++ b/R/util-MakeMetricTable.R @@ -23,17 +23,22 @@ #' @export MakeMetricTable <- function( dfResults, - dfGroups, + dfGroups = NULL, strGroupLevel = c("Site", "Country", "Study"), strGroupDetailsParams = NULL, vFlags = c(-2, -1, 1, 2) ) { + # Check for if dfGroups was provided and process group metadata if available + if(!is.null(dfGroups)) { + dfResults <- dfResults %>% + add_Groups_metadata( + dfGroups, + strGroupLevel, + strGroupDetailsParams + ) + } + dfResults <- dfResults %>% - add_Groups_metadata( - dfGroups, - strGroupLevel, - strGroupDetailsParams - ) %>% dplyr::filter( .data$Flag %in% vFlags ) @@ -61,7 +66,7 @@ MakeMetricTable <- function( stop("Expecting `dfResults` to be filtered to one unique MetricID, but many detected.") } - if (rlang::arg_match(strGroupLevel) == "Site") { + if (rlang::arg_match(strGroupLevel) == "Site" & !is.null(dfGroups)) { dfResults$Group <- glue::glue("{dfResults$GroupID} ({dfResults$InvestigatorLastName})") } else { dfResults$Group <- dfResults$GroupID From 9959854f0100badca93a3ef3d49d8c091ed7647f Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Tue, 17 Sep 2024 16:25:21 +0000 Subject: [PATCH 15/16] needa run devtools::document --- man/MakeMetricTable.Rd | 2 +- man/Widget_BarChart.Rd | 2 +- man/Widget_ScatterPlot.Rd | 2 +- man/Widget_TimeSeries.Rd | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/man/MakeMetricTable.Rd b/man/MakeMetricTable.Rd index 121dd4a67..f3209b2b2 100644 --- a/man/MakeMetricTable.Rd +++ b/man/MakeMetricTable.Rd @@ -6,7 +6,7 @@ \usage{ MakeMetricTable( dfResults, - dfGroups, + dfGroups = NULL, strGroupLevel = c("Site", "Country", "Study"), strGroupDetailsParams = NULL, vFlags = c(-2, -1, 1, 2) diff --git a/man/Widget_BarChart.Rd b/man/Widget_BarChart.Rd index 4233f9907..e4e8fd1aa 100644 --- a/man/Widget_BarChart.Rd +++ b/man/Widget_BarChart.Rd @@ -6,7 +6,7 @@ \usage{ Widget_BarChart( dfResults, - lMetric = list(), + lMetric = NULL, dfGroups = NULL, vThreshold = NULL, strOutcome = "Score", diff --git a/man/Widget_ScatterPlot.Rd b/man/Widget_ScatterPlot.Rd index 17398e039..1417efe1d 100644 --- a/man/Widget_ScatterPlot.Rd +++ b/man/Widget_ScatterPlot.Rd @@ -6,7 +6,7 @@ \usage{ Widget_ScatterPlot( dfResults, - lMetric = list(), + lMetric = NULL, dfGroups = NULL, dfBounds = NULL, bAddGroupSelect = TRUE, diff --git a/man/Widget_TimeSeries.Rd b/man/Widget_TimeSeries.Rd index 214fc842a..dfb77afad 100644 --- a/man/Widget_TimeSeries.Rd +++ b/man/Widget_TimeSeries.Rd @@ -6,7 +6,7 @@ \usage{ Widget_TimeSeries( dfResults, - lMetric = list(), + lMetric = NULL, dfGroups = NULL, vThreshold = NULL, strOutcome = "Score", From 382f0f55f40447f23b325cd31b7c1bee37463468 Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Wed, 18 Sep 2024 13:34:06 +0000 Subject: [PATCH 16/16] did I mess up doing the merge --- R/util-MakeMetricTable.R | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/R/util-MakeMetricTable.R b/R/util-MakeMetricTable.R index 960f083e3..b63844fc5 100644 --- a/R/util-MakeMetricTable.R +++ b/R/util-MakeMetricTable.R @@ -22,19 +22,11 @@ #' #' @export MakeMetricTable <- function( -<<<<<<< fix-1636 - dfResults, - dfGroups = NULL, - strGroupLevel = c("Site", "Country", "Study"), - strGroupDetailsParams = NULL, - vFlags = c(-2, -1, 1, 2) -======= dfResults, - dfGroups, + dfGroups = NULL, strGroupLevel = c("Site", "Country", "Study"), strGroupDetailsParams = NULL, vFlags = c(-2, -1, 1, 2) ->>>>>>> dev ) { # Check for if dfGroups was provided and process group metadata if available if(!is.null(dfGroups)) {