search

ropensci / software-review

rOpenSci Software Peer Review.
286 stars 104 forks source link

rangr: Species range dynamics simulation toolset #595

Closed katarzynam-165 closed 5 months ago

katarzynam-165 commented 1 year ago

Date accepted: 2024-01-21 Submitting Author Name: Katarzyna Markowska Submitting Author Github Handle: !--author1-->@katarzynam-165<!--end-author1-- Other Package Authors Github handles: !--author-others-->@LechoslawKuczynski<!--end-author-others-- Repository: https://github.com/popecol/rangr Version submitted: 1.0.0 Submission type: Stats Badge grade: silver Editor: !--editor-->@adamhsparks<!--end-editor-- Reviewers: taddallas, @taddallas, @TheAnalyticalEdge

Due date for taddallas: 2023-08-16 Archive: TBD Version accepted: TBD Language: en --- - Paste the full DESCRIPTION file inside a code block below: ``` Package: rangr Type: Package Title: Mechanistic Simulation of Species Range Dynamics Version: 1.0.0 Authors@R: c( person("Katarzyna", "Markowska", email = "katarzyna.markowska@amu.edu.pl", role = c("aut", "cre")), person("Lechosław", "Kuczyński", email = "lechu@amu.edu.pl", role = "aut")) Description: Species range dynamics simulation toolset. License: MIT + file LICENSE Imports: methods, parallel, pbapply, grDevices, graphics, stats, utils, zoo, terra, raster, assertthat Suggests: knitr, rmarkdown, testthat (>= 3.0.0), covr, bookdown VignetteBuilder: knitr Encoding: UTF-8 LazyData: true RoxygenNote: 7.2.3 Roxygen: list (markdown = TRUE, roclets = c ("namespace", "rd", "srr::srr_stats_roclet")) Depends: R (>= 3.5.0) Config/testthat/edition: 3 URL: https://github.com/popecol/rangr, https://popecol.github.io/rangr/ BugReports: https://github.com/popecol/rangr/issues ``` ## Scope - Please indicate which of our [statistical package categories](https://stats-devguide.ropensci.org/overview.html#overview-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 - [x] Spatial Analyses - [ ] Time Series Analyses ## Pre-submission Inquiry - [x] A pre-submission inquiry has been approved in [issue#\<592\>](#%3C592%3E) ## General Information - Who is the target audience and what are scientific applications of this package? rangr is an R package designed for simulating species range dynamics, primarily aimed at ecologists and conservationists who work with complex data structures such as those derived from citizen science and wildlife monitoring programs. With rangr, users can mimic the key processes that shape population numbers and spatial distributions, including local dynamics, dispersal, and habitat selection, to project population responses to environmental changes. Additionally, rangr can be used to test and evaluate different methods of modelling species distribution using simulated data as a reference. - Paste your responses to our [*General Standard* **G1.1** here](https://stats-devguide.ropensci.org/standards.html#general-standards), describing whether your software is: - *The first implementation of a novel algorithm*; or - *The first implementation within* **R** *of an algorithm which has previously been implemented in other languages or contexts*; or - *An improvement on other implementations of similar algorithms in* **R**. Please include hyperlinked references to all other relevant software. rangr is the first implementation of a novel algorithm, but there are a few packages like [RangeShiftR](https://github.com/RangeShifter/RangeShiftR-package), [poems](https://github.com/GlobalEcologyLab/poems), or [steps](https://github.com/steps-dev/steps) that serve similar purposes. However, none of them met all the criteria that were important to us in this type of simulation, such as being easy to set up and customize with other existing R functions, supporting simulations that vary in both time and space, and incorporating the Virtual Ecologist approach by providing functions for various sampling scenarios. - (If applicable) Does your package comply with our [guidance around *Ethics, Data Privacy and Human Subjects Research*](https://devguide.ropensci.org/policies.html#ethics-data-privacy-and-human-subjects-research)? Not applicable ## Badging - What grade of badge are you aiming for? ([bronze, silver, gold](https://stats-devguide.ropensci.org/pkgdev.html#pkgdev-badges)) Silver - If aiming for silver or gold, describe which of the [four aspects listed in the *Guide for Authors* chapter](https://stats-devguide.ropensci.org/pkgdev.html#pkgdev-silver) the package fulfils (at least one aspect for silver; three for gold) > Have a demonstrated *generality* of usage beyond one single envisioned use case. Software is frequently developed for one particular use case envisioned by the authors themselves. Generalising the utility of software so that it is readily applicable to other use cases, and satisfactorily documenting such generality of usage, represents another aspect which may be considered sufficient for software to attain a silver grade. This aspect is particularly well-suited due to the versatile range of applications offered by rangr. These applications include: - modelling population range dynamics, - testing various ecological scenarios, - testing species distribution modelling methods. ## Technical checks Confirm each of the following by checking the box. - [x] I have read the [rOpenSci packaging guide](https://devguide.ropensci.org/building.html). - [x] I have read the [author guide](https://devdevguide.netlify.app/authors-guide.html) and I expect to maintain this package for at least 2 years or have another maintainer identified. - [x] I/we have read the [*Statistical Software Peer Review* Guide for Authors](https://stats-devguide.ropensci.org/pkgdev.html). - [x] I/we have run [`autotest`](https://github.com/ropensci-review-tools/autotest) checks on the package, and ensured no tests fail. - [x] The [`srr_stats_pre_submit()` function](https://ropensci-review-tools.github.io/srr/reference/srr_stats_pre_submit.html) confirms this package may be submitted. - [x] The [`pkgcheck()` function](https://docs.ropensci.org/pkgcheck/reference/pkgcheck.html) confirms this package may be submitted - alternatively, please explain reasons for any checks which your package is unable to pass. This package: - [x] does not violate the Terms of Service of any service it interacts with. - [x] has a CRAN and OSI accepted license. - [x] contains a [README with instructions for installing the development version](https://devguide.ropensci.org/building.html#readme). ## Publication options - [x] Do you intend for this package to go on CRAN? - [ ] Do you intend for this package to go on Bioconductor? ## Code of conduct - [x] I agree to abide by [rOpenSci's Code of Conduct](https://ropensci.org/code-of-conduct/) during the review process and in maintaining my package should it be accepted.
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.

