aumath-advancedr2019 / Sampling

Package for making Permutation and Bootstrapping in R
https://aumath-advancedr2019.github.io/Sampling/
0 stars 0 forks source link

Peer review #1

Open SolveigOe opened 4 years ago

SolveigOe commented 4 years ago

1. Purpose of the package

The purpose of the package is described shortly in the descriptions of the functions, which is nice and easy to understand. I think a it is great with an easy-to-understand (and use) permutation and bootstrap function.

2. Completeness

I think the package is nice and with enough functions to fulfill the goal of the package. I especially like the bootstrap function, which we did not think about :). The data included in the package and used in the examples demonstrates the usefullness of the package very well. I do not believe any other packages are needed to use the package, which is super nice. It would be nice, if the authors of the package was in the description, since this is not easy to find.

3. Code quality and sophistication

The package installs without any problems, but unfortunately I could not install the vignette, it gave an error in line 20-21, where it faild to rebuild it. I also had a few problems with the examples - a typing error in the permutation() function, "Groups" instead of "Group" and for some reason I can not plot the bootstrap result, error: Error in gpar(col = "gray") : could not find function "gpar". The functions seem robust when testing with an other example (mtcars). I really like the plot from the permutation and the output, specifically for the permutation() function as a htest, and that you have two classes, which I guess is why it is able to plot so nicely.

4. Documentation

I believe the functions are well documented, but of course it would have been nice to see the vignette. Maybe a bit more information in the description of the function would be nice. I really like the examples and that you used a real dataset, this is nice and really shows the usefullness of this package. The documentation of the dataset is well written, short and easy to understand - this is really nice.

5. Interface

I really like the interface of the package. It is in general easy to understand, and it is not necessary to look a lot at the documentation to be able to use the functions, which is super nice. It could be nice, if the functions gave warnings/error messages that made more sense, when an argument, that is wrong is used (I did that first, because I did not think at all, and it gave me a pretty non-understandable error message). So there is an error message (I do not know if you made it?), but it is very hard to understand.

6. (optional) Additional comments

7. Conclusion

I really like your package, and the fact that it gives both the oppertunity to make a permutation test and to bootstrap the data.

Possible improvements:

ute commented 4 years ago

@SolveigOe : did you set build_vignettes=TRUE on install.github? I like that you write this explicitly in the readme.md on your own package :-)

SolveigOe commented 4 years ago

@ute: Yes, I did, and it gives an error: Error: Failed to install 'Sampling' from GitHub: System command error, exit status: 1, stdout + stderr (last 10 lines): E> Quitting from lines 20-21 (permutation.Rmd) E> Error: processing vignette 'permutation.Rmd' failed with diagnostics: E> there is no package called 'PackageProject' E> --- failed re-building 'permutation.Rmd' E> E> SUMMARY: processing the following file failed: E> 'permutation.Rmd' E> E> Fejl: Vignette re-building failed.

LotteHind commented 4 years ago

Maybe there was some issue with our package having different names, "Permutation", "Sampling", "PackageProject". Now it is called "Sampling" everywhere and both Cornelia and I can make the Vignette work.

SolveigOe commented 4 years ago

It works now! No problems anymore, and the vignette looks very nice (I like your table of contents in the vignette!) :)