Skip to content

Phevaluator module - #294

Open
ob325 wants to merge 18 commits into
developfrom
phevaluator
Open

Phevaluator module#294
ob325 wants to merge 18 commits into
developfrom
phevaluator

Conversation

@ob325

@ob325 ob325 commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

This adds the PheValuator module, closes #279

@ob325 ob325 linked an issue Jun 23, 2026 that may be closed by this pull request
@anthonysena

Copy link
Copy Markdown
Collaborator

@ob325 thanks for adding this new module! I see that some of the new PheValuator unit tests are failing when running the R CMD checks. Could you take a look and fix those up? I'll then give this a closer review. Thanks!

Comment thread R/Module-PheValuator.R Outdated
#' }
#' }
createModuleSpecifications = function(analysisName = "Main",
cohortDefinitionSet = data.frame(),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ob325 - we specify the cohort definition set as part of the "Shared Resources" section when constructing the overall Strategus analysis specification. So I'd expect that all cohorts you would have in your pheValuatorAnalysisList are included in that section of the analysis specification. This would include the phenotypeCohortId, xSpecCohortId, xSensCohortId, prevalenceCohortId.

Comment thread R/Module-PheValuator.R Outdated
Comment on lines +59 to +68
for (analysisSpec in spec$pheValuatorAnalysisList) {
private$.executeAnalysis(
analysisSpec = analysisSpec,
connectionDetails = connectionDetails,
executionSettings = executionSettings,
jobContext = jobContext,
outputFolder = outputFolder,
resultsFolder = resultsFolder
)
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we consider adding some messages using the cli package to indicate progress through this loop?

Comment on lines +397 to +407
pvModuleSpecifications <- pvModuleSettingsCreator$createModuleSpecifications(
phenotype = "Type 2 Diabetes",
pheValuatorAnalysisList = list(
list(
analysisId = 1,
description = "Test PheValuator analysis",
createEvaluationCohortArgs = list(xSpecCohortId = 1),
testPhenotypeAlgorithmArgs = list(phenotypeCohortId = 3, cutPoints = c("EV"))
)
)
)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is currently failing in the unit tests since the function signature changed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add module for PheValuator

2 participants