Suggested export options

[PatrickRWright]

Pretty straight forward function to help people troubleshoot why "things" may not be working or train people what type of exports to request.

I would like to add the function call after read_secuTrial_raw() in read_secuTrial() to warn people if they use the wrapper. What do you think @aghaynes @markomi ?

Also I realized that we do not seem to be tracking the export format currently since we assume its always either "CSV format" or "CSV format for MS Excel". I think we should track this export option and catch it if e.g. SAS, CDISC, SPSS is specified. I have added this to the PR too.

closes #17

[codecov-io]

Codecov Report

Merging #119 into master will increase coverage by 0.12%.
The diff coverage is 95.12%.

@@            Coverage Diff             @@
##           master     #119      +/-   ##
==========================================
+ Coverage   95.09%   95.22%   +0.12%     
==========================================
  Files          24       25       +1     
  Lines        1142     1173      +31     
==========================================
+ Hits         1086     1117      +31     
  Misses         56       56

Impacted Files	Coverage Δ
R/read_export_options.R	94.14% <100%> (+0.53%)	⬆️
R/check_export_options.R	100% <100%> (ø)
R/read_secuTrial.R	92.59% <71.42%> (-2.87%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 385e5dc...da8ecd2. Read the comment docs.

[PatrickRWright]

Adding check_export_options() to read_secuTrial() created quite a few warnings in the test suite (as expected). This should not be too hard to remedy. However, before I do that I would like to know if you both agree with running the check.

[aghaynes]

we do not seem to be tracking the export format currently since we assume its always either "CSV format" or "CSV format for MS Excel". I think we should track this export option and catch it if e.g. SAS, CDISC, SPSS is specified.

The first part (tracking the format) we certainly do... this gets the file extension which is saved in export_options as extension

secuTrialR/R/read_export_options.R

Line 143 in 385e5dc

file_extension <- unique(sapply(strsplit(files$Name[-study_options_file_idx], ".", fixed = TRUE), function(x) x[2]))

I dont remember whether we check it though...

Seems like a reasonable idea on the whole though

[PatrickRWright]

@aghaynes so any changes(?), or can I go ahead and adjust the tests accordingly so we don't have 24 warnings?

[markomi]

For me the addition of the warnings to read_secuTrial() would be fine. I'd also be OK with converting the warning to a message, if you prefer.

[aghaynes]

Its ok with me

[markomi]

@PatrickRWright should I merge now, or do you want to add something?

[PatrickRWright]

I will go over the tests and remove the warnings if possible. Edit: Actually, I will change the warnings to messages as suggested by @markomi

Also, how about having "English" as suggested language? Edit: I will do this.

[PatrickRWright]

I'm done.

[markomi]

Thanks @PatrickRWright! This will save us a lot of troubleshooting time. I'm merging.