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

gigs: Assess Fetal, Newborn, and Child Growth with International Standards #626

Open
13 of 20 tasks
simpar1471 opened this issue Feb 7, 2024 · 49 comments
Open
13 of 20 tasks

Comments

@simpar1471
Copy link

simpar1471 commented Feb 7, 2024

Submitting Author Name: Simon Parker
Submitting Author Github Handle: @simpar1471
Repository: https://github.com/lshtm-gigs/gigs/
Version submitted: 0.4.1
Submission type: Stats
Badge grade: silver
Editor: @rkillick
Reviewers: @chitrams, @BroVic

Due date for @chitrams: 2024-09-20

Due date for @BroVic: 2024-09-24
Archive: TBD
Version accepted: TBD
Language: en

  • Paste the full DESCRIPTION file inside a code block below:
Type: Package
Package: gigs
Title: Assess Fetal, Newborn, and Child Growth with International Standards
Version: 0.4.1
Authors@R: c(
    person(given = "Simon", family = "Parker",
           email = "simon.parker@lshtm.ac.uk",
           role = c("aut", "cre"), comment = c(ORCID = "0009-0003-8214-4496")),
    person(given = "Linda", family = "Vesel", role = c("dtc"),
           comment = c(ORCID = "0000-0003-3753-4172")),
    person(given = "Eric", family = "Ohuma", role = c("dtc"),
           comment = c(ORCID = "0000-0002-3116-2593")),
    person(given = "Bill & Melinda Gates Foundation", role = c("fnd", "cph"))
    )
Description: Convert between anthropometric measures and z-scores/centiles in
    multiple growth standards, and classify fetal, newborn, and child growth
    accordingly. With a simple interface to growth standards from the World
    Health Organisation and International Fetal and Newborn Growth Consortium
    for the 21st Century, gigs makes growth assessment easy and reproducible for
    clinicians, researchers and policy-makers.
License: GPL (>= 3)
URL: https://github.com/lshtm-gigs/gigs/,
    https://lshtm-gigs.github.io/gigs/
BugReports: https://github.com/lshtm-gigs/gigs/issues
Depends: 
    R (>= 4.1.0)
Imports:
    stats,
    gamlss.dist,
    checkmate,
    vctrs
Suggests:
    knitr,
    rmarkdown,
    testthat (>= 3.0.0),
    units,
    withr
VignetteBuilder: knitr
Config/testthat/edition: 3
Config/testthat/parallel: true
Config/testthat/start-first: who_gs, ig_nbs, ig_fet, ig_png
Encoding: UTF-8
Language: en-GB
LazyData: true
RoxygenNote: 7.3.0
Roxygen: list(markdown = TRUE, roclets = c ("namespace", "rd", "srr::srr_stats_roclet"))

Scope

  • Please indicate which of our statistical package categories this package falls under. (Please check one appropriate box below):

    Statistical Packages

    • Bayesian and Monte Carlo Routines
    • Dimensionality Reduction, Clustering, and Unsupervised Learning
    • Machine Learning
    • Regression and Supervised Learning
    • Exploratory Data Analysis (EDA) and Summary Statistics
    • Spatial Analyses
    • Time Series Analyses

Pre-submission Inquiry

  • A pre-submission inquiry has been approved in issue#616

General Information

  • Who is the target audience and what are scientific applications of this package?

The target audience for this package is researchers, clinicians, and policy-makers interested in nutrition and fetal/newborn/child health. The package itself is used to convert measurements, for example a baby's weight at a given age, into summary statistics such as z-scores (i.e. number of standard deviations away from mean) and percentiles (i.e. number of percentage points above/below the median, expressed as a decimal). It can also be used to classify these z-scores and percentiles into specific categories for specific facets of growth. Scientific applications of this package would therefore include using our functions to generate statistical measures of growth in individuals or populations across time or pre/post a healthcare intervention. These could then be used in further downstream analyses.

Several R packages which implement child growth charts already exist, but they differ in the range of growth charts offered, their flexibility in conversion, and in what data they make available to users.

  1. anthro converts measurements into z-scores in the WHO Child Growth Standards, but lacks any INTERGROWTH-21st standards and outputs tabular data. We provide an interface which takes vectors in and gives vectors out, so is more flexible (e.g. with dplyr pipelines). This package is available on CRAN.
  2. childsds can convert measurements to z-scores or percentiles, but cannot convert z-scores/percentiles to expected measurements. Additionally, childsds does not contain the newborn/postnatal INTERGROWTH-21st growth standards, which we implement. Though childsds does include more growth references, growth references are not within the scope of the GIGS project. We can discuss the reasons for this if necessary. This package is available on CRAN.
  3. growthstandards contains functions for converting between values and z-scores/percentiles, as in gigs. It includes the INTERGROWTH-21st fetal standards, but not the newborn or postnatal standards we implement. This package makes coefficients available to end-users, but not reference growth curves. This package is not available on CRAN. It was last updated in January 2024.
  4. intergrowth provides more fetal growth standards than gigs, but cannot convert between z-scores/percentiles in the lacks the INTERGROWTH-21st newborn or postnatal growth standards, which we have implemented in gigs. This package also does not make coefficients for the growth standards available to end-users, though it it does provide growth curve data to end-users. This package is not available on CRAN, and was last updated in January 2023.
  5. gigs provides a simple interface for working with the growth standards it implements, which can be easily included in dplyr-like data wrangling pipelines. It implements all available WHO 2006/2007 and INTERGROWTH-21st growth standards, and makes published growth curves and model coefficients are available wherever possible. In our own testing, we found that it also warns end users about bad input data more comprehensively.

