Release v0.1.0

OHDSI · Oct 5, 2023 · 26f2bf4 · 26f2bf4
2 parents d50b9b7 + 371ed18
commit 26f2bf4
Show file tree

Hide file tree

Showing 106 changed files with 7,766 additions and 1,089 deletions.
diff --git a/.github/workflows/R_CMD_check_Hades.yaml b/.github/workflows/R_CMD_check_Hades.yaml
@@ -20,30 +20,33 @@ jobs:
       fail-fast: false
       matrix:
         config:
-          - {os: windows-latest, r: 'release'}
-          - {os: macOS-latest, r: 'release'}
-          - {os: ubuntu-20.04, r: 'release', rspm: "https://packagemanager.rstudio.com/cran/__linux__/focal/latest"}
+          - {os: windows-latest, r: '4.2.3', rtools: '42', rspm: "https://cloud.r-project.org"}
+          - {os: macOS-latest, r: '4.2.3', rtools: '42', rspm: "https://cloud.r-project.org"}
+          - {os: ubuntu-20.04, r: '4.2.3', rtools: '42', rspm: "https://packagemanager.rstudio.com/cran/__linux__/focal/latest"}
+          - {os: windows-latest, r: 'release', rtools: '', rspm: "https://cloud.r-project.org"}
+          #- {os: macOS-latest, r: 'release', rtools: '', rspm: "https://cloud.r-project.org"}
+          - {os: ubuntu-20.04, r: 'release', rtools: '', rspm: "https://packagemanager.rstudio.com/cran/__linux__/focal/latest"}
 
     env:
       GITHUB_PAT: ${{ secrets.GH_TOKEN }}
       R_REMOTES_NO_ERRORS_FROM_WARNINGS: true
       RSPM: ${{ matrix.config.rspm }}
-      CDM5_ORACLE_CDM_SCHEMA: ${{ secrets.CDM5_ORACLE_CDM_SCHEMA }}
+      CDM5_ORACLE_CDM_SCHEMA: ${{ secrets.CDM5_ORACLE_CDM54_SCHEMA }}
       CDM5_ORACLE_OHDSI_SCHEMA: ${{ secrets.CDM5_ORACLE_OHDSI_SCHEMA }}
       CDM5_ORACLE_PASSWORD: ${{ secrets.CDM5_ORACLE_PASSWORD }}
       CDM5_ORACLE_SERVER: ${{ secrets.CDM5_ORACLE_SERVER }}
       CDM5_ORACLE_USER: ${{ secrets.CDM5_ORACLE_USER }}
-      CDM5_POSTGRESQL_CDM_SCHEMA: ${{ secrets.CDM5_POSTGRESQL_CDM_SCHEMA }}
+      CDM5_POSTGRESQL_CDM_SCHEMA: ${{ secrets.CDM5_POSTGRESQL_CDM54_SCHEMA }}
       CDM5_POSTGRESQL_OHDSI_SCHEMA: ${{ secrets.CDM5_POSTGRESQL_OHDSI_SCHEMA }}
       CDM5_POSTGRESQL_PASSWORD: ${{ secrets.CDM5_POSTGRESQL_PASSWORD }}
       CDM5_POSTGRESQL_SERVER: ${{ secrets.CDM5_POSTGRESQL_SERVER }}
       CDM5_POSTGRESQL_USER: ${{ secrets.CDM5_POSTGRESQL_USER }}
-      CDM5_SQL_SERVER_CDM_SCHEMA: ${{ secrets.CDM5_SQL_SERVER_CDM_SCHEMA }}
+      CDM5_SQL_SERVER_CDM_SCHEMA: ${{ secrets.CDM5_SQL_SERVER_CDM54_SCHEMA }}
       CDM5_SQL_SERVER_OHDSI_SCHEMA: ${{ secrets.CDM5_SQL_SERVER_OHDSI_SCHEMA }}
       CDM5_SQL_SERVER_PASSWORD: ${{ secrets.CDM5_SQL_SERVER_PASSWORD }}
       CDM5_SQL_SERVER_SERVER: ${{ secrets.CDM5_SQL_SERVER_SERVER }}
       CDM5_SQL_SERVER_USER: ${{ secrets.CDM5_SQL_SERVER_USER }}
