[Joss Review] Package

[AKuederle]

Hey everybody,

I will just use this issue to collect some thought about your package. Feel free to add your comments and opinions and we can figure out if there are things that would need to be addressed in a separate focused issue.

Major issues (things I think you be addressed):

• Provide more information about the vignettes in your README. I think they will be the starting point for most people who want to get started with the package.
• To build the vignettes knitr is required. The installation fails otherwise. I know most people have knitr installed, but this should be mentioned somewhere or listed in the dependencies.

Suggestions (things I thing would be cool to address):

• A link to to the devtools package in the installation section would be awesome to make things really easy for newer R users.
• Your Linux issue guide is specific for apt based systems (or maybe only ubuntu). This is fine but should be mentioned. Maybe a sentence like: "For apt based systems (e.g., Debian or Ubuntu) you can run the commands below. For other distros (or in case these commands should fail), please check the package repository of your distro for the correct command and package names to use.
• Contributing to a repo does not just mean adding code. Your contribution guide should also mention where people should open issues and how to contribute new ideas/suggestions
• The file data.R contains comments about the specific number of rows for datatype. Doesn't this depend on my input data? Or am I misunderstanding something.
• I am not sure if this is typical in the R-world, but a hosted documentation with browsable function doc-strings would be awesome (thinking ReadTheDocs in the Python world)

Awesome things:

• Including a Docker version is awesome for testing
• Test for all mature functionality
• Test data included
• CI automation for tests
• Overall repo structure really clean
• Documentation of single functions is clear and complete
• Long form documentaion is awesome

General comments:

• The described Linux installation issues were not present in Manjaro Linux
• Unfortunally, I can not comment on the overall quality of your code, as I am not familiar enough with R

[philerooski]

Some really good suggestions here @AKuederle. I'll provide comments on some points and address the others in one or more pull requests.

To build the vignettes knitr is required. The installation fails otherwise. I know most people have knitr installed, but this should be mentioned somewhere or listed in the dependencies.

R package dependencies are included under two headers within the DESCRIPTION file. Dependencies under Imports are required for package installation whereas non-essential dependencies under Suggests are used for testing and building documentation such as vignettes. knitr is included under Suggests within the DESCRIPTION file. This seems like the proper place for it. (Since Suggests style dependencies are usually installed by setting the dependencies parameter to TRUE during installation I will update the devtools::install_github example function call in the README to include a dependencies = TRUE argument. Normally the installer defaults to only installing the bare minimum of dependencies, those under Imports).

Contributing to a repo does not just mean adding code. Your contribution guide should also mention where people should open issues and how to contribute new ideas/suggestions.

Addressed by #206 and #211

The file data.R contains comments about the specific number of rows for datatype. Doesn't this depend on my input data? Or am I misunderstanding something.

data.R is describing specific sample data included with the package, rather than generic "accelerometer_data" or "gyroscope_data". When you load mhealthtools into the namespace these sample data are automatically imported as well.

I am not sure if this is typical in the R-world, but a hosted documentation with browsable function doc-strings would be awesome (thinking ReadTheDocs in the Python world)

There is a popular package called pkgdown often used for this purpose. I'll look to see if this is something I can set up without too much additional effort.

[philerooski]

Provide more information about the vignettes in your README. I think they will be the starting point for most people who want to get started with the package.

While addressing #206 I inserted a Getting Help section after the high-level Usage section. Getting Help includes a sentence on what vignettes are provided. Do you think this is enough information? I don't want to be too specific in the README about the content of the vignettes since the vignettes are subject to change in the future.

[AKuederle]

Provide more information about the vignettes in your README. I think they will be the starting point for most people who want to get started with the package.

While addressing #206 I inserted a Getting Help section after the high-level Usage section. Getting Help includes a sentence on what vignettes are provided. Do you think this is enough information? I don't want to be too specific in the README about the content of the vignettes since the vignettes are subject to change in the future.

That looks good to me!

[AKuederle]

Thank you for clarifying these points. I think it was mostly my lack of experience in the R ecosystem that let to these questions.

Regrading the installation of the vignets:
Could you add the install commands required, to get everything working to the README? I think this would resolve all confusion.

Regarding pkgdown: I really think this is just a nice to have and for sure not a requirement!

[philerooski]

I've merged #212 which makes changes to the example install commands. I tested the new instructions on the latest r-base docker image, and after installing devtools I was able to install mhealthtools+vignettes with the new command.

It turns out that building and publishing a docs website is incredibly easy with pkgdown + github pages (you can preview what it would look like here: https://philerooski.github.io/mHealthTools/). Though there is one small issue with referencing a figure from the paper using its relative path. This is something I would want to set up to work with Travis anyhow so I will add it to the to-do list and get around to it at a more convenient time.