maelle commented 1 year ago

@ropensci-review-bot check package

ropensci-review-bot commented 1 year ago

Thanks, about to send the query.

ropensci-review-bot commented 1 year ago

:rocket:

Editor check started

:wave:

ropensci-review-bot commented 1 year ago

Checks for rangr (v1.0.0)

git hash: 80ec30bc

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

Package License: MIT + file LICENSE

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 (314 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 | 157| |internal |rangr | 49| |imports |assertthat | 63| |imports |terra | 32| |imports |stats | 16| |imports |utils | 7| |imports |graphics | 5| |imports |parallel | 2| |imports |raster | 2| |imports |methods | 1| |imports |pbapply | 1| |imports |zoo | 1| |imports |grDevices | NA| |suggests |knitr | NA| |suggests |rmarkdown | NA| |suggests |testthat | NA| |suggests |covr | NA| |suggests |bookdown | 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

list (12), names (8), t (8), as.matrix (7), nrow (7), cbind (6), data.frame (6), lapply (6), length (6), ncol (6), return (6), unlist (6), c (4), call (4), dim (4), for (4), if (4), is.na (4), rep (4), summary (4), as.numeric (3), array (2), as.character (2), ifelse (2), is.null (2), match (2), match.call (2), match.fun (2), matrix (2), min (2), paste0 (2), range (2), round (2), seq_len (2), switch (2), any (1), apply (1), do.call (1), max (1), mean (1), rbind (1), sqrt (1), subset (1), tabulate (1), which (1)

assertthat

assert_that (63)

rangr

kernel (6), dynamics (4), disp (2), dist_list (2), dists_tab (2), extinction_check (2), get_initialise_call (2), get_K (2), get_observations_random (2), K_check (2), K_interpolate (2), calc_dist (1), exponential (1), get_observations (1), get_observations_from_data (1), get_observations_monitoring_based (1), gompertz (1), initialise (1), K_get_init_values (1), K_get_interpolation (1), K_n1_map_check (1), ncell_in_circle (1), one_dist_sq_disp (1), plot.sim_results (1), print.sim_data (1), print.sim_results (1), print.summary.sim_data (1), print.summary.sim_results (1), ricker (1), target_ids_in_disp (1), tfun (1), to_rast (1)

terra

values (9), app (3), nlyr (3), rast (3), crs (2), ncell (2), res (2), ext (1), extract (1), spatSample (1), vect (1), xmax (1), xmin (1), ymax (1), ymin (1)

stats

time (13), dist (1), getCall (1), rgeom (1)

utils

data (5), txtProgressBar (2)

graphics

points (5)

parallel

parLapply (1), parLapplyLB (1)

raster

distanceFromPoints (2)

methods

formalArgs (1)

pbapply

pblapply (1)

zoo

na.approx (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 17 files) and - 2 authors - 1 vignette - 1 internal data file - 11 imported packages - 19 exported functions (median 18 lines of code) - 58 non-exported functions in R (median 19 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 | 17| 76.7| | |files_vignettes | 1| 68.4| | |files_tests | 11| 91.7| | |loc_R | 939| 66.4| | |loc_vignettes | 239| 55.8| | |loc_tests | 1163| 88.9| | |num_vignettes | 1| 64.8| | |data_size_total | 2155| 63.2| | |data_size_median | 2155| 69.1| | |n_fns_r | 77| 69.5| | |n_fns_r_exported | 19| 65.9| | |n_fns_r_not_exported | 58| 71.5| | |n_fns_per_file_r | 3| 49.1| | |num_params_per_fn | 3| 33.6| | |loc_per_fn_r | 19| 57.3| | |loc_per_fn_r_exp | 18| 42.5| | |loc_per_fn_r_not_exp | 20| 62.4| | |rel_whitespace_R | 42| 85.2| | |rel_whitespace_vignettes | 51| 71.2| | |rel_whitespace_tests | 21| 87.9| | |doclines_per_fn_exp | 43| 54.1| | |doclines_per_fn_not_exp | 0| 0.0|TRUE | |fn_call_network_size | 21| 47.5| | ---

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/popecol/rangr/actions/workflows/R-CMD-check.yaml/badge.svg)](https://github.com/popecol/rangr/actions) **GitHub Workflow Results** | id|name |conclusion |sha | run_number|date | |----------:|:--------------------------|:----------|:------|----------:|:----------| | 5411847096|pages build and deployment |success |becf6f | 12|2023-06-29 | | 5411814922|pkgcheck |success |80ec30 | 2|2023-06-29 | | 5411814921|pkgdown |success |80ec30 | 11|2023-06-29 | | 5411814924|R-CMD-check |success |80ec30 | 12|2023-06-29 | | 5411814918|test-coverage |success |80ec30 | 12|2023-06-29 | --- #### 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: 81.27 #### Cyclocomplexity with [cyclocomp](https://github.com/MangoTheCat/cyclocomp) The following function have cyclocomplexity >= 15: function | cyclocomplexity --- | --- sim | 15 #### Static code analyses with [lintr](https://github.com/jimhester/lintr) [lintr](https://github.com/jimhester/lintr) found the following 25 potential issues: message | number of times --- | --- Avoid library() and require() calls in packages | 5 Lines should not be more than 80 characters. | 20

5. Other Checks

Details of other checks (click to open)

:heavy_multiplication_x: The following 8 function names are duplicated in other packages: - - `disp` from OrdMonReg, pracma, psymonitor, REAT, reporttools, Rfit - - `exponential` from bayesforecast, brms, DoseFinding, greta, mcp, MCPMod, multinma, phaseR, rstanarm, verification, VGAM - - `get_observations` from CAISEr, frostr, pharmr - - `gompertz` from drc, growthmodels, IBMPopSim, MortalityLaws, pomp, PVAClone, VGAM - - `initialise` from cicerone, rDataPipeline, VIM - - `initialize` from adana, jti, RSeed, simplegraphdb, SubpathwayLNCE - - `ricker` from metafolio, MQMF, nlraa, pomp, PVAClone - - `sim` from antitrust, aqp, arm, attrib, bidask, bidask, bliss, cacIRT, did, DTAT, foieGras, gets, goric, irtoys, itsmr, lava, mclust, netCoin, ph2rand, psych, psych, relSim, RHMS, robustETM, runexp, simsem, spMC, sysid, tfarima, trade, tscopula, WRSS

Package Versions

|package |version | |:--------|:---------| |pkgstats |0.1.3.4 | |pkgcheck |0.1.1.26 | |srr |0.0.1.192 |

Editor-in-Chief Instructions:

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

maelle commented 1 year ago

@ropensci-review-bot assign @adamhsparks as editor

ropensci-review-bot commented 1 year ago

Assigned! @adamhsparks is now the editor

maelle commented 1 year ago

@katarzynam-165 thank you for your submission!

adamhsparks commented 12 months ago

Hi @katarzynam-165, apologies, I've been rather busy with an app release and project proposal at work. I'm now getting started with taking a look at this package and will update you as I go along and I'll find some reviewers for you.

adamhsparks commented 12 months ago

Editor checks:

Editor comments

  1. Currently, I get the following NOTE when running devtools::check(), which should be corrected. This can be done by adding these files to .Rbuildignore.
❯ checking top-level files ... NOTE
  Non-standard files/directories found at top level:
    ‘LICENSE.md’ ‘README.Rmd’ ‘codecov.yml’ ‘codemeta.json’

  1. For the install instructions, I'd remove the "#" and not have it commented out in the README. If you don't want that code chunk to run when knitting, use eval=FALSE. This would clarify the installation instructions and match the commonly accepted standard on GitHub for installing R packages using {remotes}/{devtools}, et al..

  2. A Code of Conduct statement, e.g., CODE_OF_CONDUCT.md, would be a welcome addition.

  3. It would help new users to orient themselves if the "Reference" page were grouped by topic, e.g., https://docs.ropensci.org/weathercan/reference/index.html. See https://pkgdown.r-lib.org/articles/pkgdown.html#reference for documentation on this feature.

ropensci-review-bot commented 11 months ago

'srr' standards compliance:

:heavy_multiplication_x: This package complies with < 50% of all standads and is not ready to be submitted.

adamhsparks commented 11 months ago

@katarzynam-165, can you address my editor comments and get the ‘srr’ standards compliance up? Perhaps there is an easy standard that you can add or meet?

katarzynam-165 commented 11 months ago

@adamhsparks thanks for the feedback! I will make the requested changes and review the NA standards - I'm pretty sure I can meet or add at least one (I was so close the first time :( ). I'll get back to you soon.

katarzynam-165 commented 11 months ago

Hi @adamhsparks! I have made the requested changes and would like to thank you especially for the helpful comment regarding the "Reference" page. It looks much clearer now. Additionally, I have implemented some standards that should improve compliance with 'srr' standards. If there are any other improvements that you think should be made to the package, please let me know :)