-      CDM5_REDSHIFT_CDM_SCHEMA: ${{ secrets.CDM5_REDSHIFT_CDM_SCHEMA }}
+      CDM5_REDSHIFT_CDM_SCHEMA: ${{ secrets.CDM5_REDSHIFT_CDM54_SCHEMA }}
       CDM5_REDSHIFT_OHDSI_SCHEMA: ${{ secrets.CDM5_REDSHIFT_OHDSI_SCHEMA }}
       CDM5_REDSHIFT_PASSWORD: ${{ secrets.CDM5_REDSHIFT_PASSWORD }}
       CDM5_REDSHIFT_SERVER: ${{ secrets.CDM5_REDSHIFT_SERVER }}
@@ -53,11 +56,12 @@ jobs:
       CDM5_SPARK_CONNECTION_STRING: ${{ secrets.CDM5_SPARK_CONNECTION_STRING }}
 
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
 
       - uses: r-lib/actions/setup-r@v2
         with:
           r-version: ${{ matrix.config.r }}
+          rtools-version: ${{ matrix.config.rtools }}
 
       - uses: r-lib/actions/setup-tinytex@v2
 
@@ -66,7 +70,11 @@ jobs:
       - name: Install system requirements
         if: runner.os == 'Linux'
         run: |
-          sudo apt-get install -y libssh-dev
+          sudo apt-get install -y make
+          sudo apt-get install -y default-jdk
+          sudo apt-get install -y libcurl4-openssl-dev
+          sudo apt-get install -y libssl-dev
+          sudo apt-get install -y libglpk-dev
           while read -r cmd
           do
             eval sudo $cmd
@@ -84,20 +92,20 @@ jobs:
           check-dir: '"check"'
 
       - name: Upload source package
