concept function #75
concept function #75
Conversation
R/conceptCohortSet.R
Outdated
| conceptCohortSet <- function(cdm, | ||
| conceptSet = NULL, | ||
| name = "cohort", | ||
| verbose = TRUE) { |
There was a problem hiding this comment.
Can we remove this argument, we have removed it from other packages and try to just provide messages that strike a balance (if we want to have options to turn off I think we could do this as a global option for all our packages)
R/conceptCohortSet.R
Outdated
| #' | ||
| #' @return A cohort_table object. | ||
| #' | ||
| conceptCohortSet <- function(cdm, |
There was a problem hiding this comment.
what about conceptCohort() for an even shorter name?
R/conceptCohortSet.R
Outdated
| #' | ||
| conceptCohortSet <- function(cdm, | ||
| conceptSet = NULL, | ||
| name = "cohort", |
There was a problem hiding this comment.
I don't think we should provide a default name or conceptSet and we should always require the user to give us both
R/conceptCohortSet.R
Outdated
| cdm <- validateCdm(cdm) | ||
| name <- validateName(name) | ||
| verbose <- validateVerbose(verbose) | ||
| if (length(conceptSet) == 0) { |
There was a problem hiding this comment.
need to figure this out, because current behaviour would give errors for all these below examples (but if we change that we would have to deal with it)
conceptSet<-list()
length(conceptSet)
omopgenerics::newCodelist(conceptSet)
conceptSet<-list("my_empty_cs" = NA)
length(conceptSet)
omopgenerics::newCodelist(conceptSet)
conceptSet<-list("my_non_empty_cs" = c(1,2),
"my_empty_cs" = NA)
length(conceptSet)
omopgenerics::newCodelist(conceptSet)
R/conceptCohortSet.R
Outdated
| conceptCohortSet <- function(cdm, | ||
| conceptSet = NULL, | ||
| name = "cohort", | ||
| verbose = TRUE) { |
There was a problem hiding this comment.
do we not need an end date argument, or would this function only follow one option?
| @@ -46,3 +54,23 @@ validateDateRange<-function(dateRange){ | |||
| } | |||
| return(invisible(dateRange)) | |||
| } | |||
|
|
|||
| validateName <- function(name) { | |||
There was a problem hiding this comment.
@catalamarti general comment (not for this pr)- should these validation functions be exported from omopgenerics?
No description provided.