Skip to content

Commit

Permalink
Fix bug where list attributes were lost
Browse files Browse the repository at this point in the history
  • Loading branch information
anthonysena committed Nov 13, 2024
1 parent bc04ae9 commit 946f1ac
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 5 deletions.
13 changes: 9 additions & 4 deletions R/StrategusModule.R
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ StrategusModule <- R6::R6Class(
checkmate::assertClass(executionSettings, "ExecutionSettings", add = errorMessages)
checkmate::reportAssertions(collection = errorMessages)

.replaceAttributes <- function(s) {
.replaceProperties <- function(s) {
if (inherits(s, "covariateSettings") && "fun" %in% names(attributes(s))) {
if (attr(s, "fun") == "getDbCohortBasedCovariatesData") {
# Set the covariateCohortDatabaseSchema & covariateCohortTable values
Expand All @@ -329,10 +329,10 @@ StrategusModule <- R6::R6Class(
}
if (is.null(names(covariateSettings))) {
# List of lists
modifiedCovariateSettings <- lapply(covariateSettings, .replaceAttributes)
modifiedCovariateSettings <- lapply(covariateSettings, .replaceProperties)
} else {
# Plain list
modifiedCovariateSettings <- .replaceAttributes(covariateSettings)
modifiedCovariateSettings <- .replaceProperties(covariateSettings)
}
return(modifiedCovariateSettings)
}
Expand All @@ -351,7 +351,12 @@ StrategusModule <- R6::R6Class(
return(.replaceCovariateSettingsCohortTableNames(x, executionSettings))
} else if (is.list(x)) {
# If the element is a list, recurse on each element
return(lapply(x, replaceHelper))
# Keep the original attributes by saving them before modification
attrs <- attributes(x)
newList <- lapply(x, replaceHelper)
# Restore attributes to the new list
attributes(newList) <- attrs
return(newList)
} else {
# If the element is not a list or "covariateSettings", return it as is
return(x)
Expand Down
14 changes: 13 additions & 1 deletion tests/testthat/test-Settings.R
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,13 @@ test_that("Test internal function for modifying covariate settings", {
# 1) covariate settings that do not contain cohort table settings
# 2) covariate settings that contain cohort table settings
# 3) a list of covariate setting that has 1 & 2 above
# 4) Something other than a covariate setting object
esModuleSettingsCreator <- EvidenceSynthesisModule$new()
evidenceSynthesisSourceCmGrid <- esModuleSettingsCreator$createEvidenceSynthesisSource(
sourceMethod = "CohortMethod",
likelihoodApproximation = "adaptive grid"
)

cov1 <- FeatureExtraction::createDefaultCovariateSettings()
cov2 <- FeatureExtraction::createCohortBasedCovariateSettings(
analysisId = 999,
Expand All @@ -430,7 +437,8 @@ test_that("Test internal function for modifying covariate settings", {
nested1 = cov1,
nested2 = cov2,
nested3 = covariateSettings
)
),
esSettings = evidenceSynthesisSourceCmGrid
)
)
workDatabaseSchema <- "foo"
Expand All @@ -444,6 +452,9 @@ test_that("Test internal function for modifying covariate settings", {
)

testReplacedModuleSettings <- .replaceCovariateSettings(moduleSettings, executionSettings)
# For visual inspection
#ParallelLogger::saveSettingsToJson(moduleSettings, "before_unit_test.json")
#ParallelLogger::saveSettingsToJson(testReplacedModuleSettings, "after_unit_test.json")
expect_equal(testReplacedModuleSettings$analysis$something[[1]]$covariateCohortDatabaseSchema, NULL)
expect_equal(testReplacedModuleSettings$analysis$something[[1]]$covariateCohortTable, NULL)
expect_equal(testReplacedModuleSettings$analysis$something[[2]]$covariateCohortDatabaseSchema, workDatabaseSchema)
Expand All @@ -459,4 +470,5 @@ test_that("Test internal function for modifying covariate settings", {
expect_equal(testReplacedModuleSettings$analysis$somethingElse$nested3[[1]]$covariateCohortTable, NULL)
expect_equal(testReplacedModuleSettings$analysis$somethingElse$nested3[[2]]$covariateCohortDatabaseSchema, workDatabaseSchema)
expect_equal(testReplacedModuleSettings$analysis$somethingElse$nested3[[2]]$covariateCohortTable, cohortTableNames$cohortTable)
expect_equal(class(testReplacedModuleSettings$analysis$esSettings), class(moduleSettings$analysis$esSettings))
})

0 comments on commit 946f1ac

Please sign in to comment.