docs: hard deprecate get_join_keys

insightsengineering · Oct 31, 2023 · 9efe1d2 · 9efe1d2
1 parent ca91310
commit 9efe1d2
Show file tree

Hide file tree

Showing 14 changed files with 74 additions and 104 deletions.
diff --git a/NAMESPACE b/NAMESPACE
@@ -4,9 +4,9 @@ S3method("[",JoinKeys)
 S3method("[",Placeholder)
 S3method("[<-",JoinKeys)
 S3method("[<-",Placeholder)
-S3method("get_join_keys<-",JoinKeys)
-S3method("get_join_keys<-",teal_data)
+S3method("join_keys<-",JoinKeys)
 S3method("join_keys<-",Placeholder)
+S3method("join_keys<-",teal_data)
 S3method("parents<-",Placeholder)
 S3method(as_cdisc,TealDataset)
 S3method(as_cdisc,TealDatasetConnector)
@@ -30,10 +30,6 @@ S3method(get_datasets,TealDataAbstract)
 S3method(get_datasets,TealDataset)
 S3method(get_datasets,TealDatasetConnector)
 S3method(get_datasets,teal_data)
-S3method(get_join_keys,JoinKeys)
-S3method(get_join_keys,TealData)
-S3method(get_join_keys,default)
-S3method(get_join_keys,teal_data)
 S3method(get_key_duplicates,TealDataset)
 S3method(get_key_duplicates,data.frame)
 S3method(get_keys,TealDataAbstract)

diff --git a/R/JoinKeys.R b/R/JoinKeys.R
@@ -295,10 +295,10 @@ mutate_join_keys.JoinKeys <- function(x, dataset_1, dataset_2, val) {
 #'   cdisc_dataset("ADSL", ADSL),
 #'   cdisc_dataset("ADRS", ADRS)
 #' )
-#' x$get_join_keys()$get("ADSL", "ADRS")
+#' join_keys(x)$get("ADSL", "ADRS")
 #'
 #' mutate_join_keys(x, "ADSL", "ADRS", c("COLUMN1" = "COLUMN2"))
-#' x$get_join_keys()$get("ADSL", "ADRS")
+#' join_keys(x)$get("ADSL", "ADRS")
 mutate_join_keys.TealData <- function(x, dataset_1, dataset_2, val) { # nolint
   x$mutate_join_keys(dataset_1, dataset_2, val)
 }

diff --git a/R/get_join_keys.R b/R/get_join_keys.R
@@ -3,57 +3,21 @@
 #' @return Either `JoinKeys` object or `NULL` if no join keys
 #' @export
 get_join_keys <- function(data) {
-  UseMethod("get_join_keys", data)
-}
-
-#' @rdname get_join_keys
-#' @export
-get_join_keys.default <- function(data) {
-  stop("get_join_keys function not implemented for object of class ", toString(class(data)))
-}
-
-#' @rdname get_join_keys
-#' @export
-get_join_keys.teal_data <- function(data) {
-  data@join_keys
-}
-
-#' @rdname get_join_keys
-#' @export
-get_join_keys.JoinKeys <- function(data) {
-  data
-}
-
-#' @rdname get_join_keys
-#' @export
-get_join_keys.TealData <- function(data) {
-  data$get_join_keys()
+  lifecycle::deprecate_stop(
+    when = " 0.3.1",
+    what = "get_join_keys(data)",
+    details = "Use `join_keys(data)` instead."
+  )
 }
 
 #' @rdname get_join_keys
 #' @inheritParams mutate_join_keys
 #' @param value value to assign
 #' @export
 `get_join_keys<-` <- function(data, dataset_1, dataset_2 = NULL, value) {
-  UseMethod("get_join_keys<-", data)
-}
-
-
-#' @rdname get_join_keys
-#' @inheritParams mutate_join_keys
-#' @export
-`get_join_keys<-.JoinKeys` <- function(data, dataset_1, dataset_2 = NULL, value) {
-  # The reason this passthrough method is defined is to prevent a warning message
-  # The assignment is performed by `get_join_keys.JoinKeys` and `[<-.JoinKeys` combination
-  #  as well as `JoinKeys` being an R6 class
-  data
-}
-
-#' @rdname get_join_keys
-#' @export
-`get_join_keys<-.teal_data` <- function(data, dataset_1, dataset_2 = NULL, value) {
-  # The reason this passthrough method is defined is to prevent a warning message
-  # The assignment is performed by `get_join_keys.teal_data` and `[<-.JoinKeys` combination
-  #  as well as `JoinKeys` being an R6 class
-  data
+  lifecycle::deprecate_stop(
+    when = " 0.3.1",
+    what = "get_join_keys(data) <- ...",
+    details = "Use `join_keys(data) <- ...`"
+  )
 }
diff --git a/R/join_keys.R b/R/join_keys.R
@@ -47,6 +47,28 @@
   join_keys_obj
 }
 
+
+#' @rdname get_join_keys
+#' @inheritParams mutate_join_keys
+#' @export
+`join_keys<-.JoinKeys` <- function(data, dataset_1, dataset_2 = NULL, value) {
+  if (missing(dataset_1) || missing(value)) {
+    return(data)
+  }
+  data$mutate(dataset_1, dataset_2, value)
+  data
+}
+
+#' @rdname get_join_keys
+#' @export
+`join_keys<-.teal_data` <- function(data, dataset_1, dataset_2 = NULL, value) {
+  if (missing(dataset_1) || missing(value)) {
+    return(data)
+  }
+  data@join_keys <- mutate_join_keys(data@join_keys, dataset_1, dataset_2, value)
+  data
+}
+
 #' @details
 #' Getter for JoinKeys that returns the relationship between pairs of datasets.
 #'

diff --git a/R/teal_data-class.R b/R/teal_data-class.R
@@ -26,7 +26,7 @@ setOldClass("JoinKeys")
 #'  Access with [get_warnings()].
 #' @slot messages (`character`) messages raised when evaluating code.
 #' @slot join_keys (`JoinKeys`) object specifying joining keys for data sets in `@env`.
-#'  Access or modify with [get_join_keys()].
+#'  Access or modify with [join_keys()].
 #' @slot datanames (`character`) vector of names of data sets in `@env`.
 #'  Used internally to distinguish them from auxiliary variables.
 #'  Access or modify with [datanames()].

diff --git a/man/get_join_keys.Rd b/man/get_join_keys.Rd
diff --git a/man/mutate_join_keys.Rd b/man/mutate_join_keys.Rd
diff --git a/man/teal_data-class.Rd b/man/teal_data-class.Rd
diff --git a/tests/testthat/helper-get_join_keys.R b/tests/testthat/helper-get_join_keys.R
@@ -20,9 +20,9 @@ helper_generator_JoinKeys <- function(dataset_1 = "ds1", keys = c("id")) { # nol
   )
 }
 