The Other packages section of the repository README also contains links to each of these packages, and a table describing features present in gigs compared to other growth standards packages.

We have a benchmarking vignette which shows the scaling of these packages when analysing datasets containing 1 to 100,000 cases. In summary: anthro scales the worst (29 ms to 2.24 seconds). The other packages are then clustered together: growthstandards is the fastest for 100,000 cases (4.62 ms to 125 ms); childsds is next (2.40 ms to 126 ms); then gigs (1.32 ms to 128 ms).

Not applicable.

Badging

Silver

  • Demonstrating excellence in compliance with multiple standards from at least two broad sub-categories. I feel this package has a comprehensive test suite and good error/warning behaviours in response to undesired input.

Technical checks

Confirm each of the following by checking the box.

This package:

Publication options

  • Do you intend for this package to go on CRAN?
  • Do you intend for this package to go on Bioconductor?

Code of conduct

@ropensci-review-bot
Copy link
Collaborator

Thanks for submitting to rOpenSci, our editors and @ropensci-review-bot will reply soon. Type @ropensci-review-bot help for help.

@ropensci-review-bot
Copy link
Collaborator

🚀

Editor check started

👋

@simpar1471 simpar1471 changed the title gigs: gigs: Assess Fetal, Newborn, and Child Growth with International Standards Feb 7, 2024
@ropensci-review-bot
Copy link
Collaborator

Checks for gigs (v0.4.1)

git hash: a9d9654a

  • ✔️ Package name is available
  • ✔️ has a 'codemeta.json' file.
  • ✔️ has a 'contributing' file.
  • ✔️ uses 'roxygen2'.
  • ✔️ 'DESCRIPTION' has a URL field.
  • ✔️ 'DESCRIPTION' has a BugReports field.
  • ✔️ Package has at least one HTML vignette
  • ✔️ All functions have examples.
  • ✔️ Package has continuous integration checks.
  • ✔️ Package coverage is 100%.
  • ✔️ R CMD check found no errors.
  • ✔️ R CMD check found no warnings.

Package License: GPL (>= 3)


1. rOpenSci Statistical Standards (srr package)

This package is in the following category:

  • Exploratory Data Analysis

✔️ All applicable standards [v0.2.0] have been documented in this package (632 complied with; 0 N/A standards)

Click to see the report of author-reported standards compliance of the package with links to associated lines of code, which can be re-generated locally by running the srr_report() function from within a local clone of the repository.


2. Package Dependencies

Details of Package Dependency Usage (click to open)

The table below tallies all function calls to all packages ('ncalls'), both internal (r-base + recommended, along with the package itself), and external (imported and suggested packages). 'NA' values indicate packages to which no identified calls to R functions could be found. Note that these results are generated by an automated code-tagging system which may not be entirely accurate.

type package ncalls
internal gigs 401
internal base 271
imports stats 36
imports checkmate 13
imports gamlss.dist 1
imports vctrs 1
suggests knitr NA
suggests rmarkdown NA
suggests testthat NA
suggests units NA
suggests withr NA
linking_to NA NA

Click below for tallies of functions used in each package. Locations of each call within this package may be generated locally by running 's <- pkgstats::pkgstats(<path/to/repo>)', and examining the 'external_calls' table.

gigs

