Delimiter 507

[spgarbet]

No description provided.

[spgarbet]

exportFileRepositoryListing still needs upgraded, but I wanted to get this PR looked at for code review at least.

[jubilee2]

I like the overall direction here, but I think there is a blocking implementation issue: several places now use as.data.frame(makeApiCall(...), sep = rcon$csv_delimiter()). as.data.frame() does not parse CSV text using sep, so this is not equivalent to read.csv(..., sep=...).

If makeApiCall() returns CSV text, these calls likely need read.csv(text = as.character(...), sep = rcon$csv_delimiter(), na.strings = "", stringsAsFactors = FALSE) or a shared parser helper. If makeApiCall() already returns a parsed object, then passing sep here has no effect.

I’d recommend fixing this pattern before merge, since otherwise the delimiter support may appear to be wired through without actually changing parsing behavior.

[spgarbet]

@jubilee2 makeApiCall returns the response. The later call to as.data.frame via S3 dispatch to as.data.frame.response. This is a makeApiCall aware function. It corrects a host of other issues with REDCap returns from API Calls. That call turns it over to utils::read.csv has those settings as defaults and allows overrides.

https://github.com/vubiostat/redcapAPI/blob/main/R/makeApiCall.R#L325-L365

[jubilee2]

Thanks for the clarification — that makes sense.

I had initially assumed this was using the base as.data.frame() behavior, but I see now that makeApiCall() returns a response object and that S3 dispatch routes this through as.data.frame.response(), which ultimately calls utils::read.csv().

Given that, passing sep = rcon$csv_delimiter() here should indeed propagate correctly to the CSV parsing step.

Thanks for pointing that out.

[jubilee2]

[ FAIL 0 | WARN 0 | SKIP 14 | PASS 2069 ]