darwin-eu / omopgenerics

https://darwin-eu.github.io/omopgenerics/
Apache License 2.0
2 stars 1 forks source link

cohort error if not permanent tbl #421

Closed edward-burn closed 3 months ago

edward-burn commented 3 months ago

closes #413

library(omopgenerics)
#> 
#> Attaching package: 'omopgenerics'
#> The following object is masked from 'package:stats':
#> 
#>     filter
library(IncidencePrevalence)
library(dplyr)
#> 
#> Attaching package: 'dplyr'
#> The following objects are masked from 'package:stats':
#> 
#>     filter, lag
#> The following objects are masked from 'package:base':
#> 
#>     intersect, setdiff, setequal, union

cdm <- mockIncidencePrevalenceRef()
#> ■■■■■■■■■■■■■■■■ 50% | ETA: 2s
#> 
cdm <- generateDenominatorCohortSet(cdm, "denom")
#> ℹ Creating denominator cohorts
#> ✔ Cohorts created in 0 min and 5 sec
cdm$temp <- cdm$denom |> 
  compute() 

# still work
cohortCount(cdm$temp)
#> # A tibble: 1 × 3
#>   cohort_definition_id number_records number_subjects
#>                  <int>          <int>           <int>
#> 1                    1              1               1
attrition(cdm$temp)
#> # A tibble: 8 × 7
#>   cohort_definition_id number_records number_subjects reason_id reason          
#>                  <int>          <int>           <int>     <int> <chr>           
#> 1                    1              1               1         1 Starting popula…
#> 2                    1              1               1         2 Missing year of…
#> 3                    1              1               1         3 Missing sex     
#> 4                    1              1               1         4 Cannot satisfy …
#> 5                    1              1               1         5 No observation …
#> 6                    1              1               1         6 Doesn't satisfy…
#> 7                    1              1               1         7 Prior history r…
#> 8                    1              1               1        10 No observation …
#> # ℹ 2 more variables: excluded_records <int>, excluded_subjects <int>
settings(cdm$temp)
#> # A tibble: 1 × 9
#>   cohort_definition_id cohort_name        age_group sex   days_prior_observation
#>                  <int> <chr>              <chr>     <chr>                  <dbl>
#> 1                    1 denominator_cohor… 0 to 150  Both                       0
#> # ℹ 4 more variables: start_date <date>, end_date <date>,
#> #   target_cohort_definition_id <int>, target_cohort_name <chr>

# will fail with informative error
cdm$temp |> 
  recordCohortAttrition("a")
#> Error in `populateCohortSet()` at OMOPGenerics/R/classCohortTable.R:79:3:
#> ✖ Table name for cohort could not be inferred.
#> ℹ The cohort table must be a permanent table when working with databases.
#> ℹ Use dplyr::compute(temporary = FALSE, ...) to create a permanent table from a
#>   temporary table.
newCohortTable(cdm$temp)
#> Error in `populateCohortSet()` at OMOPGenerics/R/classCohortTable.R:79:3:
#> ✖ Table name for cohort could not be inferred.
#> ℹ The cohort table must be a permanent table when working with databases.
#> ℹ Use dplyr::compute(temporary = FALSE, ...) to create a permanent table from a
#>   temporary table.
validateCohortArgument(cdm$temp)
#> Error in `validateCohortArgument()`:
#> ✖ Table name for cohort could not be inferred.
#> ℹ The cohort table must be a permanent table when working with databases.
#> ℹ Use dplyr::compute(temporary = FALSE, ...) to create a permanent table from a
#>   temporary table.

Created on 2024-07-29 with reprex v2.1.0

edward-burn commented 3 months ago

@catalamarti have added an internal function. One remaining question is whether we should throw a warning/ error if using cohortCount/ settings/ attrition on a cohort that has become a temp table?

catalamarti commented 3 months ago

Should they keep the class if they are temporal tables?

I would write a method for compute.cohort_table:

What do you think? @edward-burn I can open a separate issue to discuss that

edward-burn commented 3 months ago

Should they keep the class if they are temporal tables?

I would write a method for compute.cohort_table:

* If it is permanent table -> the attributes are copied to the `new_table_name_set`

* if temporary, we lose the attributes.

What do you think? @edward-burn I can open a separate issue to discuss that

@catalamarti yes let's make it a separate issue