validate_ig_fet (22), validate_ig_nbs (17), fn_on_subset (13), validate_who_gs (11), who_gs_lms2value (9), validate_ig_png (6), gigs_xaz_lgls (5), validate_yzp (5), ig_fet_doppler_gms (3), ig_fet_efw_lms (3), ig_fet_gwg_mu_sigma (3), ig_fet_mu_sigma (3), ig_nbs_bodycomp_mu_sigma (3), ig_nbs_msnt (3), ig_nbs_wlr_mu_sigma (3), ig_png_equations (3), ig_vpns_equations (3), mu_sigma_z2y (3), validate_parameter_lengths (3), who_gs_lms (3), classify_sfga (2), gigs_laz (2), gigs_option_get (2), gigs_waz (2), gigs_wlz (2), ig_nbs_hcfga_value2zscore (2), ig_nbs_lfga_value2zscore (2), ig_nbs_wfga_value2centile (2), ig_nbs_wfga_value2zscore (2), ig_png_hcfa_value2zscore (2), ig_png_lfa_value2zscore (2), ig_png_wfa_value2zscore (2), ig_png_wfl_value2zscore (2), inrange (2), validate_hcaz_params (2), validate_laz_params (2), validate_waz_params (2), validate_wlz_params (2), who_gs_hcfa_value2zscore (2), who_gs_lhfa_value2zscore (2), who_gs_wfa_value2zscore (2), classify_stunting (1), classify_svn (1), classify_wasting (1), classify_wfa (1), drop_null_elements (1), exponent (1), gigs_hcaz (1), gigs_option_set (1), handle_invalid_chr_options (1), handle_missing_data (1), handle_oob_centiles (1), handle_oob_xvar (1), handle_undefined_data (1), handle_var (1), hcfa_mu (1), hcfa_sigma (1), hcfga_mu (1), ig_fet_acfga_centile2value (1), ig_fet_acfga_value2centile (1), ig_fet_acfga_value2zscore (1), ig_fet_acfga_zscore2value (1), ig_fet_avfga_centile2value (1), ig_fet_avfga_value2centile (1), ig_fet_avfga_value2zscore (1), ig_fet_avfga_zscore2value (1), ig_fet_bpdfga_centile2value (1), ig_fet_bpdfga_value2centile (1), ig_fet_bpdfga_value2zscore (1), ig_fet_bpdfga_zscore2value (1), ig_fet_centile2value (1), ig_fet_cmfga_centile2value (1), ig_fet_cmfga_value2centile (1), ig_fet_cmfga_value2zscore (1), ig_fet_cmfga_zscore2value (1), ig_fet_crlfga_centile2value (1), ig_fet_crlfga_value2centile (1), ig_fet_crlfga_value2zscore (1), ig_fet_crlfga_zscore2value (1), ig_fet_doppler_y2z (1), ig_fet_doppler_z2y (1), ig_fet_efw_y2z (1), ig_fet_efw_z2y (1), ig_fet_efwfga_centile2value (1), ig_fet_efwfga_value2centile (1), ig_fet_efwfga_value2zscore (1), ig_fet_efwfga_zscore2value (1), ig_fet_estimate_fetal_weight (1), ig_fet_estimate_ga (1), ig_fet_estimate_ga_crl (1), ig_fet_estimate_ga_hc (1), ig_fet_estimate_ga_hcfl (1), ig_fet_flfga_centile2value (1), ig_fet_flfga_value2centile (1), ig_fet_flfga_value2zscore (1), ig_fet_flfga_zscore2value (1), ig_fet_gafcrl_centile2value (1), ig_fet_gafcrl_value2centile (1), ig_fet_gafcrl_value2zscore (1), ig_fet_gafcrl_zscore2value (1), ig_fet_gaftcd_centile2value (1), ig_fet_gaftcd_value2centile (1), ig_fet_gaftcd_value2zscore (1), ig_fet_gaftcd_zscore2value (1), ig_fet_gwg_y2z (1), ig_fet_gwg_z2y (1), ig_fet_gwgfga_centile2value (1), ig_fet_gwgfga_value2centile (1), ig_fet_gwgfga_value2zscore (1), ig_fet_gwgfga_zscore2value (1), ig_fet_hcfga_centile2value (1), ig_fet_hcfga_value2centile (1), ig_fet_hcfga_value2zscore (1), ig_fet_hcfga_zscore2value (1), ig_fet_mu_sigma_y2z (1), ig_fet_mu_sigma_z2y (1), ig_fet_ofdfga_centile2value (1), ig_fet_ofdfga_value2centile (1), ig_fet_ofdfga_value2zscore (1), ig_fet_ofdfga_zscore2value (1), ig_fet_pifga_centile2value (1), ig_fet_pifga_value2centile (1), ig_fet_pifga_value2zscore (1), ig_fet_pifga_zscore2value (1), ig_fet_poffga_centile2value (1), ig_fet_poffga_value2centile (1), ig_fet_poffga_value2zscore (1), ig_fet_poffga_zscore2value (1), ig_fet_pvfga_centile2value (1), ig_fet_pvfga_value2centile (1), ig_fet_pvfga_value2zscore (1), ig_fet_pvfga_zscore2value (1), ig_fet_rifga_centile2value (1), ig_fet_rifga_value2centile (1), ig_fet_rifga_value2zscore (1), ig_fet_rifga_zscore2value (1), ig_fet_sdrfga_centile2value (1), ig_fet_sdrfga_value2centile (1), ig_fet_sdrfga_value2zscore (1), ig_fet_sdrfga_zscore2value (1), ig_fet_sffga_centile2value (1), ig_fet_sffga_value2centile (1), ig_fet_sffga_value2zscore (1), ig_fet_sffga_zscore2value (1), ig_fet_sfhfga_centile2value (1), ig_fet_sfhfga_value2centile (1), ig_fet_sfhfga_value2zscore (1), ig_fet_sfhfga_zscore2value (1), ig_fet_tcdfga_centile2value (1), ig_fet_tcdfga_value2centile (1), ig_fet_tcdfga_value2zscore (1), ig_fet_tcdfga_zscore2value (1), ig_fet_v2z_internal (1), ig_fet_value2centile (1), ig_fet_value2zscore (1), ig_fet_z2v_internal (1), ig_fet_zscore2value (1), ig_nbs_bfpfga_centile2value (1), ig_nbs_bfpfga_value2centile (1), ig_nbs_bfpfga_value2zscore (1), ig_nbs_bfpfga_zscore2value (1), ig_nbs_bodycomp_p2v (1), ig_nbs_bodycomp_v2p (1), ig_nbs_c2v_internal (1), ig_nbs_centile2value (1), ig_nbs_ffmfga_centile2value (1), ig_nbs_ffmfga_value2centile (1), ig_nbs_ffmfga_value2zscore (1), ig_nbs_ffmfga_zscore2value (1), ig_nbs_fmfga_centile2value (1), ig_nbs_fmfga_value2centile (1), ig_nbs_fmfga_value2zscore (1), ig_nbs_fmfga_zscore2value (1), ig_nbs_hcfga_centile2value (1), ig_nbs_hcfga_value2centile (1), ig_nbs_hcfga_zscore2value (1), ig_nbs_lfga_centile2value (1), ig_nbs_lfga_value2centile (1), ig_nbs_lfga_zscore2value (1), ig_nbs_msnt_p2v (1), ig_nbs_msnt_v2p (1), ig_nbs_v2c_internal (1), ig_nbs_value2centile (1), ig_nbs_value2zscore (1), ig_nbs_wfga_centile2value (1), ig_nbs_wfga_zscore2value (1), ig_nbs_wlrfga_centile2value (1), ig_nbs_wlrfga_p2v (1), ig_nbs_wlrfga_v2p (1), ig_nbs_wlrfga_value2centile (1), ig_nbs_wlrfga_value2zscore (1), ig_nbs_wlrfga_zscore2value (1), ig_nbs_zscore2value (1), ig_png_centile2value (1), ig_png_hcfa_centile2value (1), ig_png_hcfa_value2centile (1), ig_png_hcfa_zscore2value (1), ig_png_lfa_centile2value (1), ig_png_lfa_value2centile (1), ig_png_lfa_zscore2value (1), ig_png_v2z_internal (1), ig_png_value2centile (1), ig_png_value2zscore (1), ig_png_wfa_centile2value (1), ig_png_wfa_value2centile (1), ig_png_wfa_zscore2value (1), ig_png_wfl_centile2value (1), ig_png_wfl_value2centile (1), ig_png_wfl_zscore2value (1), ig_png_z2v_internal (1), ig_png_zscore2value (1), ig_vpns_value2zscore (1), ig_vpns_zscore2value (1), lfa_log_mu (1), lfa_sigma (1), lfga_mu (1), msg_invalid_sex_acronym (1), msg_missing_data (1), msg_oob_centiles (1), msg_oob_xvar (1), msg_undefined_data (1), mu_sigma_y2z (1), remove_attributes (1), retrieve_coefficients (1), validate_acronym (1), validate_chr (1), validate_ig_fet_estimation_param (1), validate_ig_fet_weight_estimation_param (1), validate_numeric (1), validate_sex (1), validate_xvar (1), wfa_log_mu (1), wfa_sigma (1), wfga_logmu (1), wfl_mu (1), wfl_sigma (1), who_gs_acfa_centile2value (1), who_gs_acfa_value2centile (1), who_gs_acfa_value2zscore (1), who_gs_acfa_zscore2value (1), who_gs_bfa_centile2value (1), who_gs_bfa_value2centile (1), who_gs_bfa_value2zscore (1), who_gs_bfa_zscore2value (1), who_gs_centile2value (1), who_gs_hcfa_centile2value (1), who_gs_hcfa_value2centile (1), who_gs_hcfa_zscore2value (1), who_gs_lhfa_centile2value (1), who_gs_lhfa_value2centile (1), who_gs_lhfa_zscore2value (1), who_gs_lms_v2z (1), who_gs_lms_v2z_constrained (1), who_gs_lms_v2z_over_three (1), who_gs_lms_v2z_under_minus_three (1), who_gs_lms_z2v (1), who_gs_lms_z2v_over_three (1), who_gs_lms_z2v_under_minus_three (1), who_gs_ssfa_centile2value (1), who_gs_ssfa_value2centile (1), who_gs_ssfa_value2zscore (1), who_gs_ssfa_zscore2value (1), who_gs_tsfa_centile2value (1), who_gs_tsfa_value2centile (1), who_gs_tsfa_value2zscore (1), who_gs_tsfa_zscore2value (1), who_gs_v2z_internal (1), who_gs_value2centile (1), who_gs_value2zscore (1), who_gs_wfa_centile2value (1), who_gs_wfa_value2centile (1), who_gs_wfa_zscore2value (1), who_gs_wfh_centile2value (1), who_gs_wfh_value2centile (1), who_gs_wfh_value2zscore (1), who_gs_wfl_value2zscore (1)

base

list (63), length (32), is.null (18), c (15), rep (12), log (9), is.na (8), names (8), for (7), ifelse (6), gamma (5), lengths (5), sqrt (5), data.frame (4), logical (4), matrix (4), paste0 (4), unique (4), abs (3), character (3), get (3), if (3), lapply (3), ncol (3), nrow (3), numeric (3), vapply (3), as.data.frame (2), levels (2), options (2), parent.frame (2), rep_len (2), sum (2), with (2), deparse (1), emptyenv (1), exp (1), inherits (1), max (1), mode (1), new.env (1), paste (1), q (1), range (1), rep.int (1), replace (1), seq_along (1), sign (1), substitute (1), unlist (1), vector (1)

stats

sigma (28), complete.cases (4), var (2), approx (1), qnorm (1)

checkmate

qassert (8), assert_numeric (2), assert_subset (2), assert_character (1)

gamlss.dist

pST3C (1)

vctrs

vec_recycle_common (1)


3. Statistical Properties

This package features some noteworthy statistical properties which may need to be clarified by a handling editor prior to progressing.

Details of statistical properties (click to open)

The package has:

  • code in R (100% in 15 files) and
  • 1 authors
  • 3 vignettes
  • 7 internal data files
  • 4 imported packages
  • 185 exported functions (median 6 lines of code)
  • 376 non-exported functions in R (median 8 lines of code)

Statistical properties of package structure as distributional percentiles in relation to all current CRAN packages
The following terminology is used:

  • loc = "Lines of Code"
  • fn = "function"
  • exp/not_exp = exported / not exported

All parameters are explained as tooltips in the locally-rendered HTML version of this report generated by the checks_to_markdown() function

The final measure (fn_call_network_size) is the total number of calls between functions (in R), or more abstract relationships between code objects in other languages. Values are flagged as "noteworthy" when they lie in the upper or lower 5th percentile.

measure value percentile noteworthy
files_R 15 73.0
files_vignettes 3 92.4
files_tests 10 90.7
loc_R 2689 89.0
loc_vignettes 877 89.0
loc_tests 4157 97.6 TRUE
num_vignettes 3 94.2
data_size_total 1093128 95.5 TRUE
data_size_median 11317 79.8
n_fns_r 561 98.0 TRUE
n_fns_r_exported 185 98.3 TRUE
n_fns_r_not_exported 376 97.5 TRUE
n_fns_per_file_r 21 95.9 TRUE
num_params_per_fn 3 33.6
loc_per_fn_r 6 12.1
loc_per_fn_r_exp 6 10.5
loc_per_fn_r_not_exp 8 22.6
rel_whitespace_R 13 82.6
rel_whitespace_vignettes 13 67.8
rel_whitespace_tests 12 94.8
doclines_per_fn_exp 163 97.4 TRUE
doclines_per_fn_not_exp 0 0.0 TRUE
fn_call_network_size 408 94.2