adamhsparks commented 11 months ago

Thanks, @katarzynam-165, I'll have a look at this later today or tomorrow!

adamhsparks commented 11 months ago

@ropensci-review-bot check srr

ropensci-review-bot commented 11 months ago

'srr' standards compliance:

:heavy_check_mark: This package complies with > 50% of all standads and may be submitted.

adamhsparks commented 11 months ago

Thank you, @katarzynam-165. You're right, the reference page is now much more easily read and understood. I appreciate you taking the time to address all of these issues that I raised and see it passes SRR checks now.

I'll start looking for reviewers now.

adamhsparks commented 11 months ago

@ropensci-review-bot seeking reviewers

ropensci-review-bot commented 11 months ago

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

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

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

adamhsparks commented 11 months ago

@ropensci-review-bot assign taddallas

ropensci-review-bot commented 11 months ago

I'm sorry human, I don't understand that. You can see what commands I support by typing:

@ropensci-review-bot help

adamhsparks commented 11 months ago

@ropensci-review-bot assign taddallas as reviewer

ropensci-review-bot commented 11 months ago

taddallas added to the reviewers list. Review due date is 2023-08-16. Thanks taddallas 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 11 months ago

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

adamhsparks commented 11 months ago

@ropensci-review-bot assign @taddallas as reviewer

