Make JoinKeys related changes due to refactor (#486)

Related to [teal.data PR #184](insightsengineering/teal.data#184) Make changes to `teal.slice` because of the refactor to the `JoinKeys` class from R6 to S3. This diagram shows the R6 methods along with the replacement S3 methods/functions. <img width="1214" alt="Screenshot 2023-11-15 at 3 56 36 PM" src="https://github.com/insightsengineering/teal.slice/assets/49812166/e0a5843f-f54c-46c6-aaeb-fd0e178536a5"> --------- Signed-off-by: Vedha Viyash <[email protected]> Co-authored-by: André Veríssimo <[email protected]> Co-authored-by: kartikeya kirar <[email protected]>
insightsengineering · Nov 20, 2023 · 60d9847 · 60d9847
1 parent 631e619
commit 60d9847
Show file tree

Hide file tree

Showing 18 changed files with 47 additions and 68 deletions.
diff --git a/DESCRIPTION b/DESCRIPTION
@@ -45,7 +45,7 @@ Imports:
     shinycssloaders (>= 1.0.0),
     shinyjs,
     shinyWidgets (>= 0.6.2),
-    teal.data (>= 0.3.0),
+    teal.data (>= 0.3.0.9010),
     teal.logger (>= 0.1.1),
     teal.widgets (>= 0.4.0),
     utils

diff --git a/NEWS.md b/NEWS.md
@@ -4,6 +4,7 @@
 
 * Specified minimal version of package dependencies.
 * Removed storing and restoring of `teal_slices` objects.
+* Update documentation and code to reflect the changes due to the refactor of `teal.data::JoinKeys` into `teal.data::join_keys`.
 
 # teal.slice 0.4.0
 

diff --git a/R/FilterState-utils.R b/R/FilterState-utils.R
@@ -47,7 +47,7 @@
 #'   }
 #' )
 #' if (interactive()) {
-#'   runApp(app)
+#'   shinyApp(app$ui, app$server)
 #' }
 #' @return `FilterState` object
 init_filter_state <- function(x,

diff --git a/R/FilterStates-utils.R b/R/FilterStates-utils.R
@@ -48,7 +48,7 @@
 #'   }
 #' )
 #' if (interactive()) {
-#'   runApp(app)
+#'   shinyApp(app$ui, app$server)
 #' }
 init_filter_states <- function(data,
                                data_reactive = reactive(NULL),

diff --git a/R/FilteredData-utils.R b/R/FilteredData-utils.R
@@ -8,7 +8,7 @@
 #' - `keys` (optional) primary keys.
 #' - `datalabel` (optional) label describing the `dataset`.
 #' - `parent` (optional) which `dataset` is a parent of this one.
-#' @param join_keys (`JoinKeys`) see [teal.data::join_keys()].
+#' @param join_keys (`join_keys`) see [teal.data::join_keys()].
 #' @param code (`CodeClass`) see [`teal.data::CodeClass`].
 #' @param check (`logical(1)`) whether data has been check against reproducibility.
 #' @examples
@@ -63,7 +63,7 @@ init_filtered_data.TealData <- function(x, # nolint
 init_filtered_data.default <- function(x, join_keys = teal.data::join_keys(), code = NULL, check = FALSE) { # nolint
   checkmate::assert_list(x, any.missing = FALSE, names = "unique")
   checkmate::assert_class(code, "CodeClass", null.ok = TRUE)
-  checkmate::assert_class(join_keys, "JoinKeys")
+  checkmate::assert_class(join_keys, "join_keys")
   checkmate::assert_flag(check)
   FilteredData$new(x, join_keys = join_keys, code = code, check = check)
 }

diff --git a/R/FilteredData.R b/R/FilteredData.R
@@ -67,14 +67,14 @@ FilteredData <- R6::R6Class( # nolint
     #' @param data_objects (`list`)
     #'   should named elements containing `data.frame` or `MultiAssayExperiment`.
     #'   Names of the list will serve as `dataname`.
-    #' @param join_keys (`JoinKeys` or NULL) see [`teal.data::join_keys()`].
+    #' @param join_keys (`join_keys` or NULL) see [`teal.data::join_keys()`].
     #' @param code (`CodeClass` or `NULL`) see [`teal.data::CodeClass`].
     #' @param check (`logical(1)`) whether data has been check against reproducibility.
     #'
     initialize = function(data_objects, join_keys = teal.data::join_keys(), code = NULL, check = FALSE) {
       checkmate::assert_list(data_objects, any.missing = FALSE, min.len = 0, names = "unique")
       # Note the internals of data_objects are checked in set_dataset
-      checkmate::assert_class(join_keys, "JoinKeys")
+      checkmate::assert_class(join_keys, "join_keys")
       checkmate::assert_class(code, "CodeClass", null.ok = TRUE)
       checkmate::assert_flag(check)
 
@@ -84,10 +84,9 @@ FilteredData <- R6::R6Class( # nolint
       }
 
       self$set_join_keys(join_keys)
-
       child_parent <- sapply(
         names(data_objects),
-        function(i) join_keys$get_parent(i),
+        function(i) teal.data::parent(join_keys, i),
         USE.NAMES = TRUE,
         simplify = FALSE
       )
@@ -266,7 +265,7 @@ FilteredData <- R6::R6Class( # nolint
     #' @description
     #' Get join keys between two datasets.
     #'
-    #' @return (`JoinKeys`)
+    #' @return (`join_keys`)
     #'
     get_join_keys = function() {
       return(private$join_keys)
@@ -308,7 +307,7 @@ FilteredData <- R6::R6Class( # nolint
     #'
     #' @details
     #' `set_dataset` creates a `FilteredDataset` object which keeps `dataset` for the filtering purpose.
-    #' If this data has a parent specified in the `JoinKeys` object stored in `private$join_keys`
+    #' If this data has a parent specified in the `join_keys` object stored in `private$join_keys`
     #' then created `FilteredDataset` (child) gets linked with other `FilteredDataset` (parent).
     #' "Child" dataset return filtered data then dependent on the reactive filtered data of the
     #' "parent". See more in documentation of `parent` argument in `FilteredDatasetDefault` constructor.
@@ -333,24 +332,28 @@ FilteredData <- R6::R6Class( # nolint
       # the UI also uses `datanames` in ids, so no whitespaces allowed
       check_simple_name(dataname)
 
-      join_keys <- self$get_join_keys()
-      parent_dataname <- join_keys$get_parent(dataname)
+      parent_dataname <- teal.data::parent(private$join_keys, dataname)
+      keys <- private$join_keys[dataname, dataname]
+      if (is.null(keys)) keys <- character(0)
+
       if (length(parent_dataname) == 0) {
         private$filtered_datasets[[dataname]] <- init_filtered_dataset(
           dataset = data,
           dataname = dataname,
           metadata = metadata,
           label = label,
-          keys = self$get_join_keys()$get(dataname, dataname)
+          keys = keys
         )
       } else {
+        join_keys <- private$join_keys[dataname, parent_dataname]
+        if (is.null(join_keys)) join_keys <- character(0)
         private$filtered_datasets[[dataname]] <- init_filtered_dataset(
           dataset = data,
           dataname = dataname,
-          keys = join_keys$get(dataname, dataname),
+          keys = keys,
           parent_name = parent_dataname,
           parent = reactive(self$get_data(parent_dataname, filtered = TRUE)),
-          join_keys = self$get_join_keys()$get(dataname, parent_dataname),
+          join_keys = join_keys,
           label = label,
           metadata = metadata
         )
@@ -362,12 +365,12 @@ FilteredData <- R6::R6Class( # nolint
     #' @description
     #' Set the `join_keys`.
     #'
-    #' @param join_keys (`JoinKeys`) join_key (converted to a nested list)
+    #' @param join_keys (`join_keys`) join_key (converted to a nested list)
     #'
     #' @return (`self`) invisibly this `FilteredData`
     #'
     set_join_keys = function(join_keys) {
-      checkmate::assert_class(join_keys, "JoinKeys")
+      checkmate::assert_class(join_keys, "join_keys")
       private$join_keys <- join_keys
       invisible(self)
     },
@@ -1077,7 +1080,7 @@ FilteredData <- R6::R6Class( # nolint
     # `reactive` containing teal_slices that can be selected; only active in module-specific mode
     available_teal_slices = NULL,
 
-    # keys used for joining/filtering data a JoinKeys object (see teal.data)
+    # keys used for joining/filtering data a join_keys object (see teal.data)
     join_keys = NULL,
 
     # flag specifying whether the user may add filters

diff --git a/R/FilteredDataset-utils.R b/R/FilteredDataset-utils.R
@@ -32,7 +32,7 @@
 #'   }
 #' )
 #' if (interactive()) {
-#'   runApp(app)
+#'   shinyApp(app$ui, app$server)
 #' }
 #'
 #' # MAEFilteredDataset example
@@ -61,7 +61,7 @@
 #'   }
 #' )
 #' if (interactive()) {
-#'   runApp(app)
+#'   shinyApp(app$ui, app$server)
 #' }
 #' @param dataset (`data.frame` or `MultiAssayExperiment`)\cr
 #' @param dataname (`character`)\cr

diff --git a/R/count_labels.R b/R/count_labels.R
@@ -52,7 +52,7 @@
 #'   }
 #' )
 #' if (interactive()) {
-#'   runApp(app)
+#'   shinyApp(app$ui, app$server)
 #' }
 #' @keywords internal
 countBars <- function(inputId, choices, countsmax, countsnow = NULL) { # nolint

diff --git a/man/FilteredData.Rd b/man/FilteredData.Rd
diff --git a/man/countBars.Rd b/man/countBars.Rd
diff --git a/man/init_filter_state.Rd b/man/init_filter_state.Rd
diff --git a/man/init_filter_states.Rd b/man/init_filter_states.Rd
diff --git a/man/init_filtered_data.Rd b/man/init_filtered_data.Rd
diff --git a/man/init_filtered_dataset.Rd b/man/init_filtered_dataset.Rd
diff --git a/tests/testthat/test-FilteredData.R b/tests/testthat/test-FilteredData.R
@@ -10,7 +10,7 @@ testthat::test_that("constructor accepts call with only dataset specified", {
   testthat::expect_error(FilteredData$new(list(iris = dataset)), "Must inherit")
 })
 
-testthat::test_that("constructor accepts join_keys to be JoinKeys or NULL", {
+testthat::test_that("constructor accepts join_keys to be join_keys or NULL", {
   testthat::expect_no_error(
     FilteredData$new(list(iris = list(dataset = iris)), join_keys = teal.data::join_keys())
   )
@@ -65,29 +65,6 @@ testthat::test_that("FilteredData preserves the check field when check is TRUE",
   testthat::expect_true(filtered_data$get_check())
 })
 
-testthat::test_that("FilteredData forbids cyclic graphs of datasets relationship", {
-  jk <- teal.data::join_keys(
-    teal.data::join_key("child", "parent", c("id" = "id")),
-    teal.data::join_key("grandchild", "child", c("id" = "id")),
-    teal.data::join_key("grandchild", "parent", c("id" = "id"))
-  )
-  jk$set_parents(list(child = "parent"))
-  jk$set_parents(list(grandchild = "child"))
-  jk$set_parents(list(parent = "grandchild"))
-  iris2 <- transform(iris, id = seq_len(nrow(iris)))
-  testthat::expect_error(
-    FilteredData$new(
-      list(
-        grandchild = list(dataset = head(iris2)),
-        child = list(dataset = head(iris2)),
-        parent = list(dataset = head(iris2))
-      ),
-      join_keys = jk
-    ),
-    "Graph is not a directed acyclic graph"
-  )
-})
-
 
 # datanames ----
 testthat::test_that("filtered_data$datanames returns character vector of datasets names", {
@@ -98,7 +75,7 @@ testthat::test_that("filtered_data$datanames returns character vector of dataset
 
 testthat::test_that("datanames are ordered topologically from parent to child", {
   jk <- teal.data::join_keys(teal.data::join_key("parent", "child", c("id" = "id")))
-  jk$set_parents(list(child = "parent"))
+  teal.data::parents(jk) <- list(child = "parent")
   iris2 <- transform(iris, id = seq_len(nrow(iris)))
   filtered_data <- FilteredData$new(
     list(
@@ -157,7 +134,7 @@ testthat::test_that("set_datasets creates FilteredDataset object linked with par
     )
   )
   jk <- teal.data::join_keys(teal.data::join_key("parent", "child", c("id" = "id")))
-  jk$set_parents(list(child = "parent"))
+  teal.data::parents(jk) <- list(child = "parent")
   iris2 <- transform(iris, id = seq_len(nrow(iris)))
   filtered_data <- test_class$new(data_objects = list(), join_keys = jk)
   filtered_data$set_dataset(data = head(iris), dataname = "parent", label = NULL, metadata = NULL)
@@ -170,9 +147,9 @@ testthat::test_that("set_datasets creates FilteredDataset object linked with par
 
 
 # get_keys ----
-testthat::test_that("get_join_keys returns empty JoinKeys object", {
+testthat::test_that("get_join_keys returns empty join_keys object", {
   filtered_data <- FilteredData$new(list(iris = list(dataset = head(iris))))
-  testthat::expect_s3_class(filtered_data$get_join_keys(), "JoinKeys")
+  testthat::expect_s3_class(filtered_data$get_join_keys(), "join_keys")
 })
 
 testthat::test_that("get_keys returns keys of the dataset specified via join_keys", {
@@ -374,8 +351,7 @@ testthat::test_that("get_data of the child is dependent on the ancestor filter",
     teal.data::join_key("child", "parent", c("id" = "id")),
     teal.data::join_key("grandchild", "child", c("id" = "id"))
   )
-  jk$set_parents(list(child = "parent"))
-  jk$set_parents(list(grandchild = "child"))
+  teal.data::parents(jk) <- list(child = "parent", grandchild = "child")
   iris2 <- transform(iris, id = seq_len(nrow(iris)))
   filtered_data <- FilteredData$new(
     list(
@@ -751,8 +727,7 @@ testthat::test_that("get_filter_overview return counts based on reactive filteri
     teal.data::join_key("child", "parent", c("id" = "id")),
     teal.data::join_key("grandchild", "child", c("id" = "id"))
   )
-  jk$set_parents(list(child = "parent"))
-  jk$set_parents(list(grandchild = "child"))
+  teal.data::parents(jk) <- list(child = "parent", grandchild = "child")
   iris2 <- transform(iris, id = seq_len(nrow(iris)))
   filtered_data <- FilteredData$new(
     list(

diff --git a/tests/testthat/test-init_filtered_data.R b/tests/testthat/test-init_filtered_data.R
@@ -35,10 +35,10 @@ testthat::test_that("init_filtered_data.default accepts NULL passed to code", {
   testthat::expect_no_error(init_filtered_data(list("iris" = list(dataset = iris)), code = NULL))
 })
 
-testthat::test_that("init_filtered_data.default asserts join_keys is `JoinKeys`", {
+testthat::test_that("init_filtered_data.default asserts join_keys is `join_keys`", {
   testthat::expect_error(
     init_filtered_data(list("iris" = list(dataset = iris)), join_keys = "test"),
-    regexp = "Assertion on 'join_keys' failed: Must inherit from class 'JoinKeys', but has class 'character'."
+    regexp = "Assertion on 'join_keys' failed: Must inherit from class 'join_keys', but has class 'character'."
   )
 })
 

diff --git a/vignettes/filter-panel-for-developers.Rmd b/vignettes/filter-panel-for-developers.Rmd
@@ -252,6 +252,6 @@ app <- shinyApp(
   }
 )
 if (interactive()) {
-  runApp(app)
+  shinyApp(app$ui, app$server)
 }
 ```
diff --git a/vignettes/teal-slice.Rmd b/vignettes/teal-slice.Rmd
@@ -115,6 +115,6 @@ app <- shinyApp(
   }
 )
 if (interactive()) {
-  runApp(app)
+  shinyApp(app$ui, app$server)
 }
 ```