-        if: success() && runner.os == 'macOS' && github.event_name != 'pull_request' && github.ref == 'refs/heads/main'
+        if: success() && runner.os == 'Windows' && github.event_name != 'pull_request' && github.ref == 'refs/heads/main'
         uses: actions/upload-artifact@v2
         with:
           name: package_tarball
           path: check/*.tar.gz
 
       - name: Install covr
-        if: runner.os == 'macOS'
+        if: runner.os == 'Linux'
         run: |
           install.packages("covr")
         shell: Rscript {0}
 
       - name: Test coverage
-        if: runner.os == 'macOS'
+        if: runner.os == 'Linux'
         run: covr::codecov()
         shell: Rscript {0}
 

diff --git a/.gitignore b/.gitignore
@@ -18,5 +18,4 @@ src/*.so
 src/*.dll
 /Debug
 standalone/build/*
-sql
 _targets
diff --git a/DESCRIPTION b/DESCRIPTION
@@ -1,11 +1,12 @@
 Package: Strategus
 Type: Package
 Title: Coordinating and Executing Analytics Using HADES Modules
-Version: 0.0.6
-Date: 2023-04-25
+Version: 0.1.0
+Date: 2023-10-04
 Authors@R: c(
-	person("Martijn", "Schuemie", email = "[email protected]", role = c("aut", "cre")),
-	person("Anthony", "Sena", email = "[email protected]", role = c("aut")),
+	person("Martijn", "Schuemie", email = "[email protected]", role = c("aut")),
+	person("Anthony", "Sena", email = "[email protected]", role = c("aut", "cre")),
+	person("Jamie", "Gilbert", role = c("aut")),
 	person("Observational Health Data Science and Informatics", role = c("cph"))
   )
 Maintainer: Anthony Sena <[email protected]>
@@ -14,12 +15,12 @@ License: Apache License 2.0
 URL: https://ohdsi.github.io/Strategus, https://github.com/OHDSI/Strategus
 BugReports: https://github.com/OHDSI/Strategus/issues
 Depends:
-	R (>= 4.0.0),
-	CohortGenerator (>= 0.7.0),
-	DatabaseConnector (>= 5.1.0)
+	R (>= 4.2.0),
+	CohortGenerator (>= 0.8.0),
+	DatabaseConnector (>= 6.2.3)
 Imports:
   targets,
-  renv (>= 0.15.5),
+  renv (>= 1.0.0),
   ParallelLogger (>= 3.1.0),
   dplyr,
   checkmate,
@@ -30,6 +31,7 @@ Imports:
   digest,
   methods,
   tibble,
+  ResultModelManager (>= 0.3.0),
   SqlRender (>= 1.11.0)
 Suggests:
   testthat (>= 3.0.0),
@@ -40,6 +42,7 @@ Suggests:
   withr
 Remotes:
   ohdsi/CohortGenerator,
+  ohdsi/ResultModelManager,
   ohdsi/Eunomia
 VignetteBuilder: knitr
 NeedsCompilation: no

diff --git a/NAMESPACE b/NAMESPACE
@@ -4,6 +4,7 @@ export(addModuleSpecifications)
 export(addSharedResources)
 export(createCdmExecutionSettings)
 export(createEmptyAnalysisSpecificiations)
+export(createResultDataModels)
 export(createResultsExecutionSettings)
 export(ensureAllModulesInstantiated)
 export(execute)

diff --git a/NEWS.md b/NEWS.md
@@ -1,3 +1,19 @@
+Strategus 0.1.0
+===============
+
+- Adds an initial implementation for uploading results to a results database (#72)
+- Robust handling of connection details via keyring (#74)
+- Ensures uniqueness of all CDM tables when gathering database metadata (#82)
+- `tempEmulationSchema` added to execution settings and passed properly to modules (#82)
+- Adding logging to module initalization to detect `renv` restore errors (#82)
+- Adopt HADES-wide lock file in latest versions of all modules (#83)
+- Use renv >= v1.0.0 for all modules and Strategus (#83)
+- Add GitHub unit tests for HADES adopted version (currently v4.2.3) and the latest R versions for all modules and Strategus. (#83)
+- Ensure all Strategus GitHub unit tests run on all operating systems and available OHDSI test database platforms (#83)
+- Use CDM v5.4 schemas for all unit tests (#85)
+- Allow for passing `renv`configuration options when running Strategus (#88)
+- Adds SQL for test cohorts to package (#1)
+
 Strategus 0.0.6
 ===============
 

diff --git a/R/DatabaseMetaData.R b/R/DatabaseMetaData.R
@@ -42,7 +42,11 @@ createDatabaseMetaData <- function(executionSettings, keyringName = NULL) {
     connection = connection,
     databaseSchema = executionSettings$cdmDatabaseSchema
   )
-  cdmTableList <- tolower(cdmTableList)
+  cdmTableList <- unique(tolower(cdmTableList))
+
+  if (length(cdmTableList) == 0) {
+    stop(sprintf("FATAL ERROR: No tables found in your OMOP CDM. Please confirm you are using the proper connection information, in particular the CDM schema name."))
+  }
 
   if (!length(cdmTableList[which(x = cdmTableList %in% requiredTables)]) == length(requiredTables)) {
     missingCdmTables <- requiredTables[!(requiredTables %in% cdmTableList)]

diff --git a/R/Execution.R b/R/Execution.R
@@ -24,18 +24,13 @@
 #' Execute analysis specifications.
 #'
 #' @template AnalysisSpecifications
-#' @param executionSettings       An object of type `ExecutionSettings` as created
-#'                                by [createCdmExecutionSettings()] or [createResultsExecutionSettings()].
+#' @template executionSettings
 #' @param executionScriptFolder   Optional: the path to use for storing the execution script.
 #'                                when NULL, this function will use a temporary
 #'                                file location to create the script to execute.
-#'
 #' @template keyringName
-#'
 #' @param restart                 Restart run? Requires `executionScriptFolder` to be specified, and be
 #'                                the same as the `executionScriptFolder` used in the run to restart.
-#'
-#'
 #' @return
 #' Does not return anything. Is called for the side-effect of executing the specified
 #' analyses.
@@ -53,6 +48,18 @@ execute <- function(analysisSpecifications,
   checkmate::assertChoice(x = keyringName, choices = keyringList$keyring, null.ok = TRUE, add = errorMessages)
   checkmate::reportAssertions(collection = errorMessages)
 
+  # Assert that the temp emulation schema is set if required for the dbms
+  # specified by the executionSettings
+  if (is(executionSettings, "CdmExecutionSettings")) {
+    connectionDetails <- retrieveConnectionDetails(
+      connectionDetailsReference = executionSettings$connectionDetailsReference,
+      keyringName = keyringName
+    )
+    DatabaseConnector::assertTempEmulationSchemaSet(
+      dbms = connectionDetails$dbms,
+      tempEmulationSchema = executionSettings$tempEmulationSchema
+    )
+  }
   modules <- ensureAllModulesInstantiated(analysisSpecifications)
 
   if (is.null(executionScriptFolder)) {
@@ -74,6 +81,7 @@ execute <- function(analysisSpecifications,
   }
   dependencies <- extractDependencies(modules)
 
+
   fileName <- generateTargetsScript(
     analysisSpecifications = analysisSpecifications,
     executionSettings = executionSettings,
@@ -82,7 +90,6 @@ execute <- function(analysisSpecifications,
     restart = restart,
     keyringName = keyringName
   )
-
   # targets::tar_manifest(script = fileName)
   # targets::tar_glimpse(script = fileName)
   targets::tar_make(script = fileName, store = file.path(executionScriptFolder, "_targets"))
@@ -93,6 +100,59 @@ generateTargetsScript <- function(analysisSpecifications, executionSettings, dep
   if (restart) {
     return(fileName)
   }
+
+  ### Note anything inisde this block will be scoped inside the targets script file
+  targets::tar_script(
+    {
+      ##
+      # Generated by Strategus - not advisable to edit by hand
+      ##
+      analysisSpecificationsLoad <- readRDS(analysisSpecificationsFileName)
+      moduleToTargetNames <- readRDS(moduleToTargetNamesFileName)
+      dependencies <- readRDS(dependenciesFileName)
+
+      targets::tar_option_set(packages = c("Strategus", "keyring"), imports = c("Strategus", "keyring"))
+      targetList <- list(
+        targets::tar_target(analysisSpecifications, readRDS(analysisSpecificationsFileName)),
+        # NOTE Execution settings could be mapped to many different cdms making re-execution across cdms much simpler
+        targets::tar_target(executionSettings, readRDS(executionSettingsFileName)),
+        targets::tar_target(keyringSettings, readRDS(keyringSettingsFileName))
+      )
+
+      # factory for producing module targets based on their dependencies
+      # This could be inside Strategus as an exported function
+      # it would also be much cleaner to use a targets pattern = cross(analysisSpecifications$moduleSpecifications)
+      # however, working out how to handle dependencies wasn't obvious
+      # This approach could be modified to allow multiple executionSettings, but that would require a substantial re-write
+      for (i in 1:length(analysisSpecificationsLoad$moduleSpecifications)) {
+        moduleSpecification <- analysisSpecificationsLoad$moduleSpecifications[[i]]
+        targetName <- sprintf("%s_%d", moduleSpecification$module, i)
+        dependencyModules <- dependencies[dependencies$module == moduleSpecification$module, ]$dependsOn
+        dependencyTargetNames <- moduleToTargetNames[moduleToTargetNames$module %in% dependencyModules, ]$targetName
+
+        # Use of tar_target_raw allows dynamic names
+        targetList[[length(targetList) + 1]] <- targets::tar_target_raw(targetName,
+          substitute(Strategus:::runModule(analysisSpecifications, keyringSettings, i, executionSettings),
+            env = list(i = i)
+          ),
+          deps = c("analysisSpecifications", "keyringSettings", "executionSettings", dependencyTargetNames)
+        )
+
+        if (execResultsUpload) {
+          resultsTargetName <- paste0(targetName, "_results_upload")
+          targetList[[length(targetList) + 1]] <- targets::tar_target_raw(resultsTargetName,
+            substitute(Strategus:::runResultsUpload(analysisSpecifications, keyringSettings, i, executionSettings),
+              env = list(i = i)
+            ),
+            deps = c("analysisSpecifications", "keyringSettings", "executionSettings", targetName)
+          )
+        }
+      }
+      targetList
+    },
+    script = fileName
+  )
+
   # Store settings objects in the temp folder so they are available in targets
   analysisSpecificationsFileName <- gsub("\\\\", "/", file.path(executionScriptFolder, "analysisSpecifications.rds"))
   saveRDS(analysisSpecifications, analysisSpecificationsFileName)
@@ -101,25 +161,6 @@ generateTargetsScript <- function(analysisSpecifications, executionSettings, dep
   keyringSettingsFileName <- gsub("\\\\", "/", file.path(executionScriptFolder, "keyringSettings.rds"))
   saveRDS(list(keyringName = keyringName), keyringSettingsFileName)
 
-
-  # Dynamically generate targets script based on analysis specifications
-  lines <- c(
-    "library(targets)",
-    "tar_option_set(packages = c('Strategus', 'keyring'))",
-    "list(",
-    "  tar_target(",
-    "    analysisSpecifications,",
-    sprintf("    readRDS('%s')", analysisSpecificationsFileName),
-    "  ),",
-    "  tar_target(",
-    "    executionSettings,",
-    sprintf("    readRDS('%s')", executionSettingsFileName),
-    "  ),",
-    "  tar_target(",
-    "    keyringSettings,",
-    sprintf("    readRDS('%s')", keyringSettingsFileName),
-    "  ),"
-  )
   # Generate target names by module type
   moduleToTargetNames <- list()
   for (i in 1:length(analysisSpecifications$moduleSpecifications)) {
@@ -131,38 +172,28 @@ generateTargetsScript <- function(analysisSpecifications, executionSettings, dep
     )
   }
   moduleToTargetNames <- bind_rows(moduleToTargetNames)
+  moduleToTargetNamesFileName <- gsub("\\\\", "/", file.path(executionScriptFolder, "moduleTargetNames.rds"))
+  saveRDS(moduleToTargetNames, moduleToTargetNamesFileName)
+
+  dependenciesFileName <- gsub("\\\\", "/", file.path(executionScriptFolder, "dependencies.rds"))
+  saveRDS(dependencies, dependenciesFileName)
+
+  execResultsUpload <- all(c(
+    is(executionSettings, "CdmExecutionSettings"),
+    !is.null(executionSettings$resultsConnectionDetailsReference),
+    !is.null(executionSettings$resultsDatabaseSchema)
+  ))
+
+  # Settings required inside script. There is probably a much cleaner way of doing this
+  writeLines(c(
+    sprintf("analysisSpecificationsFileName <- '%s'", analysisSpecificationsFileName),
+    sprintf("executionSettingsFileName <- '%s'", executionSettingsFileName),
+    sprintf("keyringSettingsFileName <- '%s'", keyringSettingsFileName),
+    sprintf("moduleToTargetNamesFileName <- '%s'", moduleToTargetNamesFileName),
+    sprintf("dependenciesFileName <- '%s'", dependenciesFileName),
+    sprintf("execResultsUpload <- '%s'", execResultsUpload),
+    readLines(fileName)
+  ), fileName)
 
-  # Generate targets code, inserting dependencies
-  for (i in 1:length(analysisSpecifications$moduleSpecifications)) {
-    moduleSpecification <- analysisSpecifications$moduleSpecifications[[i]]
-    targetName <- sprintf("%s_%d", moduleSpecification$module, i)
-    dependencyModules <- dependencies %>%
-      filter(.data$module == moduleSpecification$module) %>%
-      pull(.data$dependsOn)
-    dependencyTargetNames <- moduleToTargetNames %>%
-      filter(.data$module %in% dependencyModules) %>%
-      pull(.data$targetName)
-
-    command <- sprintf(
-      "Strategus:::runModule(analysisSpecifications, keyringSettings, %d, executionSettings%s)",
-      i,
-      ifelse(length(dependencyTargetNames) == 0, "", sprintf(", %s", paste(dependencyTargetNames, collapse = ", ")))
-    )
-
-
-    lines <- c(
-      lines,
-      "  tar_target(",
-      sprintf("    %s,", targetName),
-      sprintf("    %s", command),
-      ifelse(i == length(analysisSpecifications$moduleSpecifications), "  )", "  ),")
-    )
-  }
-
-  lines <- c(lines, ")")
-
-  sink(fileName)
-  cat(paste(lines, collapse = "\n"))
-  sink()
   return(fileName)
 }