ropensci-review-bot commented 11 months ago

@taddallas added to the reviewers list. Review due date is 2023-08-16. Thanks @taddallas 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 11 months ago

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

taddallas commented 11 months 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: 3-4

Review Comments

I get a warning when loading the sp package (it is a dependency of raster). Have the authors developed a plan for handling this?

The legacy packages maptools, rgdal, and rgeos, underpinning the sp package, which was just loaded, will retire in October 2023.

Consider adding the Code_of_Conduct.md to .Rbuildignore (it gets detected as a non-standard file when checking the package using devtools).

Does the initalize function have default values for some of the arguments like dens_dep and border? It seems like ‘absorbing’ is the default for border and ‘K2N’ is default for dens_dep, but this should maybe be noted in the documentation?

A small note, but the authors argue that r_sd is demographic stochasticity, but it is a time-varying growth rate applied to all cells equally (this may or may not be right). This seems like a measure of environmental stochasticity (e.g., there are good years and bad years). Perhaps future development of the package could include spatial variation in growth rates? Covariation between r and K (or the variance therein)? One way the authors could do this is to have the r argument take any number of forms (a raster the same size as n, a single value, etc.). It’s tough to do the change through time bits, as the initialize step sets up so much of the hyperparameters, but the simulation is done by sim. Perhaps this is not a real problem, and time and space -varying growth rates or carrying capacities could be done by handing the initialize function a list of raster objects.

