Small issues with vignette raised by Pat in email
qdread opened this issue · 4 comments
@billspat I just took the liberty of pasting some stuff you wrote in your email into some issues on here. Thanks again for the feedback.
- While the " stringsAsFactors" option is only needed for R version < 4.0 some folks are old computers that can't upgrade. If your package doesn't require R 4, then I would leave this in.
I noticed in the vignette that a new column "log_weight" is created when the data set already has a "logweight" column - are the different bases? Is the new column necessary?- I did find a few other issues in the example code (Roxygen mainly) when I ran the 'check package' in Rstudio but you are probably aware of those.
I fixed a few of the issues that come up on package check (e16f4b4) but mainly those involved with the source code, not the vignette example code yet. So that still needs to be done
Also yes, the logweight column in the original dataset was log base e. The newly calculated column has log base 10. It won't change the results at all but the log base 10 is more interpretable I think. So no need to change that.
All right, I fixed the vignette so that it no longer depends on dplyr! It still depends on ggplot2, so I just went ahead and added ggplot2 to the import dependencies of the entire package (since it's needed for Ostats_plot() anyway). I also had to add gridExtra to the dependencies b/c of the grid.arrange function we call. See commit 7b2580d.
Basically the only thing left here are the issues which we will see anyway when we run devtools::check()