3a. Network visualisation

Click to see the interactive network visualisation of calls between objects in package


4. goodpractice and other checks

Details of goodpractice checks (click to open)

3a. Continuous Integration Badges

R-CMD-check.yaml
pkgcheck
test-coverage.yaml

GitHub Workflow Results

id name conclusion sha run_number date
7815041175 pages build and deployment success ea59a9 20 2024-02-07
7815003364 pkgcheck success a9d965 5 2024-02-07
7815003368 pkgdown success a9d965 45 2024-02-07
7814822521 R-CMD-check success a9d965 51 2024-02-07
7814822525 test-coverage success a9d965 51 2024-02-07
7815003373 Update CITATION.cff success a9d965 2 2024-02-07

3b. goodpractice results

R CMD check with rcmdcheck

rcmdcheck found no errors, warnings, or notes

Test coverage with covr

Package coverage: 100

Cyclocomplexity with cyclocomp

No functions have cyclocomplexity >= 15

Static code analyses with lintr

lintr found the following 69 potential issues:

message number of times
Avoid library() and require() calls in packages 2
Lines should not be more than 80 characters. 67


Package Versions

package version
pkgstats 0.1.3.9
pkgcheck 0.1.2.14
srr 0.0.1.194


Editor-in-Chief Instructions:

This package is in top shape and may be passed on to a handling editor

@ldecicco-USGS
Copy link

@ropensci-review-bot check srr

@ropensci-review-bot
Copy link
Collaborator

'srr' standards compliance:

  • Complied with: 53 / 102 = 52% (eda: 8 / 34; general: 45 / 68)
  • Not complied with: 49 / 102 = 48% (eda: 26 / 34; general: 23 / 68)

✔️ This package complies with > 50% of all standads and may be submitted.

@rkillick
Copy link

@ropensci-review-bot assign @rkillick as editor

@ropensci-review-bot
Copy link
Collaborator

Assigned! @rkillick is now the editor

@simpar1471
Copy link
Author

Hi @rkillick, have you had any chance to look at this? Not meaning to be pushy if you're rained in with other things at the moment!

@ldecicco-USGS
Copy link

Sorry about this! I think I missed an internal discussion about reassigning editors. I'll take over and get some reviewers assigned!

@ldecicco-USGS
Copy link

@ropensci-review-bot assign @ldecicco-USGS as editor

@ropensci-review-bot
Copy link
Collaborator

Assigned! @ldecicco-USGS is now the editor

@ldecicco-USGS
Copy link

@ropensci-review-bot add @chitrams to reviewers

@ropensci-review-bot
Copy link
Collaborator

@chitrams added to the reviewers list. Review due date is 2024-09-20. Thanks @chitrams for accepting to review! Please refer to our reviewer guide.

rOpenSci’s community is our best asset. We aim for reviews to be open, non-adversarial, and focused on improving software quality. Be respectful and kind! See our reviewers guide and code of conduct for more.

@ropensci-review-bot
Copy link
Collaborator

@chitrams: If you haven't done so, please fill this form for us to update our reviewers records.

@ldecicco-USGS
Copy link

@ropensci-review-bot add @BroVic to reviewers

@ropensci-review-bot
Copy link
Collaborator

@BroVic added to the reviewers list. Review due date is 2024-09-24. Thanks @BroVic for accepting to review! Please refer to our reviewer guide.

rOpenSci’s community is our best asset. We aim for reviews to be open, non-adversarial, and focused on improving software quality. Be respectful and kind! See our reviewers guide and code of conduct for more.

@ropensci-review-bot
Copy link
Collaborator

@BroVic: If you haven't done so, please fill this form for us to update our reviewers records.

@ldecicco-USGS
Copy link

@ropensci-review-bot assign @rkillick as editor

@ropensci-review-bot
Copy link
Collaborator

Assigned! @rkillick is now the editor

@chitrams
Copy link

Thank you @rkillick I really appreciate it!

@ropensci-review-bot
Copy link
Collaborator

📆 @BroVic you have 2 days left before the due date for your review (2024-09-24).

@BroVic
Copy link

BroVic commented Sep 24, 2024

Hello @rkillick I would also like to request an extension till Monday September 30. As a first-time reviewer, I really want to take my time to review the documentation, coupled with some pressure at my day job. I hope this is accepted. Cheers :)

@rkillick
Copy link

@BroVic thanks for letting me know your review will be late. Receiving your review by Monday 30 September is fine. If you need any general advice as a first-time reviewer, feel free to reach out privately (r.killick@lancaster.ac.uk).

@BroVic
Copy link

BroVic commented Sep 25, 2024

Much appreciated @rkillick . I will holler if I run into any obstacles.

@chitrams
Copy link

chitrams commented Sep 26, 2024

Package Review

  • Briefly describe any working relationship you may have (had) with the package authors (or otherwise remove this statement)

  • 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).

With the following point:

The potential reviewer/editor has significantly contributed to a competitor project.

I would like to mention that I have worked on developing the WHO {anthro} package in 2019. However, I do not believe I have a conflict of interest as (1) I am not an active developer of {anthro}; and (2) I am excited to see the development of {gigs} for the field of childhood malnutrition generally.


Compliance with Standards

  • This package complies with a sufficient number of standards for a silver badge
  • This grade of badge is the same as what the authors wanted to achieve
  • This package extends beyond minimal compliance with standards in the following ways: (please describe)

Documentation and testing are excellent for this package, fulfilling many sub-categories: for example 6.1.1 Documentation, 6.1.5 Testing, 6.3.1 Documentation, 6.3.4 Return results/output data (results are consistent with input type), 6.3.6 Testing.


General 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
  • Community guidelines including contribution guidelines in the README or CONTRIBUTING

Perhaps you can add a CONTRIBUTING document to specify how people could contribute? I know you've added a Code of Conduct at the bottom of your README, but this additional CONTRIBUTING document could be helpful.

  • The documentation is sufficient to enable general use of the package beyond one specific use case

Algorithms

I think these algorithms are well-encoded. Conversions I tested were fine.

Testing

Tests seem good to me!

Package Design

Package design is good for its intended purpose. I have some suggestions that might make this package easier to use below. Feel free to implement these changes or not :)

  • I wonder if it would be annoying for users to type the standard before the conversion?
    • As an example, there are three beginning with ig_. Perhaps it would be better to name the functions to start with nbs_ig, png_ig, and fet_ig instead?
  • In the package's current state, you have to type the whole standard out before the conversion. I think as a user, I would be thinking about the conversion first, for example: value2zscore, value2centile, zscore2value, and centile2value, then the standard name can be an option in the function call instead?
    • Then as a user when I use Tab to auto-complete, this could be more convenient.
    • The function names as they currently are also just leads to unwieldy function names–they are quite long.
    • My suggestion would be a bit of work, as you would need to restructure the function organisation, but I believe would make it easier for the user to use these functions.
  • Similar to the point above. Might it be easier to have the function names shorter as val2z, z2val, and val2centile instead? This point might be a bit overkill though.

Other Comments

A few other minor suggestions:

  • Potential issue within README.md: exclamation mark emoji (❕) is white, this may be hard to read on a white background.
  • Have you considered submitting your software to Zenodo after? It's an open data repository operated by CERN. Submitting your code to Zenodo means you'll get a persistent digital object identifier (DOI), which means your code will be more easily cited in academic contexts. Here are some instructions on getting started.

In summary: thank you @simpar1471 for submitting this package, it's fabulous! I can see that a lot of work has gone into this. Congratulations and well done for putting this together.


  • Packaging guidelines: The package conforms to the rOpenSci packaging guidelines

Estimated hours spent reviewing: 7

  • Should the author(s) deem it appropriate, I agree to be acknowledged as a package reviewer ("rev" role) in the package DESCRIPTION file.

@simpar1471
Copy link
Author

Thank you for your review @chitrams! With respect to your comments:

  1. Package design
    • We like your idea of refactoring the functions to make them less unwieldy. This is something we've heard from several others who've looked at the package. I think it's probably time to rip the plaster off.
    • I'm reluctant to rename the INTERGROWTH-21st standard acronyms from "ig_nbs" to "nbs_ig" etc., if they're going to be parameters in overall value2centile()/value2zscore() etc. functions - mostly to save on extra dev time, as I've moved from the role which was employing me full-time to build the package out. Is that alright if we're implementing your refactoring suggestions? So the interface would look something like this:
    value2zscore(y = weight_kg , x = age_days, sex = sex, family = "ig_nbs", acronym = "wfga")
    
  2. Other comments
    • We'll replace the exclamation mark emoji - for now either with the red exclamation mark (❗) or warning sign (⚠️).
    • We weren't considering Zenodo, but we will now that you've suggested it! We'll sort a DOI out after we've addressed your comments.
  3. Other things we should note
    • In the dev branch for gigs we have a range of new functions which apply specific growth standards for infants depending on their gestational + chronological ages. These new functions will be included alongside the refactors you've suggested. This branch also has a CONTRIBUTING.md file.
    • We've had some issues with autotest recently, which are detailed below. Can we resubmit whilst autotest is failing?

Thank you again for taking the time to review the package, we really appreciate you giving the time for this.

@rkillick
Copy link

@simpar1471 thanks for the fast turnaround with your response to the first review. I would encourage you to wait for the second review to come in over the next few days before embarking on major changes. Just incase you get some conflicting comments you might want to rationalise.

@BroVic
Copy link

BroVic commented Sep 30, 2024

Package Review

  • 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).

Functionality

  • Installation: Installation succeeds as documented.
  • Functionality: Any functional claims of the software have been confirmed.
  • Performance: Any performance claims of the software have been confirmed.
  • Automated tests: Unit tests cover essential functions of the package and a reasonable range of inputs and conditions. All tests pass on the local machine.
  • Packaging guidelines: The package conforms to the rOpenSci packaging guidelines.

Estimated hours spent reviewing: 12

  • ☒ Should the author(s) deem it appropriate, I agree to be acknowledged as a package reviewer (“rev” role) in the package DESCRIPTION file.

Review Comments

