CohortDiagnostics icon indicating copy to clipboard operation
CohortDiagnostics copied to clipboard

WashoutPeriod always defaults to 0 (or triggers sql error)

Open MaximMoinat opened this issue 2 years ago • 1 comments

I was doing some tests with running CD on a cohort without a json definition. This failed as the incidence rate query threw an error due to an empty value for the washoutPeriod. Upon digging into the code, I discovered two issues with the following lines of code:

https://github.com/OHDSI/CohortDiagnostics/blob/5f4d80e9f4210ffaf2ac7d4ab2b26102e4987c58/R/IncidenceRates.R#L241-L250

  1. The tryCatch does not work as intended, because on an empty named list the use of $ will not throw an error. It will simply return NULL, causing a sql error as described above. image

  2. But there is also unintended behaviour when a valid cohort json is given. The last item being get, PriorDays, can't be accessed with the $ operation. This throws an error, making it always go into error block and setting the washout period to 0. image

The following should fix both issues:

washoutPeriod <- cohortExpression$ 
       PrimaryCriteria$ 
       ObservationWindow[["PriorDays"]]
if (is.null(washoutPeriod)) {
  washoutPeriod <- 0
}

MaximMoinat avatar Sep 12 '22 15:09 MaximMoinat

Hi @MaximMoinat, thanks for filing this and coming up with a solution! I have tested this and completed a PR that should make it in to the development build and next release. If you have a need to get this out faster I'm more than happy to make a hotfix for it.

azimov avatar Sep 12 '22 21:09 azimov