-#' Test suite for default get_join generated by helper
-helper_test_get_join_keys <- function(obj, dataset_1 = "ds1") {
-  jk <- get_join_keys(obj)
+#' Test suite for default join_keys generated by helper
+helper_test_getter_join_keys <- function(obj, dataset_1 = "ds1") {
+  jk <- join_keys(obj)
 
   expect_s3_class(jk, c("JoinKey", "R6"))
   expect_length(jk$get(), 1)
@@ -32,11 +32,11 @@ helper_test_get_join_keys <- function(obj, dataset_1 = "ds1") {
 }
 
 #' Test suite for JoinKeys after manual adding a primary key
-helper_test_get_join_keys_add <- function(obj, dataset_1 = "ds1", new_dataset_1 = "ds2", new_keys = c("id")) {
-  obj <- helper_test_get_join_keys(obj, dataset_1)
-  get_join_keys(obj)[new_dataset_1] <- c(new_keys)
+helper_test_getter_join_keys_add <- function(obj, dataset_1 = "ds1", new_dataset_1 = "ds2", new_keys = c("id")) {
+  obj <- helper_test_getter_join_keys(obj, dataset_1)
+  join_keys(obj)[new_dataset_1] <- c(new_keys)
 
-  jk <- get_join_keys(obj)
+  jk <- join_keys(obj)
 
   checkmate::expect_r6(jk, c("JoinKeys"))
   expect_length(jk$get(), 2)

diff --git a/tests/testthat/test-JoinKeys.R b/tests/testthat/test-JoinKeys.R
@@ -655,7 +655,7 @@ testthat::test_that("JoinKeys$check_parent_child throws error if no join_keys ex
 
 test_that("cdisc_join_keys will generate JoinKeys for named list with non-named elements", {
   new_dataset <- cdisc_join_keys("ADSL", ADTTE = rADTTE)
-  jk <- get_join_keys(new_dataset)
+  jk <- join_keys(new_dataset)
 
   expect_identical(unname(jk$get("ADSL", "ADSL")), default_cdisc_keys[["ADSL"]]$primary)
   expect_identical(unname(jk$get("ADTTE", "ADTTE")), default_cdisc_keys[["ADTTE"]]$primary)
@@ -666,7 +666,7 @@ test_that("cdisc_join_keys will generate JoinKeys for named list with non-named
 
 test_that("cdisc_join_keys will generate JoinKeys for character list", {
   new_dataset <- cdisc_join_keys("ADSL", "ADTTE")
-  jk <- get_join_keys(new_dataset)
+  jk <- join_keys(new_dataset)
 
   expect_identical(unname(jk$get("ADSL", "ADSL")), default_cdisc_keys[["ADSL"]]$primary)
   expect_identical(unname(jk$get("ADTTE", "ADTTE")), default_cdisc_keys[["ADTTE"]]$primary)
@@ -677,7 +677,7 @@ test_that("cdisc_join_keys will generate JoinKeys for character list", {
 
 test_that("cdisc_join_keys will generate JoinKeys for named list", {
   new_dataset <- cdisc_join_keys(ADSL = rADSL, ADTTE = rADTTE)
-  jk <- get_join_keys(new_dataset)
+  jk <- join_keys(new_dataset)
 
   expect_identical(unname(jk$get("ADSL", "ADSL")), default_cdisc_keys[["ADSL"]]$primary)
   expect_identical(unname(jk$get("ADTTE", "ADTTE")), default_cdisc_keys[["ADTTE"]]$primary)
@@ -738,7 +738,7 @@ test_that("cdisc_join_keys does nothing with TealDataset", {
     function() as.data.frame(as.list(setNames(nm = get_cdisc_keys("ADAE"))))
   )
   adae_cdc <- cdisc_dataset_connector("ADAE", adae_cf, keys = get_cdisc_keys("ADAE"))
-  expect_length(get_join_keys(cdisc_join_keys(adae_cdc))$get(), 0)
+  expect_length(join_keys(cdisc_join_keys(adae_cdc))$get(), 0)
 })
 
 test_that("[.JoinKeys returns the primary key if arguments only have 1 dataset", {

diff --git a/tests/testthat/test-cdisc_data.R b/tests/testthat/test-cdisc_data.R
@@ -57,7 +57,7 @@ testthat::test_that("cdisc_data sets the join_keys internally", {
     join_key("ADTTE", "ADAE", c("STUDYID", "USUBJID"))
   )
   jks$set_parents(list(ADSL = character(0), ADTTE = "ADSL", ADAE = "ADSL"))
-  testthat::expect_equal(data$get_join_keys(), jks)
+  testthat::expect_equal(join_keys(data), jks)
 })
 
 testthat::test_that(
@@ -76,7 +76,7 @@ testthat::test_that(
     )
     jks$set_parents(list(ADSL = character(0), ADTTE = "ADSL", ADAE = "ADSL"))
     testthat::expect_equal(
-      data$get_join_keys(),
+      join_keys(data),
       jks
     )
   }
@@ -96,7 +96,7 @@ testthat::test_that("cdisc_data sets primary keys as join_keys when no join_keys
     join_key("df2", "df2", "df2_id")
   )
   jks$set_parents(list(df1 = character(0), df2 = character(0)))
-  testthat::expect_equal(data$get_join_keys(), jks)
+  testthat::expect_equal(join_keys(data), jks)
 })
 
 testthat::test_that("cdisc_data throws error when a parent/child graph is not correct", {
@@ -182,7 +182,7 @@ testthat::test_that("List values", {
 
   test_relational_data_equal <- function(data1, data2) {
     testthat::expect_equal(data1$get_items(), data2$get_items())
-    testthat::expect_equal(data1$get_join_keys(), data2$get_join_keys())
+    testthat::expect_equal(join_keys(data1), join_keys(data2))
     mapply(testthat::expect_equal, data1$get_ui("test"), data2$get_ui("test"))
   }
 

diff --git a/tests/testthat/test-get_join_keys.R b/tests/testthat/test-get_join_keys.R
@@ -1,23 +1,23 @@
-test_that("get_join_keys.teal_data will successfully obtain object from teal_data", {
+test_that("join_keys.teal_data will successfully obtain object from teal_data", {
   obj <- helper_generator_teal_data()
 
-  expect_identical(obj@join_keys, get_join_keys(obj))
-  helper_test_get_join_keys(obj, "ds1")
+  expect_identical(obj@join_keys, join_keys(obj))
+  helper_test_getter_join_keys(obj, "ds1")
 })
 
-test_that("get_join_keys.JoinKeys will return itself", {
+test_that("join_keys.JoinKeys will return itself", {
   obj <- helper_generator_JoinKeys()
 
-  expect_identical(obj, get_join_keys(obj))
-  helper_test_get_join_keys(obj, "ds1")
+  expect_identical(obj, join_keys(obj))
+  helper_test_getter_join_keys(obj, "ds1")
 })
 
-test_that("get_join_keys<-.teal_data", {
+test_that("join_keys<-.teal_data", {
   obj <- helper_generator_teal_data()
-  helper_test_get_join_keys_add(obj, "ds1", "ds2")
+  helper_test_getter_join_keys_add(obj, "ds1", "ds2")
 })
 
-test_that("get_join_keys<-.JoinKeys", {
+test_that("join_keys<-.JoinKeys", {
   obj <- helper_generator_JoinKeys()
-  helper_test_get_join_keys_add(obj, "ds1", "ds2")
+  helper_test_getter_join_keys_add(obj, "ds1", "ds2")
 })
diff --git a/tests/testthat/test-teal_data.R b/tests/testthat/test-teal_data.R
@@ -146,7 +146,7 @@ testthat::test_that("teal_data sets passed join_keys to datasets correctly", {
   )
   jk_expected$set_parents(list(df1 = character(0), df2 = character(0)))
 
-  testthat::expect_equal(data$get_join_keys(), jk_expected)
+  testthat::expect_equal(join_keys(data), jk_expected)
 })
 
 testthat::test_that("teal_data sets passed JoinKeys to datasets correctly when key names differ", {
@@ -165,7 +165,7 @@ testthat::test_that("teal_data sets passed JoinKeys to datasets correctly when k
   )
   jk_expected$set_parents(list(df1 = character(0), df2 = character(0)))
 
-  testthat::expect_equal(data$get_join_keys(), jk_expected)
+  testthat::expect_equal(join_keys(data), jk_expected)
 })
 
 testthat::test_that("teal_data sets passes JoinKeys to datasets correctly when key names differ (multiple keys)", {
@@ -182,7 +182,7 @@ testthat::test_that("teal_data sets passes JoinKeys to datasets correctly when k
     join_key("df1", "df2", c(id = "fk", id2 = "fk2"))
   )
   jk_expected$set_parents(list(df1 = character(0), df2 = character(0)))
-  testthat::expect_equal(data$get_join_keys(), jk_expected)
+  testthat::expect_equal(join_keys(data), jk_expected)
 })
 
 testthat::test_that("teal_data returns TealData object with cdisc_dataset input", {

diff --git a/vignettes/join-keys.Rmd b/vignettes/join-keys.Rmd
@@ -57,7 +57,7 @@ data <- teal_data(
     # join_key("D2", "D2", keys = c("V", "W")), # equivalent to using primary_key
   )
 )
-data$get_join_keys()
+join_keys(data)
 ```
 
 ### Merge keys