Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow integer as cohort ID #146

Closed
schuemie opened this issue May 27, 2024 · 1 comment · Fixed by #166
Closed

Allow integer as cohort ID #146

schuemie opened this issue May 27, 2024 · 1 comment · Fixed by #166
Milestone

Comments

@schuemie
Copy link
Member

The isCohortDefinitionSet() currently throws an error if the cohortId is an integer. However, I think integer should be totally fine. Could we be a bit more lenient?

(Currently Capr::makeCohortSet() generates the concept ID as integer, I would prefer it to work without having to convert the integer to numeric.)

Reproducible example:

cds <- data.frame(cohortId = as.integer(1),
                  cohortName = "",
                  sql = "",
                  json = "")
CohortGenerator::isCohortDefinitionSet(cds)
# [1] FALSE                                                                                                                                                                                                                     
# Warning message:
# In checkAndFixCohortDefinitionSetDataTypes(x = x, fixDataTypes = FALSE,  :
#   Your cohortDefinitionSet had a mismatch in data types. Please check your cohortDefinitionSet to ensure it conforms to the following expected data types:Expected column == data type
# --------------------------
# cohortId == numeric
# cohortName == character
# sql == character
# json == character
# --------------------------
# Your cohortDefinitionSet 
# --------------------------
# cohortId == integer
# cohortName == character
# sql == character
# json == character
@anthonysena anthonysena added this to the v0.10 milestone May 30, 2024
@anthonysena
Copy link
Collaborator

Absolutely. I'd like to tackle #152 and then make this check more lenient once we remove bit64.

@anthonysena anthonysena linked a pull request Jun 17, 2024 that will close this issue
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 a pull request may close this issue.

2 participants