JOSS Review

[trashbirdecology]

@mattansb The first section contains elements required to fulfill JOSS review standards. Following are suggested edits that are not required for JOSS.

Required fixes for JOSS review

This section contains required and suggested edits to the manuscript, the software, and the documentation/repository.

• required: Remove extra 'to' in "It is often of interest to to a"
• required: Remove hyphen in 'R-package' (unless this is something common that I am just not privy to; if you keep, stay consistent throughout)
• suggest: add hyphens between words of "easy to use"
• suggest: "In the following," to something like, "in this article"
• suggest: "we show some brief examples" to something like, 'we provide basic usage examples of the most common features for estimating effect size'.
• suggest: remove "However" in "However, a comprehensive overview including in-depth examples are accessible via the dedicated website"
• remove library(effectsize) from the first example. Probably unnecessary to include loading instructions in the paper, but if kept, should be removed from a single, specific example.
• question/minor: should it be noted that it's currently available via CRAN? (not sure JOSS requirements or recommendations on this)
• "Person's" --> "Pearson's"
• "returns an updated model," the function doesn't return an updated model, it provides updated coefficients, including Classes ‘effectsize_table’, ‘see_effectsize_table’, ‘effectsize_std_params’. I.e., the output of standardize_parameters() does not == the output of lm()
• suggest: "from a provided model" -- briefly list the available model types which standardize_paramters can handle
• I suggest editing the entire first paragraph to convince me further why I should care. If the phrasing is not edited, at a minimum, the first sentence should be updated to tell me to "whom" or for what reason it "is of interest to assess..". It may be worth removing this background and package justification out of the "Aims of the Package" section, but this is minor.
• "fills this important gap" Not sure to what "important gap" you refer at this point.
• minor: In other text locations you point to specific vignettes, and state this as such. In "More information is available in the package’s documentation." you point to "Interpret" vignette but say "package documentation"
• suggest: "via direct interaction with contributors and developers." Link to Issues and/or Contributing.md, or explicitly state this is how you prefer feedback/contributions, rather than vaguely saying "direct interaction".
• Community guidelines currently insufficient. Please add a Contributions file, or provide more specific instructions than what is currently provided in readme ("everybody is welcome to contribute by adding support for the interpretation of new indices.").
• "State of the field: Do the authors describe how this software compares to other commonly-used packages?" This is not addressed in current draft.
• Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided? The first paragraph sort of achieves this, but I think it could be written a little more clearly such that the language is easily interpretable by a broader audience.
• A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is? Not sufficiently
• Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)? I've made more specific suggestions on writing below.
• References: Provide refernces for statistics at first instance. E.g., Cohen 1998 isn't referenced until the third instance of Cohen's d in the paper
• References: No reference provided in list for in-text ref to Cohen 1998
• Paper does not currently discuss other relevant packages.
• Dependency warnings on devtools::document():

Warning: [/Users/jburnett/Documents/GitHub/effectsize/R/eta_squared_posterior.R:3] @description Link to unavailable package: rstantools::posterior_predict. there is no package called ‘rstantools’
Warning: [/Users/jburnett/Documents/GitHub/effectsize/R/standardize_parameters.R:19] @details Link to unavailable package: lm.beta::lm.beta. there is no package called ‘lm.beta’

(lm.beta and rstantools are in suggests)

• FAILURE devtools::check() on vignette build @mattansb

