Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JOSS review: paper comments #304

Open
10 of 11 tasks
vankesteren opened this issue Aug 5, 2024 · 2 comments
Open
10 of 11 tasks

JOSS review: paper comments #304

vankesteren opened this issue Aug 5, 2024 · 2 comments

Comments

@vankesteren
Copy link

vankesteren commented Aug 5, 2024

Hey @gavinsimpson,

congrats on your submission to JOSS! This is an issue coming out of the JOSS review here. This looks like a great & very useful package. Here is a checklist for points relating to your paper.

  • I have a feeling that the statement of need could be improved to instil an increased sense of urgency in the reader. What is the current problem with GAMs which is solved by {gratia}? Who will use it? The last paragraph of that section (starting line 82) is what I actually want to read much earlier, maybe even in the summary section.
  • Simplify package loading in your example usage in the paper. Maybe the following would be easier to parse for newcomers? for (pkg in pkgs) library(pkg, character.only = TRUE)
  • In the paper, add a code comment before the line crs <- "+proj=ortho +lat_0=20 +lon_0=-40" for accessibility purposes. Something like: # Define the coordinate system to use for plotting on a map.
    These geo coordinate reference system codes are not clear to newcomers!
  • In the paper and "getting started" vignette alike, it would be nice to have a short explanation of what a "partial effect" means. What are we actually seeing in the plots? Is that the smooth evaluated at the average of all other variables, is it evaluated marginalizing out the other variables, or is some other reference chosen? Some engagement with, e.g., the marginaleffects package (Arel-Bundock, Greifer, & Heiss, Forthcoming) might be appropriate here. This relates to the next point
  • State of the field: in the JOSS review checklist, it says "Do the authors describe how this software compares to other commonly-used packages?". This is currently not the case, so a section for this needs to be added.
  • A bit nitpicky, but consider making the following language more accessible for newcomers
    • L132: "which is easily plotted using" (easy for experts, not for newcomers)
    • l135: "quite a simple matter to calculate" (simple for experts, not for newcomers)
    • L159: "While it would be a simple matter to compute the interval with base R commands" (idem)
  • Space before comma on line 150
  • Maybe consider adding a very short 1 paragraph conclusion to your article (personal preference, feel free to ignore / check off)

Arel-Bundock V, Greifer N, Heiss A (Forthcoming). “How to Interpret Statistical Models Using marginaleffects in R and Python.” Journal of Statistical Software. https://marginaleffects.com

gavinsimpson added a commit that referenced this issue Aug 5, 2024
gavinsimpson added a commit that referenced this issue Aug 5, 2024
gavinsimpson added a commit that referenced this issue Aug 5, 2024
@gavinsimpson
Copy link
Owner

Re

Simplify package loading in your example usage in the paper. Maybe the following would be easier to parse for newcomers?

I don't really agree here; while the suggested change might be easier to understand for newcomers, I don't see why we can't use pretty basic language features. And as most people (certainly the intended users gratia) probably don't understand for() loops anyway, I don't see the issue with using vapply().

@gavinsimpson
Copy link
Owner

@vankesteren I have now addressed all the points raised in your review comments above, with the exception of the one about loading packages as discussed in this thread.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants