JOSS review

[nhejazi]

Code/Repo

• Note that the example in the README doesn't work immediately when copy-pasted into an R session (I often do this, presumably others too) because the object df used in the example is created in a hidden Rmd code block, which downloads a CSV of a dataset that is then cleaned to produce df. A cleaner solution would be to move this hidden block to a standalone R script (stored in the package) and distribute the cleaned dataset as part of your package, as described at https://r-pkgs.org/data.html; this has the advantage of making the package self-contained wrt data, as changes in the availability of that CSV won't affect this package's examples/demos.
• Code organization: Most of the package's core functions seem to be organized in a file general.R, which is fine but may prove challenging to work with for contributors. It's recommended that functions be stored in individual files or at least thematically organized (e.g., classes.R to store your S3 class definitions); see https://r-pkgs.org/code.html#sec-code-organising.
• The suite of unit tests seems to be reasonably diverse but it's hard to get a sense without a code coverage check in place. On a related note, GitHub Actions seems to be activated but is only being used to produce the JOSS PDF. A better use of the Actions workflow would be to automate the running of unit tests (activated on push and PRs) and code coverage checks; some examples of actions for R are at https://github.com/r-lib/actions
• Per JOSS review guidelines, some statement about community guidelines is necessary. This could be either as a separate markdown file in the repo or a new section in the existing README file.

Paper

• There's a minor typo in the abstract (line 14 of JOSS PDF): "The joint variable importance plot (jointVIP) package to guides decisions about which variables to prioritize for adjustment by quantifying and visualizing each variable's relationship to both treatment and outcome" (emphasis mine).
• On "Development" (lines 49-50 of JOSS PDF): It would be clearer to note that the package exposes a new S3 class, called jointVIP, and exposes methods of the generic functions print(), summary(), and plot() for this class (as opposed to saying that it "leverages system generic functions").
• On "Usage", a few typos: "outcomeis" instead of "outcome is" and "functionto" instead of "function to"

[ldliao]

@nhejazi apologies for the delay in response!

Thank you so much for taking the time to review.
The reorganization of the code will be taking some more time; but the edits in paper is completed (as seen below). Once approved, I'll go ahead and merge the paper_edits branch with the main branch.

Paper

• There's a minor typo in the abstract (line 14 of JOSS PDF): "The joint variable importance plot (jointVIP) package to guides decisions about which variables to prioritize for adjustment by quantifying and visualizing each variable's relationship to both treatment and outcome" (emphasis mine).

  • Thank you for highlighting this typo. The updated text is below.
  • The joint variable importance plot (jointVIP) package guides decisions about which variables to prioritize for adjustment by quantifying and visualizing each variable's relationship to both treatment and outcome.

• On "Development" (lines 49-50 of JOSS PDF): It would be clearer to note that the package exposes a new S3 class, called jointVIP, and exposes methods of the generic functions print(), summary(), and plot() for this class (as opposed to saying that it "leverages system generic functions").

  • Thank you for pointing out this for clarification. The updated text is below.
  • The jointVIP package was created in the R programming language. The package creates a new S3 class called "jointvip" and uses S3 generic to dispatch print(), summary(), and plot().

• On "Usage", a few typos: "outcomeis" instead of "outcome is" and "functionto" instead of "function to"

  • Thank you for pointing out these typos. They have been updated accordingly.

[nhejazi]

hi @ldliao, please see a recent comment in openjournals/joss-reviews#6093 about the status of this review. my understanding is that some of the changes to the code are still outstanding, so my JOSS review needs to stay open, but please let me know if the changes have been made and i'll then check off the relevant items

[ldliao]

Hi @nhejazi thank you for your comment! Yes, this is still outstanding, thank you for your patience.

[ldliao]

Hello @nhejazi Thank you so much for your patience and taking the time to review this package. Please see point by point response to your concerns for code/repo below. (I have corrected the paper reviews in previous comment above)

Please let me know if you have any questions or additional recommended edits. Many thanks!

Code/Repo

• Note that the example in the README doesn't work immediately when copy-pasted into an R session (I often do this, presumably others too) because the object df used in the example is created in a hidden Rmd code block, which downloads a CSV of a dataset that is then cleaned to produce df. A cleaner solution would be to move this hidden block to a standalone R script (stored in the package) and distribute the cleaned dataset as part of your package, as described at https://r-pkgs.org/data.html; this has the advantage of making the package self-contained wrt data, as changes in the availability of that CSV won't affect this package's examples/demos.

  • Thank you for making note of this. Now the README is updated to load the data. The data has been updated to be stored in a standalone R script (as suggested). An updated reference is also in the README.

• Code organization: Most of the package's core functions seem to be organized in a file general.R, which is fine but may prove challenging to work with for contributors. It's recommended that functions be stored in individual files or at least thematically organized (e.g., classes.R to store your S3 class definitions); see https://r-pkgs.org/code.html#sec-code-organising.

  • Thank you for the note on the organization. There is a major update to all the functions and organization such that each major function is now stored as its individual files with one support.R file with the supporting functions.

• The suite of unit tests seems to be reasonably diverse but it's hard to get a sense without a code coverage check in place. On a related note, GitHub Actions seems to be activated but is only being used to produce the JOSS PDF. A better use of the Actions workflow would be to automate the running of unit tests (activated on push and PRs) and code coverage checks; some examples of actions for R are at https://github.com/r-lib/actions

  • Thank you for note on the unit tests. Code coverage check has been updated with both Actions workflow (test-coverage) as well as a more organized and covr. Additional unit tests were added such that the jointVIP package is covered 97.83%. Additional actions were added including R-CMD-check and pkgdown.

• Per JOSS review guidelines, some statement about community guidelines is necessary. This could be either as a separate markdown file in the repo or a new section in the existing README file.

  • Thank you for making notes about the community guidelines. CONTRIBUTING.md is included.

[nhejazi]

thanks for these updates @ldliao -- i'll take another look through the package in the next two weeks