Prepare for switch to Andromeda-arrow #179
Conversation
|
|
||
| # local work folder | ||
| work/ |
There was a problem hiding this comment.
This is just for my convenience. I like to have an ignored folder where I can store work in progress.
There was a problem hiding this comment.
You can delete this if you like.
R/CovariateData.R
Outdated
| @@ -129,13 +129,13 @@ setMethod("show", "CovariateData", function(object) { | |||
| setMethod("summary", "CovariateData", function(object) { | |||
| covariateValueCount <- 0 | |||
| if (!is.null(object$covariates)) { | |||
| covariateValueCount <- covariateValueCount + (object$covariates %>% count() %>% pull()) | |||
| covariateValueCount <- covariateValueCount + (nrow(object$covariates)) | |||
There was a problem hiding this comment.
The change from count() %>% pull() to nrow() won't work with both versions of Andromeda so I might need to add a function to Andromeda that will work with both arrow filesystemdatasets and sqlite tbl references.
| @@ -63,8 +63,6 @@ test_that("test saveCovariateData", { | |||
| expect_error(saveCovariateData()) #empty call error | |||
| expect_error(saveCovariateData(covariateData)) #no file error | |||
| expect_error(saveCovariateData(errCovData, file = saveFileTest)) #non covariateData class error | |||
| expect_message(saveCovariateData(covariateData, file = saveFileTest), | |||
There was a problem hiding this comment.
The new version of andromeda does not disconnect on save.
| @@ -34,7 +34,7 @@ test_that("filterByCohortDefinitionId works", { | |||
|
|
|||
| test_that("filterByCohortDefinitionId handles locally aggregated data", { | |||
| locallyAggregated <- aggregateCovariates(covariateData) | |||
| expect_error(filterByCohortDefinitionId(locallyAggregated, 1), "no such column") | |||
| expect_error(filterByCohortDefinitionId(locallyAggregated, 1)) | |||
There was a problem hiding this comment.
still catches the error message but the message text is different.
| @@ -60,11 +60,11 @@ runSpotChecks <- function(connectionDetails, cdmDatabaseSchema, ohdsiDatabaseSch | |||
| results <- results[order(results$rowId), ] | |||
|
|
|||
| covariateIds <- covariateData$covariateRef %>% | |||
| filter(rlang::sym("analysisId") == 1) %>% | |||
| select(rlang::sym("covariateId")) | |||
There was a problem hiding this comment.
this syntax doesn't work with arrow. The change should work with both the old and new versions.
| @@ -15,8 +15,8 @@ test_that("Test stdDiff continuous variable computation", { | |||
| # FeatureExtraction::saveCovariateData(data, "inst/testdata/continuousCovariateData.zip") | |||
| # ------------------------------------------------------------------------------ | |||
|
|
|||
|
|
|||
| data <- loadCovariateData(getTestResourceFilePath("continuousCovariateData.zip")) | |||
There was a problem hiding this comment.
For some reason this function was causing problems with devtools::test(). Inlining the system.file call fixed it.
| @@ -1,15 +1,17 @@ | |||
| # Download the JDBC drivers used in the tests | |||
|
|
|||
| oldJarFolder <- Sys.getenv("DATABASECONNECTOR_JAR_FOLDER") | |||
There was a problem hiding this comment.
I made a few changes to the setup file for efficiency.
…ovariates by cohort ID when no cohortDefinitionId present. Fixes unit test that didn't pick up arrow error.
…Lite Andromeda versions
Codecov Report
@@ Coverage Diff @@
## develop #179 +/- ##
===========================================
- Coverage 93.47% 93.08% -0.40%
===========================================
Files 15 15
Lines 1165 1171 +6
===========================================
+ Hits 1089 1090 +1
- Misses 76 81 +5
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
@ablack3 thanks for making these changes. Based on the HADES call yesterday, I'm going to mark this PR as a draft in the event we want to come back to the Arrow implementation. Thanks! |
|
Yea sounds good. I'm hopeful that we'll be using arrow at some point but there are some issues that need to get worked out in arrow first. |
@anthonysena, I had to make a few changes to FeatureExtraction to get tests passing with the Arrow branch.
One thing I need help with is to know how to update the data in inst/testdata. How is this data created? Can you add a script in extras/ that I can run to update the data. I'm assuming it takes a while to create the testdata which is why it is not created on the fly.