JOSS Reviewer Comments - Githubissues

ropensci / jagstargets

Reproducible Bayesian data analysis pipelines with targets and JAGS

https://docs.ropensci.org/jagstargets

Other

10 stars 6 forks source link

JOSS Reviewer Comments #25

Closed AoifeHughes closed 2 years ago

AoifeHughes commented 3 years ago

Hi there,

Re: joss review here

Very happy to be reviewing this package, off the bat it looks great and like we'll have no problems. But, just to do my job properly, I will be very nitpicky. I promise that if any comments read bluntly that it's just for brevity :)

Many of these comments could probably be ignored, but I am giving a "blind" attempt to use the package and noting any issues one may encounter.

Installing from remotes, maybe worth pointing out that install.packages("remotes") is needed?
License etc. is fine, I wonder if the naming of the authors is okay, usually papers are "Surname et al." rather than "company".
The maths in https://docs.ropensci.org/jagstargets/articles/introduction.html doesn't seem to display the model correctly at the top, it renders as plain text for me

I'm having some issues with rjags, so reinstalling R (this is a me problem). Will Continue review when I have this sorted...

wlandau commented 3 years ago

Thanks for taking the time to review, @AoifeHughes. To address your comments:

263c50cebe6408efa61df03d3e32a901ad67d418 adds install.packages("remotes") to the README.
For the manuscript, please let me know if the YAML front matter should be changed to correctly state my affiliation. For the package itself, I agree the conventions may seem strange. I have been trying to follow the general guidance at https://stackoverflow.com/questions/40704920/listing-a-company-as-the-copyright-holder-of-an-r-package and http://r-pkgs.had.co.nz/description.html, and the practices of popular packages, e.g. https://github.com/tidyverse/dplyr.
Unfortunately, I cannot locally reproduce the issue rendering math formulas, and the site is rendered through r-universe. Hopefully the last commit will fix any transient problems. If not, I can just use plain text.

wlandau commented 3 years ago

@jeroen, do you know why https://docs.ropensci.org/jagstargets/articles/introduction.html has trouble displaying this LaTeX code? Is there an issue MathJax in r-universe? The vignette renders just fine on my local machine.

wlandau commented 3 years ago

This is what I see in Safari:

Screen Shot 2021-11-04 at 10 20 52 AM

jeroen commented 3 years ago

IIRC, our theme made mathjax opt-in. Can you add mathjax: true to your _pkgdown.yml? This should work I think:

template:
  params:
    mathjax: true

But you can also just add mathjax: true to the top. level of your _pkgdown.yml

wlandau commented 3 years ago

Thanks, @jeroen! Seems to have fixed the issue.

AoifeHughes commented 2 years ago

Up and running again:

in the Readme file you do provide an example script under usage, however this is incomplete without the accompanying model which is specified here: https://docs.ropensci.org/jagstargets/articles/introduction.html would it be useful to just include it on the readme also?

Again, sorry a small comment, everything else is looking well and I think I can finished the review shortly.

wlandau commented 2 years ago

Sure, added the model to the README in 9ca970631254f359be2ac008b0e4f766fd2dd6c4.

kthyng commented 2 years ago

@AoifeHughes is this issue completed, in terms of your review for JOSS?

wlandau commented 2 years ago

Looks like the paper was accepted. Thanks everyone for your feedback!