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

remove deprecated cohortId param #245

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 0 additions & 8 deletions R/GetCovariates.R
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,6 @@
#' specify both the database and the schema, so for example
#' 'cdm_instance.dbo'.
#' @param cohortTableIsTemp Is the cohort table a temp table?
#' @param cohortId DEPRECATED:For which cohort ID(s) should covariates be constructed? If set to -1,
#' covariates will be constructed for all cohorts in the specified cohort
#' table.
#' @param cohortIds For which cohort ID(s) should covariates be constructed? If set to c(-1),
#' covariates will be constructed for all cohorts in the specified cohort
#' table.
Expand Down Expand Up @@ -100,7 +97,6 @@ getDbCovariateData <- function(connectionDetails = NULL,
cohortTable = "cohort",
cohortDatabaseSchema = cdmDatabaseSchema,
cohortTableIsTemp = FALSE,
cohortId = -1,
cohortIds = c(-1),
rowIdField = "subject_id",
covariateSettings,
Expand All @@ -115,10 +111,6 @@ getDbCovariateData <- function(connectionDetails = NULL,
if (cdmVersion == "4") {
stop("CDM version 4 is not supported any more")
}
if (!missing(cohortId)) {
warning("cohortId argument has been deprecated, please use cohortIds")
cohortIds <- cohortId
}
errorMessages <- checkmate::makeAssertCollection()
minCharacterizationMean <- utils::type.convert(minCharacterizationMean, as.is = TRUE)
checkmate::assertNumeric(x = minCharacterizationMean, lower = 0, add = errorMessages)
Expand Down
5 changes: 0 additions & 5 deletions R/GetCovariatesFromCohortAttributes.R
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ getDbCohortAttrCovariatesData <- function(connection,
oracleTempSchema = NULL,
cdmDatabaseSchema,
cohortTable = "#cohort_person",
cohortId = -1,
cohortIds = c(-1),
cdmVersion = "5",
rowIdField = "subject_id",
Expand All @@ -85,10 +84,6 @@ getDbCohortAttrCovariatesData <- function(connection,
if (cdmVersion == "4") {
stop("Common Data Model version 4 is not supported")
}
if (!missing(cohortId)) {
warning("cohortId argument has been deprecated, please use cohortIds")
cohortIds <- cohortId
}
start <- Sys.time()
writeLines("Constructing covariates from cohort attributes table")

Expand Down
9 changes: 2 additions & 7 deletions R/GetCovariatesFromOtherCohorts.R
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ getDbCohortBasedCovariatesData <- function(connection,
oracleTempSchema = NULL,
cdmDatabaseSchema,
cohortTable = "#cohort_person",
cohortId = -1,
cohortIds = c(-1),
cdmVersion = "5",
rowIdField = "subject_id",
Expand All @@ -44,18 +43,14 @@ getDbCohortBasedCovariatesData <- function(connection,
checkmate::assertCharacter(oracleTempSchema, len = 1, null.ok = TRUE, add = errorMessages)
checkmate::assertCharacter(cdmDatabaseSchema, len = 1, null.ok = TRUE, add = errorMessages)
checkmate::assertCharacter(cohortTable, len = 1, add = errorMessages)
checkmate::assertIntegerish(cohortId, add = errorMessages)
# checkmate::assertCharacter(cdmVersion, len = 1, add = errorMessages)
checkmate::assertIntegerish(cohortIds, add = errorMessages)
checkmate::assertCharacter(cdmVersion, len = 1, add = errorMessages)
checkmate::assertCharacter(rowIdField, len = 1, add = errorMessages)
checkmate::assertClass(covariateSettings, "covariateSettings", add = errorMessages)
checkmate::assertLogical(aggregated, len = 1, add = errorMessages)
minCharacterizationMean <- utils::type.convert(minCharacterizationMean, as.is = TRUE)
checkmate::assertNumeric(x = minCharacterizationMean, lower = 0, add = errorMessages)
checkmate::reportAssertions(collection = errorMessages)
if (!missing(cohortId)) {
warning("cohortId argument has been deprecated, please use cohortIds")
cohortIds <- cohortId
}

start <- Sys.time()
message("Constructing covariates from other cohorts")
Expand Down
5 changes: 0 additions & 5 deletions R/GetDefaultCovariates.R
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ getDbDefaultCovariateData <- function(connection,
oracleTempSchema = NULL,
cdmDatabaseSchema,
cohortTable = "#cohort_person",
cohortId = -1,
cohortIds = c(-1),
cdmVersion = "5",
rowIdField = "subject_id",
Expand All @@ -79,10 +78,6 @@ getDbDefaultCovariateData <- function(connection,
if (!missing(targetCovariateTable) && !is.null(targetCovariateTable) && aggregated) {
stop("Writing aggregated results to database is currently not supported")
}
if (!missing(cohortId)) {
warning("cohortId argument has been deprecated, please use cohortIds")
cohortIds <- cohortId
}
errorMessages <- checkmate::makeAssertCollection()
minCharacterizationMean <- utils::type.convert(minCharacterizationMean, as.is = TRUE)
checkmate::assertNumeric(x = minCharacterizationMean, lower = 0, add = errorMessages)
Expand Down
10 changes: 2 additions & 8 deletions R/HelperFunctions.R
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ filterByRowId <- function(covariateData, rowIds) {
#' Filter covariates by cohort definition IDs
#'
#' @param covariateData An object of type \code{CovariateData}
#' @param cohortId DEPRECATED The cohort definition IDs to keep.
#' @param cohortIds The cohort definition IDs to keep.
#'
#' @return
Expand All @@ -86,8 +85,7 @@ filterByRowId <- function(covariateData, rowIds) {
#' }
#'
#' @export
filterByCohortDefinitionId <- function(covariateData,
cohortId = 1,
filterByCohortDefinitionId <- function(covariateData,
cohortIds = c(1)) {
if (!isCovariateData(covariateData)) {
stop("Data not of class CovariateData")
Expand All @@ -98,11 +96,7 @@ filterByCohortDefinitionId <- function(covariateData,
if (!isAggregatedCovariateData(covariateData)) {
stop("Can only filter aggregated data by cohortIds")
}
if (!missing(cohortId)) {
warning("cohortId argument has been deprecated, please use cohortIds")
cohortIds <- cohortId
}


if (is.null(covariateData$covariates)) {
covariates <- NULL
} else {
Expand Down
2 changes: 1 addition & 1 deletion R/UnitTestHelperFunctions.R
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@
#' oracleTempSchema = NULL,
#' cdmDatabaseSchema = "main",
#' cohortTable = "cohort",
#' cohortId = 1,
#' cohortIds = 1,
#' cdmVersion = "5",
#' rowIdField = "subject_id",
#' covariateSettings = covSettings,
Expand Down
5 changes: 0 additions & 5 deletions extras/GetHdpsCovariates.R
Original file line number Diff line number Diff line change
Expand Up @@ -36,16 +36,11 @@ getDbHdpsCovariateData <- function(connection,
oracleTempSchema = NULL,
cdmDatabaseSchema,
cohortTable = "cohort_person",
cohortId = -1,
cohortIds = c(-1),
cdmVersion = "5",
rowIdField = "subject_id",
covariateSettings,
aggregated = FALSE) {
if (!missing(cohortId)) {
warning("cohortId argument has been deprecated, please use cohortIds")
cohortIds <- cohortId
}
if (cohortIds != -1)
stop("Haven't implemented restricting to cohort ID yet.")
if (aggregated)
Expand Down
3 changes: 0 additions & 3 deletions man-roxygen/GetCovarParams.R
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,6 @@
#' covariates. If it is a temp table, the name should have a hash prefix,
#' e.g. '#temp_table'. If it is a non-temp table, it should include the
#' database schema, e.g. 'cdm_database.cohort'.
#' @param cohortId DEPRECATED:For which cohort ID should covariates be constructed? If set to -1,
#' covariates will be constructed for all cohorts in the specified cohort
#' table.
#' @param cohortIds For which cohort ID(s) should covariates be constructed? If set to c(-1),
#' covariates will be constructed for all cohorts in the specified cohort
#' table.
Expand Down
2 changes: 1 addition & 1 deletion man/dot-getDbLooCovariateData.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 1 addition & 3 deletions man/filterByCohortDefinitionId.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 0 additions & 5 deletions man/getDbCohortAttrCovariatesData.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 0 additions & 5 deletions man/getDbCohortBasedCovariatesData.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 0 additions & 5 deletions man/getDbCovariateData.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 0 additions & 5 deletions man/getDbDefaultCovariateData.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 0 additions & 11 deletions tests/testthat/test-CovariateData.R
Original file line number Diff line number Diff line change
Expand Up @@ -144,17 +144,6 @@ test_that("Test show method", {
on.exit(rm(cvData))
})

test_that("getDbCovariateData cohortId warning", {
skip_if_not(dbms == "sqlite")
settings <- createDefaultCovariateSettings()
expect_warning(getDbCovariateData(connectionDetails = eunomiaConnectionDetails,
cdmDatabaseSchema = eunomiaCdmDatabaseSchema,
cohortDatabaseSchema = eunomiaOhdsiDatabaseSchema,
cohortId = c(1),
covariateSettings = settings,
aggregated = FALSE), "cohortId argument has been deprecated, please use cohortIds")
})

test_that("getDbCovariateData settings list - check metaData", {
skip_if_not(dbms == "sqlite")
looCovSet <- FeatureExtraction:::.createLooCovariateSettings(useLengthOfObs = TRUE)
Expand Down
20 changes: 0 additions & 20 deletions tests/testthat/test-GetCovariatesFromCohortAttributes.R
Original file line number Diff line number Diff line change
Expand Up @@ -72,23 +72,3 @@ test_that("createCohortAttrCovariateSettings check", {
result <- createCohortAttrCovariateSettings(attrDatabaseSchema = "main")
expect_equal(class(result), "covariateSettings")
})

test_that("getDbCohortAttrCovariatesData cohortId warning", {
skip_if_not(dbms == "sqlite")
covariateSettings <- createCohortAttrCovariateSettings(
attrDatabaseSchema = eunomiaOhdsiDatabaseSchema,
cohortAttrTable = cohortAttributeTable,
attrDefinitionTable = attributeDefinitionTable,
includeAttrIds = c(1),
isBinary = FALSE,
missingMeansZero = TRUE
)
# cohortId argument
expect_warning(getDbCohortAttrCovariatesData(
connection = eunomiaConnection,
cdmDatabaseSchema = eunomiaCdmDatabaseSchema,
cohortTable = cohortTable,
covariateSettings = covariateSettings,
cohortId = 1
), "cohortId argument has been deprecated, please use cohortIds")
})
2 changes: 1 addition & 1 deletion tests/testthat/test-GetDefaultCovariates.R
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ test_that("Test exit conditions", {
expect_error(getDbDefaultCovariateData(
connection = eunomiaConnection,
cdmDatabaseSchema = "main",
cohortId = -1,
cohortIds = -1,
covariateSettings = createDefaultCovariateSettings(),
targetDatabaseSchema = "main",
targetCovariateTable = "cov",
Expand Down
Loading