search

ropensci / software-review

rOpenSci Software Peer Review.
295 stars 104 forks source link

GLMMcosinor: Fit a cosinor model using the glmmTMB framework. #603

Closed RWParsons closed 10 months ago

RWParsons commented 1 year ago

Date accepted: 2024-01-09

Submitting Author Name: Rex Parsons Submitting Author Github Handle: !--author1-->@RWParsons<!--end-author1-- Other Package Authors Github handles: @oliverjayasinghe, @nicolemwhite Repository: https://github.com/RWParsons/GLMMcosinor Version submitted: 0.1.0 Submission type: Stats Badge grade: silver Editor: !--editor-->@Paula-Moraga<!--end-editor-- Reviewers: @sachsmc, @jcavieresg

Archive: TBD Version accepted: TBD Language: en

Type: Package
Package: GLMMcosinor
Title: Fit a cosinor model using a generalised mixed modelling framework
Version: 0.1.0
Authors@R: c(
    person("Rex", "Parsons", , "rex.parsons94@gmail.com", role = c("aut", "cre"),
           comment = c(ORCID = "0000-0002-6053-8174")),
    person("Oliver", "Jayasinghe", role = "aut"),
    person("Nicole", "White", role = "aut",
           comment = c(ORCID = "0000-0002-9292-0773"))
  )
Description: Fit the cosinor model using the glmmTMB framework, allowing
    for a wide range of link functions and capacity of heirachical
    structures.
License: GPL (>= 3)
URL: https://github.com/RWParsons/GLMMcosinor,
    https://rwparsons.github.io/GLMMcosinor/
BugReports: https://github.com/RWParsons/GLMMcosinor/issues
Imports:
    assertthat,
    cowplot,
    ggforce,
    ggplot2,
    glmmTMB,
    lme4,
    rlang,
    scales,
    stats
Suggests:
    cosinor,
    covr,
    dplyr,
    DT,
    flextable,
    ftExtra,
    knitr,
    rmarkdown,
    testthat (>= 3.0.0),
    vdiffr,
    withr
VignetteBuilder: 
    knitr
Config/testthat/edition: 3
Encoding: UTF-8
LazyData: true
Roxygen: list(markdown = TRUE, roclets = c ("namespace", "rd",
    "srr::srr_stats_roclet"))
RoxygenNote: 7.2.3

Scope

Pre-submission Inquiry

General Information

People analysing rhythmic/circular data - for example, circadian biologists.

This is the first implementation of a cosinor modelling package which can handle generalised models (link functions) in R. There are other packages in python but these are limited to count-data related families. Similarly, there are very limited other packages in R that can handle a hierarchical structure or have helpful plotting methods for the model objects. This package is based on the {cosinor} R package but that is limited to linear models. A summary of existing software is given in a table in the README.

NA

Badging

Technical checks

Confirm each of the following by checking the box.

This package:

Publication options

Code of conduct

ropensci-review-bot commented 1 year ago

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

ropensci-review-bot commented 1 year ago

:rocket:

The following problem was found in your submission template:

:wave:

ropensci-review-bot commented 1 year ago

Checks for GLMMcosinor (v0.1.0)

git hash: c4251092

(Checks marked with :eyes: may be optionally addressed.)

Package License: GPL (>= 3)

1. rOpenSci Statistical Standards (srr package)

This package is in the following category:

:heavy_check_mark: All applicable standards [v0.2.0] have been documented in this package (296 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 |base | 311| |internal |GLMMcosinor | 28| |internal |utils | 7| |imports |stats | 57| |imports |ggplot2 | 14| |imports |glmmTMB | 5| |imports |scales | 3| |imports |rlang | 2| |imports |assertthat | 1| |imports |cowplot | 1| |imports |lme4 | 1| |imports |ggforce | NA| |suggests |cosinor | NA| |suggests |covr | NA| |suggests |dplyr | NA| |suggests |DT | NA| |suggests |flextable | NA| |suggests |ftExtra | NA| |suggests |knitr | NA| |suggests |rmarkdown | NA| |suggests |testthat | NA| |suggests |vdiffr | 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()', and examining the 'external_calls' table.

base

c (31), max (18), list (17), for (16), names (15), length (14), paste0 (14), paste (12), unlist (12), matrix (7), nrow (7), round (7), which (7), data.frame (6), F (6), lapply (6), attr (5), rep (5), sqrt (5), structure (5), cos (4), diag (4), dim (4), eval (4), min (4), pi (4), sin (4), abs (3), as.data.frame (3), cbind (3), class (3), gsub (3), match.call (3), mean (3), missing (3), ncol (3), substitute (3), t (3), args (2), grep (2), seq (2), seq_along (2), with (2), all.vars (1), array (1), as.character (1), as.factor (1), atan2 (1), col (1), colnames (1), deparse (1), deparse1 (1), drop (1), ifelse (1), labels (1), levels (1), rbind (1), regmatches (1), return (1), rownames (1), seq_len (1), signif (1), solve (1), stopifnot (1), str2lang (1), sum (1), summary (1), tan (1)

stats

formula (11), offset (6), qnorm (6), df (5), family (4), terms (4), coefficients (3), time (3), vcov (3), pchisq (2), runif (2), as.formula (1), coef (1), end (1), pnorm (1), predict (1), sd (1), start (1), var (1)

GLMMcosinor

get_new_coefs (4), amp_acro (3), sub_summary.cosinor.glmm (3), update_formula_and_data (3), amp_acro_iteration (2), cosinor.glmm (2), data_processor_plot (2), formula_eval (2), get_varnames (2), autoplot.cosinor.glmm (1), check_group_var (1), data_processor (1), fit_model_and_process (1), summary.cosinor.glmm (1)

ggplot2

element_blank (7), aes (3), ggplot (2), facet_grid (1), vars (1)

utils

data (7)

glmmTMB

fixef (4), glmmTMB (1)

scales

breaks_pretty (3)

rlang

sym (2)

assertthat

is.number (1)

cowplot

plot_grid (1)

lme4

findbars (1)

**NOTE:** Some imported packages appear to have no associated function calls; please ensure with author that these 'Imports' are listed appropriately.

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 14 files) and - 3 authors - 6 vignettes - 1 internal data file - 9 imported packages - 16 exported functions (median 27 lines of code) - 45 non-exported functions in R (median 40 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](https://docs.ropensci.org/pkgcheck/reference/checks_to_markdown.html) 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 | 14| 70.8| | |files_vignettes | 7| 98.5| | |files_tests | 9| 89.6| | |loc_R | 1990| 84.1| | |loc_vignettes | 1610| 95.9|TRUE | |loc_tests | 1493| 91.6| | |num_vignettes | 6| 98.7|TRUE | |data_size_total | 2990| 64.7| | |data_size_median | 2990| 71.3| | |n_fns_r | 61| 62.9| | |n_fns_r_exported | 16| 60.6| | |n_fns_r_not_exported | 45| 64.7| | |n_fns_per_file_r | 3| 52.5| | |num_params_per_fn | 6| 77.4| | |loc_per_fn_r | 34| 80.7| | |loc_per_fn_r_exp | 27| 58.8| | |loc_per_fn_r_not_exp | 40| 86.1| | |rel_whitespace_R | 15| 80.3| | |rel_whitespace_vignettes | 19| 92.4| | |rel_whitespace_tests | 9| 78.9| | |doclines_per_fn_exp | 54| 67.1| | |doclines_per_fn_not_exp | 0| 0.0|TRUE | |fn_call_network_size | 35| 58.7| | ---

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](https://github.com/RWParsons/GLMMcosinor/actions/workflows/R-CMD-check.yaml/badge.svg)](https://github.com/RWParsons/GLMMcosinor/actions) **GitHub Workflow Results** | id|name |conclusion |sha | run_number|date | |----------:|:--------------------------|:----------|:------|----------:|:----------| | 5947357214|pages build and deployment |success |6204b8 | 99|2023-08-23 | | 5947315446|pkgdown |success |c42510 | 115|2023-08-23 | | 5947315449|R-CMD-check |success |c42510 | 151|2023-08-23 | | 5947315448|test-coverage |success |c42510 | 120|2023-08-23 | --- #### 3b. `goodpractice` results #### `R CMD check` with [rcmdcheck](https://r-lib.github.io/rcmdcheck/) rcmdcheck found no errors, warnings, or notes #### Test coverage with [covr](https://covr.r-lib.org/) Package coverage: 91.83 #### Cyclocomplexity with [cyclocomp](https://github.com/MangoTheCat/cyclocomp) The following functions have cyclocomplexity >= 15: function | cyclocomplexity --- | --- autoplot.cosinor.glmm | 34 polar_plot.cosinor.glmm | 32 summary.cosinor.glmm | 15 #### Static code analyses with [lintr](https://github.com/jimhester/lintr) [lintr](https://github.com/jimhester/lintr) found the following 10 potential issues: message | number of times --- | --- Avoid library() and require() calls in packages | 10

5. Other Checks

Details of other checks (click to open)

:heavy_multiplication_x: The following function name is duplicated in other packages: - - `simulate_cosinor` from cosinor

Package Versions

|package |version | |:--------|:---------| |pkgstats |0.1.3.7 | |pkgcheck |0.1.2.1 | |srr |0.0.1.192 |

Editor-in-Chief Instructions:

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

noamross commented 1 year ago

@ropensci-review-bot assign @Paula-Moraga as editor

ropensci-review-bot commented 1 year ago

Assigned! @Paula-Moraga is now the editor

Paula-Moraga commented 1 year ago

@ropensci-review-bot seeking reviewers

ropensci-review-bot commented 1 year ago

Please add this badge to the README of your package repository:

[![Status at rOpenSci Software Peer Review](https://badges.ropensci.org/603_status.svg)](https://github.com/ropensci/software-review/issues/603)

Furthermore, if your package does not have a NEWS.md file yet, please create one to capture the changes made during the review process. See https://devguide.ropensci.org/releasing.html#news

Paula-Moraga commented 1 year ago

@ropensci-review-bot assign @sachsmc as reviewer

ropensci-review-bot commented 1 year ago

@sachsmc added to the reviewers list. Review due date is 2023-10-05. Thanks @sachsmc 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 commented 1 year ago

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

ropensci-review-bot commented 1 year ago

:calendar: @sachsmc you have 2 days left before the due date for your review (2023-10-05).

sachsmc commented 1 year ago

Package Review

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

Documentation

The package includes all the following forms of documentation:

Functionality

Estimated hours spent reviewing: 4

Review Comments

Paula-Moraga commented 1 year ago

Many thanks @sachsmc for your time and very thoughtful review!

Paula-Moraga commented 1 year ago

@ropensci-review-bot submit review https://github.com/ropensci/software-review/issues/603#issuecomment-1744900580 time 4

ropensci-review-bot commented 1 year ago

Logged review for sachsmc (hours: 4)

RWParsons commented 1 year ago

Thank you very much @sachsmc for the positive feedback and helpful comments! We will work to address them over the coming couple weeks but I'd just like to clarify how you would picture the use of the proposed feature for the predict function?

Are you suggesting that the user pass the inputs and the output would be a visualised rhythm?

say something like:

predict(MESOR = 5, amplitude= 2, acrophase = pi, period = 12) # or passed as a named vector or some other way so that it plays nicely with the stats::predict() generic

And it would spit out something similar to simulate_cosinor() but as if there were no noise term? I think I'm struggling to distinguish how this would be very different from simulate_cosinor() so it likely that I'm not understanding how you intend for it to work.

Thanks!

sachsmc commented 1 year ago

No, not a visualized rhythm, but more like a summary statistic of the rhythm for a vector of covariate values. So the main arguments would be the newdata, but then you could add a type = "amplitude", or "mesor", for example, which would give the predicted parameter for the subpopulation with those covariate values. For simple models, those parameters can be obtained directly from the print method, but this predict approach would be useful for complex models with continuous covariates, for example. I had the predict methods from the survival package in mind when I made the suggestion.

Come to think of it, is it true that it is not possible to specify a model such that the amplitude and acrophase vary continuously in the covariate? Is there a need for such a model or would that be too biologically implausible?

Anyway these are just minor suggestions, so if this is to complex or there is no need for such things, feel free to ignore.

RWParsons commented 1 year ago

Oh right!

You're right, it's currently not possible to specify a model such that the amplitude/acrophase can vary over time. It can only vary in relation the grouping levels but that's provided to the user when using print(model).

I've given some thought to the type of model that you're suggesting before though. circacompare allows the user to specify a decay term on amplitude or acrophase so this feature may be useful there, but it's only possible because it uses nonlinear regression instead of the cosinor model. Otherwise, I think a cosinor model would need to be adjusted to have an interaction term between time (or possibly a nonlinear spline on time) with the cosinor components (rrr and sss). This could sort of allow the confidence ellipse of a rhythm "move" over time and be could be evaluated at any given time-point. (I was thinking to do this for evaluating how fast an animal adjusts to a phase shift but never pursued it.)

This feature might be out of scope for this initial version but I'll keep in mind for perhaps a future development of the package. I think it'd probably require some adjustments to amp_acro() to allow the user to specify an interaction term with time and that'd have some flow on effects to the summary/print/autplot/polar_plot methods.

Paula-Moraga commented 1 year ago

@ropensci-review-bot assign @jcavieresg as reviewer

ropensci-review-bot commented 1 year ago

@jcavieresg added to the reviewers list. Review due date is 2023-10-31. Thanks @jcavieresg 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 commented 1 year ago

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

Paula-Moraga commented 1 year ago

@ropensci-review-bot set due date for @jcavieresg to 2023-11-30

ropensci-review-bot commented 1 year ago

Review due date for @jcavieresg is now 30-November-2023

ropensci-review-bot commented 1 year ago

@RWParsons, @oliverjayasinghe, @nicolemwhite: please post your response with @ropensci-review-bot submit response <url to issue comment> if you haven't done so already (this is an automatic reminder).

Here's the author guide for response. https://devguide.ropensci.org/authors-guide.html

jcavieresg commented 1 year ago

title: "Review package GLMMcosinor" output: rmarkdown::md_document: pandoc_args: [ "--wrap=none" ]

Package Review

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

Documentation

The package includes all the following forms of documentation:

Functionality

Estimated hours spent reviewing: 5

Review Comments

Suggestions

Minor suggestions

Paula-Moraga commented 1 year ago

Many thanks for your time and review, @jcavieresg!

Paula-Moraga commented 1 year ago

@ropensci-review-bot submit review https://github.com/ropensci/software-review/issues/603#issuecomment-1820575066 time 5

ropensci-review-bot commented 1 year ago

Logged review for jcavieresg (hours: 5)

Paula-Moraga commented 1 year ago

Many thanks again @sachsmc and @jcavieresg for your time and effort reviewing the package!

@RWParsons, @oliverjayasinghe, @nicolemwhite, please consider updating the package by incorporating the comments made by the reviewers. Looking forward to seeing the new version!

ropensci-review-bot commented 12 months ago

@RWParsons, @oliverjayasinghe, @nicolemwhite: please post your response with @ropensci-review-bot submit response <url to issue comment> if you haven't done so already (this is an automatic reminder).

Here's the author guide for response. https://devguide.ropensci.org/authors-guide.html

RWParsons commented 11 months ago

Thanks @Paula-Moraga and thank you @sachsmc and @jcavieresg for your helpful and constructive reviews of the package.

I made an issues on the GLMMcosinor repo with the suggestions from each reviewer.

@jcavieresg's review issue: https://github.com/RWParsons/GLMMcosinor/issues/4

@sachsmc's review issue: https://github.com/RWParsons/GLMMcosinor/issues/2

In each of these issues, I have left a comment on how we addressed each suggestion (and checked them off as they were completed).

These changes are currently on the dev branch. I'll merge to main once it's approved in case there are more suggestions.

RWParsons commented 11 months ago

@ropensci-review-bot submit response https://github.com/ropensci/software-review/issues/603#issuecomment-1856982320

ropensci-review-bot commented 11 months ago

Logged author response!

RWParsons commented 11 months ago

Hi @Paula-Moraga

Just checking in to check that the ball isn't still in my court. I've responded to the reviewer comments and made the changes to the dev branch of the repo and done the submit response thing. Should the label be changed to '5/awaiting reviewers response' now or notify the reviewers to check the changes/responses?

Thanks!

Paula-Moraga commented 10 months ago

Thanks, @RWParsons!

@sachsmc and @jcavieresg, can you please check if you are satisfied with the changes and if you have additional comments? Thanks!

sachsmc commented 10 months ago

I am satisfied with the changes and response.

Best, Michael

On Mon, Jan 8, 2024, 12:51 Paula Moraga @.***> wrote:

Thanks, @RWParsons https://github.com/RWParsons!

@sachsmc https://github.com/sachsmc and @jcavieresg https://github.com/jcavieresg, can you please check if you are satisfied with the changes and if you have additional comments? Thanks!

— Reply to this email directly, view it on GitHub https://github.com/ropensci/software-review/issues/603#issuecomment-1880855222, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABJWA6LS5S5UVNPJEH7QKYTYNPMVLAVCNFSM6AAAAAA324XNRSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOBQHA2TKMRSGI . You are receiving this because you were mentioned.Message ID: @.***>

jcavieresg commented 10 months ago

Sorry! I missed the previous response of @RWParsons

Considering the answers of the author, all my comments/observations were solved. Best wishes,

RWParsons commented 10 months ago

Thanks for your quick responses and helpful reviews, @sachsmc and @jcavieresg!!

Paula-Moraga commented 10 months ago

Great! Thanks so much @sachsmc and @jcavieresg for your time and effort. I am very pleased to approve the package. Well done and congratulations, @RWParsons!

Paula-Moraga commented 10 months ago

@ropensci-review-bot approve GLMMcosinor

ropensci-review-bot commented 10 months ago

Approved! Thanks @RWParsons for submitting and @sachsmc, @jcavieresg for your reviews! :grin:

To-dos:

Should you want to acknowledge your reviewers in your package DESCRIPTION, you can do so by making them "rev"-type contributors in the Authors@R field (with their consent).

Welcome aboard! We'd love to host a post about your package - either a short introduction to it with an example for a technical audience or a longer post with some narrative about its development or something you learned, and an example of its use for a broader readership. If you are interested, consult the blog guide, and tag @ropensci/blog-editors in your reply. They will get in touch about timing and can answer any questions.

We maintain an online book with our best practice and tips, this chapter starts the 3d section that's about guidance for after onboarding (with advice on releases, package marketing, GitHub grooming); the guide also feature CRAN gotchas. Please tell us what could be improved.

Last but not least, you can volunteer as a reviewer via filling a short form.

RWParsons commented 10 months ago

@ropensci-review-bot finalize transfer of GLMMcosinor

ropensci-review-bot commented 10 months ago

Transfer completed. The GLMMcosinor team is now owner of the repository and the author has been invited to the team