Error in (function (command = NULL, args = character(), error_on_status = TRUE,  : 
  System command 'R' failed, exit status: 1, stdout + stderr (last 10 lines):
E> Quitting from lines 261-267 (standardize_parameters.Rmd) 
E> Error: processing vignette 'standardize_parameters.Rmd' failed with diagnostics:
E> Package `lmerTest` required for Satterthwaite approximation. Please install it.
E> --- failed re-building ‘standardize_parameters.Rmd’
E> 
E> SUMMARY: processing the following file failed:
E>   ‘standardize_parameters.Rmd’
E> 
E> Error: Vignette re-building failed.
E> Execution halted

My session info:

devtools::session_info()
#> ─ Session info ───────────────────────────────────────────────────────────────
#>  setting  value                       
#>  version  R version 4.0.1 (2020-06-06)
#>  os       macOS Mojave 10.14.6        
#>  system   x86_64, darwin17.0          
#>  ui       X11                         
#>  language (EN)                        
#>  collate  en_US.UTF-8                 
#>  ctype    en_US.UTF-8                 
#> ─ Packages ───────────────────────────────────────────────────────────────────
#>  package     * version date       lib source        
#>  assertthat    0.2.1   2019-03-21 [1] CRAN (R 4.0.0)
#>  backports     1.1.10  2020-09-15 [1] CRAN (R 4.0.2)
#>  callr         3.5.1   2020-10-13 [1] CRAN (R 4.0.2)
#>  cli           2.1.0   2020-10-12 [1] CRAN (R 4.0.2)
#>  crayon        1.3.4   2017-09-16 [1] CRAN (R 4.0.0)
#>  desc          1.2.0   2018-05-01 [1] CRAN (R 4.0.0)
#>  devtools      2.3.2   2020-09-18 [1] CRAN (R 4.0.2)
#>  digest        0.6.27  2020-10-24 [1] CRAN (R 4.0.2)
#>  ellipsis      0.3.1   2020-05-15 [1] CRAN (R 4.0.0)
#>  evaluate      0.14    2019-05-28 [1] CRAN (R 4.0.0)
#>  fansi         0.4.1   2020-01-08 [1] CRAN (R 4.0.0)
#>  fs            1.4.1   2020-04-04 [1] CRAN (R 4.0.0)
#>  glue          1.4.2   2020-08-27 [1] CRAN (R 4.0.2)
#>  highr         0.8     2019-03-20 [1] CRAN (R 4.0.0)
#>  htmltools     0.5.0   2020-06-16 [1] CRAN (R 4.0.2)
#>  knitr         1.30    2020-09-22 [1] CRAN (R 4.0.2)
#>  magrittr      1.5     2014-11-22 [1] CRAN (R 4.0.0)
#>  memoise       1.1.0   2017-04-21 [1] CRAN (R 4.0.0)
#>  pkgbuild      1.1.0   2020-07-13 [1] CRAN (R 4.0.2)
#>  pkgload       1.1.0   2020-05-29 [1] CRAN (R 4.0.0)
#>  prettyunits   1.1.1   2020-01-24 [1] CRAN (R 4.0.0)
#>  processx      3.4.4   2020-09-03 [1] CRAN (R 4.0.2)
#>  ps            1.3.3   2020-05-08 [1] CRAN (R 4.0.0)
#>  R6            2.4.1   2019-11-12 [1] CRAN (R 4.0.2)
#>  remotes       2.2.0   2020-07-21 [1] CRAN (R 4.0.2)
#>  rlang         0.4.8   2020-10-08 [1] CRAN (R 4.0.2)
#>  rmarkdown     2.3     2020-06-18 [1] CRAN (R 4.0.2)
#>  rprojroot     1.3-2   2018-01-03 [1] CRAN (R 4.0.0)
#>  sessioninfo   1.1.1   2018-11-05 [1] CRAN (R 4.0.0)
#>  stringi       1.5.3   2020-09-09 [1] CRAN (R 4.0.2)
#>  stringr       1.4.0   2019-02-10 [1] CRAN (R 4.0.2)
#>  testthat      2.3.2   2020-03-02 [1] CRAN (R 4.0.0)
#>  usethis       1.6.3   2020-09-17 [1] CRAN (R 4.0.2)
#>  withr         2.2.0   2020-04-20 [1] CRAN (R 4.0.0)
#>  xfun          0.18    2020-09-29 [1] CRAN (R 4.0.2)
#>  yaml          2.2.1   2020-02-01 [1] CRAN (R 4.0.0)
#> 
#> [1] /Library/Frameworks/R.framework/Versions/4.0/Resources/library

JLB suggestions (not required for JOSS review)

• feature request: as a potential user I would find it useful to a quick-reference table in the paper and/or readme that allows me to easily identify what my standardization, conversion, and interpretation options are.
• FYI, 11 warnings on devtools::test(), mostly (if not all) package dependency issues.
• Rename primary branch from 'master' to 'main' (or something else)
• README: section "This package is focused on indices of effect size. But there are hundreds of them! Thus, everybody is welcome to contribute by adding support for the interpretation of new indices. If you’re not sure how to code it’s okay, just open an issue to discuss it and we’ll help :)" is not very helpful as is. I suggest providing more background/relevance of the package, and then, in a separate line, provide more intuitive contributing instruictions
• Add code of conduct to GH repo

JLB to do list

• Check singularity warning when running following example in paper. I haven't yet looked into this, just noting here to remember to do so.

data("ChickWeight")
options(contrasts = c('contr.sum', 'contr.poly'))
ChickWeight$Time <- factor(ChickWeight$Time)
model <- aov(weight ~ Diet * Time + Error(Chick / Time),
+              data = ChickWeight)
Warning message:
In aov(weight ~ Diet * Time + Error(Chick/Time), data = ChickWeight) :
  Error() model is singular

• Tests won't run using testthat::test_check() (@mattansb if you can recommend a quick fix or simply point out the reason test_check() doesn't work but test() does, I would be grateful

test_check("effectsize")
Error: No tests found for effectsize

...but they do run using devtools::test().

[mattansb]

Hey @trashbirdecology - sorry, but I'm not sure if you're done listing these or not?
(from you recent commit on the review issue, sounds like you're waiting for me?)

[trashbirdecology]

oh, sorry about that @mattansb. I would like to go back through after the suggested edits are resolved (or not).

[mattansb]

Okay great - then I'll start working on these and let you know when I'm done addressing these.

[mattansb]

I think I've addresses all the issues you've raised. You can find them in the master branch (or, if you'd prefer a comparison, you can take a look at this PR #202 ). Below are some direct comments to some of your points / questions.

Please let me know if anything is missing or unclear.

Thanks @trashbirdecology !

Required fixes for JOSS review

question/minor: should it be noted that it's currently available via CRAN? (not sure JOSS requirements or recommendations on this)

This wasn't required for our previous submissions, so if that's any indication, I think this is not necessary.

"returns an updated model," the function doesn't return an updated model, it provides updated coefficients, including Classes ‘effectsize_table’, ‘see_effectsize_table’, ‘effectsize_std_params’. I.e., the output of standardize_parameters() does not == the output of lm()

I have made the distinction between standardize() (which returns an updated model) and standardize_parameters() (which returns a table of parameters) more explicit in the example. Please let me know if this is now clearer.

suggest: "from a provided model" -- briefly list the available model types which standardize_paramters can handle

I've added a reference to the insight package, which is the backbone of the easystats ecosystem.

Dependency warnings on devtools::document()

All easystats packages are designed to be light-weight on dependencies, so when the packages are not available these fails are the intended behavior - asking users to only install any additional packages as they are needed. When running the tests with all the suggested packages installed, all tests pass (as is done in CRAN-checks).

FAILURE devtools::check() on vignette build @mattansb

This has been fixed - thanks for catching this!

JLB suggestions (not required for JOSS review)

feature request: as a potential user I would find it useful to a quick-reference table in the paper and/or readme that allows me to easily identify what my standardization, conversion, and interpretation options are.

I think these are too many to list concisely in a single table - but I've added a hyper link to https://easystats.github.io/effectsize/reference/index.html in both the paper and the readme ("a full list of features and functions").

Rename primary branch from 'master' to 'main' (or something else)

Great suggestion! I will definitely do this - when the review is completed, as the JOSS review is currently linked to the "master" branch.

JLB to do list

Check singularity warning when running following example in paper. I haven't yet looked into this, just noting here to remember to do so.

Oops, forgot to add a line that filters out any chickens with incomplete data (never thought I'd write that sentence...). Fixed.

Tests won't run using testthat::test_check() (@mattansb if you can recommend a quick fix or simply point out the reason test_check() doesn't work but test() does, I would be grateful

Seems like test_check() shouldn't be called directly? As per Hadely

[trashbirdecology]

Thanks for responding to some of my questions and reference to test_check note!
The only thing I haven't checked off is the first reference to @cohen1998statistical appears at the second instance of Cohen's d.

" such as Cohen's d "

This is minor, so I will go ahead and resolve this issue.

[trashbirdecology]

@mattansb Cohen's still isn't cited until the last reference to Cohen's d.

[mattansb]

@trashbirdecology How did I miss that??
Added (and a bunch more on the way!) - 0065daa