atorus-research/metacore

yn_to_tf returning NA instead of T/F for Y/N

esimms999-gsk opened this issue · 2 comments

image

Test at "A" in screenshot is checking for 'Yes' or 'yes' or 'No' or 'no' or 'Y' or 'y' or 'N' or 'n', so a 'Y' or 'N' would pass the test. But then at "B" in screenshot, the values of only 'Yes' or 'yes' are assigned TRUE and the values of only 'No' or 'no' are assigned FALSE. 'Y', 'y', 'N', 'n' are assigned to NA.

I discovered this because spec_type_to_ds_vars() calls yn_to_tf() for the keep variable.

Also, a value of 'YES' or 'NO' is unacceptable and triggers the Warning message in the "else" block of the yn_to_tf function.

@statasaurus I think there are two ways we could approach this. First would be to update the regex to be case insensitive (and plug a small logic hole:

test <- c("Yes", "YES", "Y", "yes", "y", "NO", "N", "No", "n", "Yo", "None")

stringr::str_detect(test, regex("^y$|^n$|yes$|no$", ignore_case = T))
# [1]  TRUE  TRUE  TRUE  TRUE  TRUE  TRUE  TRUE  TRUE  TRUE FALSE FALSE

Second would be to loosen our assumptions and just look for strings starting with Y or N:

test <- c("Yes", "YES", "Y", "yes", "y", "NO", "N", "No", "n", "Yo", "None")

stringr::str_detect(test, regex("^y|^n", ignore_case = T))
#  [1] TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE

@esimms999-gsk do you have any thoughts on expectations as a user?

I think it should be the first way you suggest.