JOSS: Code Review @craddm & @anibalsolon

[humanfactors]

This pull request addresses reviewer comments from @craddm on openjournals/joss-reviews#2340

I initially thought there were a few functions that don't currently have examples (e.g. expand_sleep_series()) that would benefit from them. But then I realized that those functions aren't exported, user-facing functions. You might want to make them internal functions (e.g. using @Keywords internal) so that they don't show up in the user-facing API docs

Thanks for noticing this. I've now gone through and added the @keywords internal flag to all functions that are intended purely for developers (and have no useful purpose being in front-facing documentation)

I'm not generally familiar with the area so have no example data of my own to test the package out. The package has a great vignette that generates data of the appropriate type, but one thing I'm left a little curious about is how you normally get data of that type. Specifically, there's a function mentioned in the pdf, parse_sleepwake_sequence(), that's intended for dealing with a common format used by other software for sleep/wake sequences. Examples of this type of data and the use of the function would be appreciated.

This is a good point, so to firstly address question directly. The "sleep times" format used by parse_sleeptimes is predominately custom. A variety of sleep tracking applications return data in formats similar to this (though usually dramatically more complex). We intended the use case for this to be individuals who are transcribing a pen and paper sleep diary to digital via a spreadsheet. It turned out to also be a nice human readable format for generating simulation scenarios.

To make this point clearer, we now mention this fact explicitly in the vignettes/FIPS-simulation-walkthrough.Rmd to match the manuscript.

Now, to focus on your question regarding parse_sleepwake_sequence(), this format typically is returned by actigraphy device algorithms (though sometimes labeled with S and W for sleep/wake). For instance, the dipetkov/actigraph.sleepr package will convert raw actigraphy data to a bitvector. There are many different ways of presenting this, but all our devices have returned it in a newline separated .txt file with a comment header line and then just a series of S and W.

We've made a number of changes to the repository to make clearer what FIPS expects the bitvector to look like, and how to handle such data. Specifically:

• We added an entire section to vignettes/FIPS-simulation-walkthrough.Rmd entitled Bitvector Sequence Format, which shows a full example of using the parse_sleepwake_sequence function.
• We have added an @examples section to the function documentation of parse_sleepwake_sequence

I can see you have some tests set up using testthat, which is great. But I'd also suggest hooking them up to a continuous integration service such as Travis-CI.

Thank you for this encouragement. I had no prior experience with Travis CI, but am glad to now have a successful automated integration build. I believe I've configured this appropriately in f450fad. Screenshot of test passed below.