First, allow me to thank the author and other contributors/funders for this package, as it will be very useful to health and other researchers and policymakers. The following are my comments/suggestions for further improvement:

  1. There are some typographical errors in the documentation (READMe.md):
    i). At the beginning of para 2, it should read “gigs is to be of use…”.
    ii). Component standards for INTERGROWTH-21^st Newborn Size, first row of field Description should read “weight-for-gestational age”.
    iii). Section Conversion functions > Terminology currently reads “The package operates with on measured anthropometric values”. Use one preposition instead.

  2. Accessibility to non-expert users: The README assumes that the user is intimately familiar with the subject. I suggest an introductory paragraph that would appeal to a non-expert (maybe even lay) audience. Consider it a gentle introduction :)

  3. The author has provided links to related functions within a help file, but I would suggest adding more. For example, the help file for classify_sfga might benefit from links to other classify_* functions. This will ease navigation for the user. Alternatively, where these functions share enough similarity in their signature, a combined help file would be easier for users, as was done for, say, the ig_fet_* functions. Key differences can then be highlighted in the Details section.

  4. The benchmarking of {gigs} with other packages/software is highly commendable. In the benchmarking for running time, please indicate which of the gigs is for R and Stata.

  5. It seems to me that {gigs} is very useful as an intermediary in the data analysis pipeline. So, I have some concerns about the issue of unused factor levels when it comes to the classify_* functions. The main purpose is to classify based on the available data (as can be seen with the use of the severe parameter), but what if a user wants to represent unused factor levels downstream? This is a design decision of the author, but even if the outputs remain unchanged, I would recommend a clear mention in the help file

  6. Looking through NEWS I noted that API changes were made via PATCH version. Following the Semantic Versioning guideline, I would recommend that such changes be made, at minimum, a MINOR version update. Then, seeing that the author has made changes to parameter names before, I would suggest the deprecation of lenht_cm (for length/height) in favour of just len_cm, in the future. This could help naive users who are not that familiar with the correct terminology.

  7. In the file data.R, change the @format tag element for describing the fields of the dataset from itemize to describe (line 264); this will remove the R CMD check NOTE.

  8. Consider adding {ggsci} and {microbenchmark} to Suggests - required for building the Benchmarking gigs against other software packages website article; and {cowplot} and {viridis} for NTERGROWTH-21st Body Composition Equations.

  9. Fix the broken link pointing to https://simpar1471.github.io/gigs/ as specified in _pkgdown.yml.

  10. Provide alt-text for images the README and vignette generation in the R chunks via the fig.alt chunk option (see vignette(pkgdown::accessibility)).

  11. goodpractice:gp() complains about some lines of code that were longer than 80 characters. Although none was longer than 82 characters, the maintainer might consider refactoring the code so as to adhere to the recommended limit.

@rkillick
Copy link

Thanks @chitrams and @BroVic for your thoughtful reviews. @simpar1471 now over to you to consider your reply and and changes you want to make.

@chitrams
Copy link

chitrams commented Oct 2, 2024

Hi @simpar1471, I will reply point-by-point:

  • We like your idea of refactoring the functions to make them less unwieldy. This is something we've heard from several others who've looked at the package. I think it's probably time to rip the plaster off.
  • I'm reluctant to rename the INTERGROWTH-21st standard acronyms from "ig_nbs" to "nbs_ig" etc., if they're going to be parameters in overall value2centile()/value2zscore() etc. functions - mostly to save on extra dev time, as I've moved from the role which was employing me full-time to build the package out. Is that alright if we're implementing your refactoring suggestions? So the interface would look something like this:
value2zscore(y = weight_kg , x = age_days, sex = sex, family = "ig_nbs", acronym = "wfga")

Yep, that is more than fair enough! If the standard acronyms are already a function parameter, I think that would be more than sufficient for usability :)

  • We'll replace the exclamation mark emoji - for now either with the red exclamation mark (❗) or warning sign (⚠️).

Fantastic! I'd suggest the yellow warning sign as you already have an existing red emoji for the cross. Thank you for taking this into consideration!

  • We weren't considering Zenodo, but we will now that you've suggested it! We'll sort a DOI out after we've addressed your comments.

Great! Feel free to ask me if you have any questions about this process.

  • In the dev branch for gigs we have a range of new functions which apply specific growth standards for infants depending on their gestational + chronological ages. These new functions will be included alongside the refactors you've suggested. This branch also has a CONTRIBUTING.md file.

Perfect! I had a quick look and this looks great to me. Quick question, I was wondering why the CONTRIBUTING.md file is in the .github folder?

Not a problem at all, I'm more than happy to help review gigs. Again, great work! :)

@chitrams
Copy link

chitrams commented Oct 2, 2024

Hi @rkillick, could you help with @simpar1471's question from his comment above please? I don't know the answer to this one. Thank you! 😄

  • We've had some issues with autotest recently, which are detailed below. Can we resubmit whilst autotest is failing?

@rkillick
Copy link

rkillick commented Oct 2, 2024

@simpar1471 I'm not up to date with the autotest issues, maybe @ldecicco-USGS knows more as EiC?
My personal opinion is that if automated things are not working, this should not preclude resubmission. As with CRAN checks, if there is a result that needs explaining then a comment on this should be sufficient.

@mpadge
Copy link
Member

mpadge commented Oct 3, 2024

@chitrams @simpar1471 autotest is still only "recommended" and not "required", so you are free to ignore any issues there, and proceed regardless. Sorry that I haven't yet responded to your issue in the autotest repo 😄

@simpar1471
Copy link
Author

@BroVic Thanks you the comprehensive review, especially the pointers to issues in the docs + DESCRIPTION. These will be fixed in the next version. Do you have an ORCID we can insert in DESCRIPTION for your reviewer role?

@chitrams On the CONTRIBUTING.md front - I can't quite recall why it's buried there! I think I copied it from the dplyr. Would you recommend I put it somewhere else?

@mpadge No worries! I'll keep chugging on for now in that case.

I'm going to make the changes to the API as discussed above. @rkillick What should I do when I feel I have a version ready to be looked over?

@BroVic
Copy link

BroVic commented Oct 3, 2024

@simpar1471

Here's my ORCID - 0000-0003-3716-0668. Thanks for your consideration.

@rkillick
Copy link

rkillick commented Oct 3, 2024

@simpar1471 when you have a new version you want looking over, pop back here and provide the usual "response to reviewers" summary of changes/responses and tag in the reviewers (and me) again.

@rkillick
Copy link

rkillick commented Oct 3, 2024

@simpar1471 if something comes up as you are implementing things, you are still welcome to comment here to chat with myself and the reviewers about it.