get_observations documentation is a little short, but mainly just needs a good sentence or two upfront as the description. It all starts to make sense reading through the @param arguments. Could be interesting to work this function into the existing package vignette (this is not strictly necessary, but it might help highlight all the functionality the package has to offer).

Are the LICENSE and LICENSE.md both needed? They could be, but I’m just making sure.

Is it worth including a seed argument to the sim function? This is not strictly necessary, but for reproducibility it means the end user will be responsible for setting the seed before the sim function.

Otherwise it looks great. Really well-documented and useful package. It installs easily and passes R package checks. ran through all vignette code and played around with most (if not all) the functions in the package.

Check duration: 1m 45.6s 0 errors | 0 warnings | 1 note

checking top-level files … NOTE Non-standard file/directory found at top level: CODE_OF_CONDUCT.md

Below is random code from me fiddling around with the package (included for completeness, but feel free to ignore). Some of the simulations take a very long time (e.g., if dispersal rate is really high or if landscape size gets large). If the simulation is initialized with all values of n1 (e.g., n1_small) set to 0, it does not error out that gracefully. It could easily be asked ‘why would the user do this?’, but probably best to cover as many bases with informative error messages?

getSDSim <- function(n1=n1_small, K=K_small, 
  r_sd=0, K_sd=0, timez=100, d=1){

  sim_data_01 <- initialise(
    n1_map = n1,
    K_map = K,
    r = log(2),
    r_sd=r_sd,
    K_sd=K_sd,
    rate = d
  )

  sim_result_01 <- sim(obj = sim_data_01, time = timez)
  plot(sim_result_01)
  return(sim_result_01)
}

# playing around with r_sd and seed-set behavior
sim0 <- getSDSim(r_sd=0, timez=500)

set.seed(123)
sim1 <- getSDSim(r_sd=log(2), timez=500)
set.seed(123)
sim1b <- getSDSim(r_sd=log(2), timez=500)

sim2 <- getSDSim(r_sd=log(4), timez=500)

# working through some random simulations on constructed landscapes
makeLandscape <- function(popSize=4, n=10, m=10){
  r <- terra::rast(matrix(popSize, nrow=n, ncol=m))
  crs(r) <- "+proj=lcc +lat_1=48 +lat_2=33 +lon_0=-100 +ellps=WGS84"
  return(r)
}

n1 <- makeLandscape(10,10,10)
k1 <- n1
values(k1) <- rep(100, 100)

sim1a <- getSDSim(n1, k1, r_sd=0, d=0.001, timez=100)
sim2a <- getSDSim(n1, k1, r_sd=1, d=0.001, timez=100)

n0 <- makeLandscape(0,10,10)
sim0start <- getSDSim(n0, k1, timez=10)

Error in .as.raster.continuous(out, x, type) : outrange[2] > outrange[1] is not TRUE

katarzynam-165 commented 11 months ago

Thank you, @taddallas , for your thoughtful review and helpful feedback! I'm currently incorporating the suggested changes and will get back to you shortly.

adamhsparks commented 11 months ago

Just to update everyone, I'm still looking for a second reviewer. The individual that I've contacted most recently hasn't responded and they have a few more days to do so. However, I'll be on holiday this coming week, so I won't be doing anything until at least next Sunday (AWST) unless I hear back from them before tomorrow.

ropensci-review-bot commented 10 months ago

:calendar: taddallas you have 2 days left before the due date for your review (2023-08-16).

ropensci-review-bot commented 10 months ago

:calendar: @taddallas you have 2 days left before the due date for your review (2023-08-16).

adamhsparks commented 10 months ago

@taddallas, please ignore this. I was away last week and now this week I'm unwell so some of my duties have been missed here. You have obviously provided a nice review, I just need to log it.

adamhsparks commented 10 months ago

@ropensci-review-bot submit review https://github.com/ropensci/software-review/issues/595#issuecomment-1658535279 time 3.5

ropensci-review-bot commented 10 months ago

Logged review for taddallas (hours: 3.5)

adamhsparks commented 10 months ago

@katarzynam-165, apologies. I'm having difficulties finding a second reviewer for your package. I'm still looking, if you have any suggestions of someone that you think might be suitable and able, I would be happy to take that into consideration.

katarzynam-165 commented 10 months ago

@adamhsparks, we appreciate your ongoing efforts to secure a second reviewer for our package. We do have two potential candidates in mind who could be suitable, although we cannot guarantee their availability or willingness to participate in the review:

katarzynam-165 commented 10 months ago

@taddallas, once again, thank you for taking the time to review our package, and for your valuable feedback. We greatly appreciate your efforts in evaluating our work.

  1. We added you to the DESCRIPTION file in the following way:person("Tad", "Dallas", comment = c(github = "@taddallas"), role = "rev")). Please inform us if you'd like us to include your email or any additional information.
  2. We acknowledge that the raster package will no longer be supported soon. We've developed the code exclusively using the terra package and are actively working on optimizing it. Users should experience no difference in using our package.
  3. We've included the Code_of_Conduct.md file in .Rbuildignore as per your suggestion.
  4. We updated the documentation for the initialize function and included the default values for all relevant arguments.
  5. We appreciate your insight into the interpretation of r_sd as demographic vs. environmental stochasticity. We will explore this concept in the future – for now we added this as a GitHub issue.
  6. Documentation of the get_observation function was indeed lacking. We added some examples to the package vignette and expanded the help file a little.
  7. LICENSE and LICENSE.md are both needed and serve different purposes. More explanation can be found here.
  8. In our experience setting seed by the user before executing the function is more common and we find it more straightforward. Therefore, we've opted not to include a seed argument in the sim function.

Once again, we extend our gratitude for your thorough review. Please feel free to reach out if you have any further suggestions or inquiries.

katarzynam-165 commented 9 months ago

@adamhsparks, I was wondering if there's been any progress in finding a second reviewer for rangr? Any updates would be greatly appreciated. Thank you!

adamhsparks commented 9 months ago

Dear @katarzynam-165, I do not have any updates. As soon as I'm able to find a second reviewer I will assign them here and you will know. Unfortunately I have as yet been unable to find anyone to provide a second review and I have to wait a full week before I move on to the next person if I don't get a response when I send an e-mail.

katarzynam-165 commented 9 months ago

Thank you for the update! I understand the situation and appreciate your efforts. I hope that you'll be able to find somebody soon.

adamhsparks commented 8 months ago

@ropensci-review-bot assign @TheAnalyticalEdge as reviewer

ropensci-review-bot commented 8 months ago

@TheAnalyticalEdge added to the reviewers list. Review due date is 2023-11-06. Thanks @TheAnalyticalEdge 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 8 months ago

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

TheAnalyticalEdge commented 8 months 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

General Review

Documentation

The package includes all the following forms of documentation:

Visualisation (where appropriate)

Yes. I'd like to see ability to manipulate rows/cols in the plots. E.g., 

# generate visualisation
plot(sim_result_01,
     time_points = c(1, 10, 25, 50, 75, 100),
     template = sim_data_01$K_map, 
     **nrow=3**
)

It will make it easier for reporting if plots can be put in to certain dimensions.

Package Design

I'm worried this package still has warnings for dependancies on sp, which is now retired.

Estimated hours spent reviewing: 3-4 hours.

Extra comments to the Author:

I see a clear need for this package in statistical ecology, and found it fairly easy to implement and use. Help files are all great and good to follow. Code in worked examples all works, etc.

In the get_observations() function, I wonder if extra options for sdlog can be provided. I believe this is some 'random noise' added to the observation to simulate the detectability effect. But often detectability is one-way (people count less animals than present) and for future, having different options in here would be good.

In the help files, contrasting ?initialise (good) and ?sim (less good), can the formatting of the input arguments be changed? the helpful arguments run off the screen which makes it annoying to read (picky of me, I know!) compared to ?initialise where the arguments are all on separate lines.

There is a minor typo on the help page: 'and more suitable for the virtual species'. Just after creating 'K_small_changing' object.

HTH :)

adamhsparks commented 8 months ago

@ropensci-review-bot submit review https://github.com/ropensci/software-review/issues/595#issuecomment-1788232241 time 3.5

ropensci-review-bot commented 8 months ago

Logged review for TheAnalyticalEdge (hours: 3.5)

adamhsparks commented 8 months ago

Dear @katarzynam-165, we now have the second review complete, thank you @TheAnalyticalEdge! I do share the concern that both reviewers raised about the use of packages relying on {sp}, as this was officially retired last month. Can you please update the codebase to remove this dependency on {raster} completely by moving to {terra}?

katarzynam-165 commented 8 months ago

@TheAnalyticalEdge thank you for your review and valuable suggestions! I appreciate your feedback. @adamhsparks I'm currently attending a statistical course until the end of this week, but I'll make the necessary changes next week. I'll also address the reviewer's suggestions then. Sorry for the delay!