OuhscBbmc / REDCapR

R utilities for interacting with REDCap
https://ouhscbbmc.github.io/REDCapR
Other
112 stars 45 forks source link

use `readr::read_csv()` in `checkbox_choices()` #500

Closed wibeasley closed 11 months ago

wibeasley commented 11 months ago

@BlairCooper is threatening to try more irregular checkbox scenarios. The checkbox_choices() is using a single regex pattern to process the whole string, and it is getting pretty complicated for me (and I unironically love regexes):

pattern_checkboxes <- "(?<=\\A| \\| |\\| )(?<id>\\d{1,}), (?<label>[^|]{1,}?)(?= \\| |\\| |\\Z)"

readr's csv parser is widely tested against a lot of good & bad csvs styles, so let's use that instead.

I popped this little snippet into checkbox_choices() and all the tests passed immediately.

  strsplit(select_choices, split = "|", fixed = TRUE)[[1]] |>
    I() |>
    readr::read_csv(
      col_names = c("id", "label"),
      col_types = "cc"
    )

@BlairCooper, do you want to try this new branch with your scenarios?

Unless a pipe is used (for some other reason than separating choices) or a comma is used (for some other reason than separating the id from the label), I don't see how this can be improved to be more robust or maintainable.

BlairCooper commented 11 months ago

@wibeasley One thing I like about the current implementation is that it exercises regex_named_captures() as well. I'm my next pull request I'll include some documentation on the RegEx.

I also tested your snippet and it does work though it is slower. Ran the metadata-utilities test three times each with regex and csv. regex ran pretty consistently at 0.3 seconds, csv ran pretty consistently at 0.5 seconds.

Side note: It is actually now not possible to create these scenarios in REDCap 13.7.5. Looks like they've added JavaScript to clean up the options when the field is saved. I was encountering these issues with fields that had been setup with an earlier version of REDCap. So you can't create these scenarios in 13.7.5 but they can still exist in that version.

wibeasley commented 11 months ago

close issue. Replaced by #504