@chitrams
Copy link

chitrams commented Oct 4, 2024

@simpar1471 in response to your comment:

On the CONTRIBUTING.md front - I can't quite recall why it's buried there! I think I copied it from the dplyr. Would you recommend I put it somewhere else?

I would recommend having it in a docs/ directory or in the root directory, although opinions vary and this is probably a matter of personal preference 😆 The rOpenSci dev guide recommends having it in .github/ or docs/.

My reasoning for not having it in .github/ is: I believe it would be more versatile in case a user does not use GitHub. In that instance, a reasonable place someone would look for this documentation is in a docs/ dir or in the root dir.

Cheers,
Chitra

@simpar1471
Copy link
Author

simpar1471 commented Nov 9, 2024

Hi folks! It's been a bit of while since the changes you asked for required some pretty hefty refactoring of the source code + tests, but I've updated the package to reflect your comments, and further updated it to have some nicer formatting in warnings/errors with cli. This is all currently in the dev version, but I’ll bump the minor version once we the changes are reviewed.

General response

Package design

The package has been refactored considerably, with growth standard agnostic value2score(), value2centile(), centile2value() and zscore2value() functions from which users pick a growth standard with family and acronym parameters. For example, the INTERGROWTH-21st newborn size standard for weight-for-gestational age is now accessed with value2zscore(y = ..., x = ..., sex = ..., family = "ig_nbs", acronym = "wfga"). This was updated in commit lshtm-gigs/gigs@2e20ac3.

Response to @chitrams

Documentation

  • Community guidelines including contribution guidelines in the README or CONTRIBUTING
  • Response: Added in lshtm-gigs/gigs@c02b7f1

Other comments

  • Potential issue within README.md: exclamation mark emoji (❕) is white, this may be hard to read on a white background.
  • Response: Changed to a warning symbol (⚠) in lshtm-gigs/gigs@5c0dbb3

Response to @BroVic

Review comments

  1. README issues
    Response: The README has been updated to not have the misakes you pointed out.

  2. Accessibility to non-expert users
    Response: I've added some more context in commit lshtm-gigs/gigs@15fa1c1 - is this gentle enough, or should I change the tone further?

  3. Help file linking
    Response: Added some more links in lshtm-gigs/gigs@3ec893e. Are more still needed?

  4. Benchmarking
    Response: The benchmarking file has been updated generally; is the new format okay? This is mostly in commit lshtm-gigs/gigs@752c21f, but you can see the results best on the website.

  5. Unused factor levels in classify_()/compute_()/categorise_*() outputs.
    Response: I have added @notes specifying that unused factor levels will be present in classify_()/compute_()/categorise_*() outputs (see commit lshtm-gigs/gigs@3ec893e). Are these sufficient?

  6. Semantic versioning considerations
    Response: Acknowledged + I'll make sure these are at least a MINOR version bump in the future. As for the lenht_cm parameter is specifically used because length and height in anthropometry are measured differently, and we want users to know they can use height values here too.

  7. Change \itemize to \describe
    Response: Done in commit lshtm-gigs/gigs@c02b7f1

  8. Adding items to Suggest in DESCRIPTION
    Response: Done in commit lshtm-gigs/gigs@c451a25

  9. Fix the broken link pointing to https://simpar1471.github.io/gigs/ as specified in _pkgdown.yml.
    Response: Done recently, contained the PR merge at lshtm-gigs/gigs@752c21f

  10. Provide alt-text for images the README and vignette generation...
    Response: Done recently, contained the PR merge at lshtm-gigs/gigs@752c21f

  11. Edit all lines of code down to <80 characters based on goodpractice output
    Response: I've not done this given the breadth of other changes required to get the package ready to submit. Some lines of the roxygen documentation are also >80 characters because I'm writing out Markdown tables, which I don't think I can make any narrower.

@chitrams
Copy link

chitrams commented Nov 9, 2024

@simpar1471 fantastic work refactoring the functions to be standard-agnostic! I'll have a go testing these in the next week or so, I'm looking forward to trying out the changes :) Just from quickly glancing at the changes, it must have been a lot of work.

Also, well done for the excellent documentation in responding to our suggestions and comments! I really appreciate how well-organised your comments and responses are :)

@BroVic
Copy link

BroVic commented Nov 10, 2024

@simpar1471

Thanks for your detailed report on the changes made! And I think the change made for would-be non-expert users is adequate, as long as they RTFM 😊. A suggestion for future versions just came to mind - detect unused factor levels and signal a warning.

Cheers!

@simpar1471
Copy link
Author

@chitrams Thanks for the quick feedback!

@BroVic I'll add in something for warning users about unused factor levels - should have it implemented soon.

@simpar1471
Copy link
Author

simpar1471 commented Nov 11, 2024

@BroVic I've added a package-level option for handling unused factor levels when doing growth outcome classification. By default users will be warned about unused factor levels, but they'll remain the in the outputs. They can change this using gigs_option_set() to make it so that the package stops warning them, or starts dropping the unused levels. This is all in commits lshtm-gigs/gigs@35c606d to lshtm-gigs/gigs@96beb45; with corresponding updates to test suite/docs/website included. How are these?

@BroVic
Copy link

BroVic commented Nov 17, 2024

@simpar1471, this is a great addition to the package. Your effort is highly commendable.

Best.

@rkillick
Copy link

Just to check where we are on this one. @BroVic are you happy with all the changes and responses to your comments? @chitrams you said you haven't had chance to review the changes yet. Is this correct?

@BroVic
Copy link

BroVic commented Nov 18, 2024

@rkillick yes, I'm happy with the changes made. Thanks!

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

No branches or pull requests

7 participants