tasks for DOCUMENTATION

[ernestguevarra]

Please check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide

• Briefly describe any working relationship you have (had) with the package authors.
• As the reviewer I confirm that there are no conflicts of interest for me to review this work (if you are unsure whether you are in conflict, please speak to your editor before starting your review).

Documentation

The package includes all the following forms of documentation:

• A statement of need: clearly stating problems the software is designed to solve and its target audience in README
• Installation instructions: for the development version of package and any non-standard dependencies in README
• Vignette(s): demonstrating major functionality that runs successfully locally
• Function Documentation: for all exported functions
• Examples: (that run successfully locally) for all exported functions
• Community guidelines: including contribution guidelines in the README or CONTRIBUTING, and DESCRIPTION with URL, BugReports and Maintainer (which may be autogenerated via Authors@R).

---

Review Comments

On statement of need

Package has a README that details why the software was developed and what needs it fills/addresses. However, the background on why the tool was developed (IPC AMN) and the rules (SMART) and tooling (nipnTK) the package is built on is not clearly expounded. I think it is important to state here that the package being reviewed primarily uses already existing functionalities from another package (nipnTK) and what the current package primarily does is provide a wrapper to these functions to implement checks based on SMART rules and how IPC performs its data quality checks. As such, the package name {ipccheckr} is a bit misleading as the checks performed are not really guidelines set by IPC but rather an implementation of rules set by SMART and using functions/tools provided by {nipnTK}. Hence, a more appropriate name should signify/indicate such. A name that alludes to the SMART origins of the rules might be more appropriate (smartchecks) or a name that indicates that this is primarily a tool for reporting on anthropometric data - so maybe something like anthroporter short for anthropometric reporter. The point of the comment basically is that the name is not compatible with the purpose stated and the problems that are being addressed.

In terms of coherence of software purpose to actual functionality, why add functions for prevalence estimation when this is meant to be a checker? I think this is an inconsistency and prevalence estimation is out of scope.

In relation to the statement of need are specific feedback conerning the README text and the README code/implementation:

• in the Rmd setup code chunk, you used devtools::install_github("tomaszaba/ipccheckr"). You shouldn't add code like this on any script especially in an Rmd. The decision to install should be the option of the user. Locally, you should install the package using devtools::install() before knitting the README.

• Too much use of specialist jargon and acronyms/abbreviations (e.g., IPC AMN, plausibility checks) and use of technical shorthand (e.g., 6:23 instead of 6 to 23 months, etc.).

• Need to check grammar and sentence construction. A lot of sentences are too long and sometimes incoherent.

• Workflow using DiagrammeR is clever but the diagram doesn't really look nice/good; alignment is off; using grViz requires/forces you to specify always_allow_html: true which then messes up github markdown such that your callout-tip is now working properly. I suggest using mermaid instead and using mermaid syntax directly rather than wrapping it in R using DiagrammeR.

• the hex sticker is too low; the recommended is to align it to the main heading.

• add badge for repostatus (set to WIP - work in progress) and badge for lifecycle (set to experimental)

• CSS style for callout-tip is showing in the HTML output. This shouldn't be the case. The HTML implementation is getting confused with your use of the grViz from DiagrammeR.

• instructions on how to install should be somewhere at the start right after the what does ipccheckr do.

• use remotes::install_github() rather than devtools::install_github() - remotes uses a lot less dependency than devtools; in Windows, installing devtools require installing developer tools which will be much complicated for regular R users.

• There is no contributing guidelines and there is no community guidelines. These are important.

Action points for README:

• README: remove installation code in R chunk in README.Rmd #2
• README: copy-edit README text; simplify and make more coherent #3
• README: move hex sticker to header for title #4
• README: add badge for repostatus - set to WIP #5
• README: add badge for lifecycle - set to experimental #6
• README: shift diagram to mermaid graph #7
• README: fix callout-tip CSS style notation #8
• README: remove always_allow_html: true in YAML #9
• README: move installation instructions earlier in the README #10
• README: use remotes rather than devtools in the installation instructions #11
• README: add contributing guidelines #12
• README: add code of conduct/community guidelines #13

Vignettes

• like the README, vignettes are too wordy and a bit incoherent
• when setting css: style.css in yaml, you need to create a style.css file as well
• doesn't make sense to put a dynamic date in vignettes - date: "r format(Sys.Date(), '%B %d, %Y')" - even if you didn't change the vignette, everytime you deploy the package to pkgdown for every new R-CMD-check, the vignette will be re-rendered and with a new date will seem like it has been updated. Either set a fixed date and the change when you edit/change vignette or don't put a date at all.

Action points for vignettes:

• VIGNETTES: copy-edit text in all vignettes; simplify and make more coherent #14
• VIGNETTES: in plausibility.Rmd, remove css: style.css in yaml or create a style.css file #15
• VIGNETTES: remove date or add a fixed date #16

Function documentation

• function documentation has descriptions that are too long!! Descriptions should be more succinct and simple. If added notation is needed/wanted, then this should be added as detail and with sections that you can refer to. These notes get added after the documentation about the parameters. This is more readable in that users can see syntax first/right away before any detail. Right now, the user is bombarded with very long text even before he/she can see the syntax.

• the roxygen notation is very chaotic and not human-readable. I prefer that each argument has its own @param line rather than combinations. I find this much harder to read as a user when looking at help files.

• the roxygen notation for example code is very chaotic. Treat this in the same way as if you were coding as a script.

Action points for function documentation:

• ROXYGEN: shorten @description and add details in @detail and in @section #17
• ROXYGEN: use one @param for every argument #18
• ROXYGEN: make use of @Rdname to group functions into commeon Roxygen #19

Other documentation feedback/action points

• on devtools::check() I see:

   < Potential spelling errors:
   <   WORD           FOUND IN
   < Bassa          prevalence.Rmd:225
   < Cahora         prevalence.Rmd:225
   < Chiuta         prevalence.Rmd:225
   < Maravia        prevalence.Rmd:183,225
   < Metuge         prevalence.Rmd:183,225
   < Oftentimes     README.md:30
   < R's            plausiblity.Rmd:27
   <                sample_size.Rmd:37
   < ipc            sample_size.Rmd:67
   < obs            sample_size.Rmd:67
   < plausibile     prevalence.Rmd:102
   < requeriments   sample_size.Rmd:2
   < zscorer        plausiblity.Rmd:110
   < ️            README.md:117
   < If these are false positive, run `spelling::update_wordlist()`.All Done!

I see about 3 of these words needing spelling correction

• SPELLING: check spelling as per spell_check() #20

[tomaszaba]

Now that all functions have been revised, including their documentation, I think we can close this issue as completed.