allodb: An R package for biomass estimation at extratropical forest plots #436
Comments
|
@ropensci-review-bot assign @adamhsparks as editor |
|
Assigned! @adamhsparks is now the editor |
Editor checks:
Editor commentsG'day, Erika, The package passes all standard local checks and tests with flying colours. But checking the code syntax using Also, I note that your package's website is incomplete, but once accepted to rOpenSci a website will be generated for you and I note that your current README contains the necessary information on how to install and use the package, so we're good there. Once you've addressed the issues flagged with |
|
@ropensci-review-bot run goodpractice |
|
Hi, @gonzalezeb, just checking in to see if there’s anything I can do to help you with your submission process? |
|
Hi Adam, thanks for checking in. When I run |
|
Yes, it passes both the CRAN checks when I run them with, and This is what I see when I run Some of these can easily be addressed, others like the first hit with names, probably have good reasons in your field to have the object name like that. That's fine, but it should be noted for the reviewers. > lintr::lint_package()
................
R/est_params.R:46:24: style: Variable and function name style should be snake_case.
Nres = 1e4
^~~~
R/est_params.R:64:51: style: Commas should always have a space after.
coords = dfobs[i, c("long","lat")],
^
R/est_params.R:72:66: style: Put spaces around all infix operators.
if (length(unique(df$equation_id)) == 1) df[1, 3] <- df[1, 3]*1.01
~^~
R/get_biomass.R:56:25: style: Variable and function name style should be snake_case.
Nres = 1e4) {
^~~~
R/illustrate_allodb.R:48:31: style: Variable and function name style should be snake_case.
Nres = 1e4) {
^~~~
R/imports.R:6:1: style: Variable and function name style should be snake_case.
data.frame <- function(...) {
^~~~~~~~~~
R/new_equations.R:62:1: style: functions should have cyclomatic complexity of less than 15, this has 24.
new_equations <- function(subset_taxa = "all",
^
R/new_equations.R:71:27: style: Variable and function name style should be snake_case.
new_minDBH = NULL,
^~~~~~~~~~
R/new_equations.R:72:27: style: Variable and function name style should be snake_case.
new_maxDBH = NULL,
^~~~~~~~~~
R/new_equations.R:73:27: style: Variable and function name style should be snake_case.
new_sampleSize = NULL,
^~~~~~~~~~~~~~
R/new_equations.R:74:27: style: Variable and function name style should be snake_case.
new_unitDBH = "cm",
^~~~~~~~~~~
R/new_equations.R:75:27: style: Variable and function name style should be snake_case.
new_unitOutput = "kg",
^~~~~~~~~~~~~~
R/new_equations.R:76:27: style: Variable and function name style should be snake_case.
new_inputVar = "DBH",
^~~~~~~~~~~~
R/new_equations.R:77:27: style: Variable and function name style should be snake_case.
new_outputVar = "Total aboveground biomass",
^~~~~~~~~~~~~
R/new_equations.R:88:34: style: Variable and function name style should be snake_case.
suppressWarnings(new_equations$dbh_unit_CF <-
^~~~~~~~~~~
R/new_equations.R:90:34: style: Variable and function name style should be snake_case.
suppressWarnings(new_equations$output_units_CF <-
^~~~~~~~~~~~~~~
R/new_equations.R:112:5: style: Variable and function name style should be snake_case.
toMerge <- eq_jansen[, c("hsub", "equation_allometry")]
^~~~~~~
R/new_equations.R:113:64: style: Variable and function name style should be snake_case.
eq_jansen$equation_allometry <- apply(toMerge, 1, function(X) {
^
R/new_equations.R:181:30: style: There should be a space between right parenthesis and an opening curly brace.
if (is.matrix(new_coords)){
^~
R/new_equations.R:265:5: style: Variable and function name style should be snake_case.
equationID <- paste0("new", seq_len(length(new_taxa)))
^~~~~~~~~~
R/new_equations.R:266:5: style: Variable and function name style should be snake_case.
coordsEq <- cbind(long = new_coords[, 1],
^~~~~~~~
R/new_equations.R:268:5: style: Variable and function name style should be snake_case.
rcoordsEq <- round(coordsEq * 2 - 0.5) / 2 + 0.25
^~~~~~~~~
R/new_equations.R:270:5: style: Variable and function name style should be snake_case.
koppenZones <- apply(rcoordsEq, 1, function(X) {
^~~~~~~~~~~
R/new_equations.R:270:49: style: Variable and function name style should be snake_case.
koppenZones <- apply(rcoordsEq, 1, function(X) {
^
R/new_equations.R:273:5: style: Variable and function name style should be snake_case.
koppenZones <- as.character(unlist(koppenZones))
^~~~~~~~~~~
R/new_equations.R:301:5: style: Variable and function name style should be snake_case.
dbhCF <-
^~~~~
R/new_equations.R:303:5: style: Variable and function name style should be snake_case.
outputCF <-
^~~~~~~~
R/new_equations.R:305:28: style: Variable and function name style should be snake_case.
suppressWarnings(dbhCF$dbh_unit_CF <-
^~~~~~~~~~~
R/new_equations.R:307:31: style: Variable and function name style should be snake_case.
suppressWarnings(outputCF$output_units_CF <-
^~~~~~~~~~~~~~~
R/resample_agb.R:41:26: style: Variable and function name style should be snake_case.
Nres = 1e4) {
^~~~
R/resample_agb.R:78:47: style: Variable and function name style should be snake_case.
list_dbh <- apply(dfsub[, 1:3], 1, function(X) {
^
R/resample_agb.R:89:5: warning: local variable ‘sampled_dbh’ assigned but may not be used
sampled_dbh <- list_dbh
^~~~~~~~~~~
R/resample_agb.R:97:7: warning: local variable ‘sampled_dbh’ assigned but may not be used
sampled_dbh <- list_dbh[[j]]
^~~~~~~~~~~
R/weight_allom.R:45:31: style: Variable and function name style should be snake_case.
suppressWarnings(dfequation$wN <-
^~
R/weight_allom.R:53:3: style: Variable and function name style should be snake_case.
coordsSite <- t(as.numeric(coords))
^~~~~~~~~~
R/weight_allom.R:54:3: style: Variable and function name style should be snake_case.
rcoordsSite <- round(coordsSite * 2 - 0.5) / 2 + 0.25
^~~~~~~~~~~
R/weight_allom.R:56:3: style: Variable and function name style should be snake_case.
koppenObs <- apply(rcoordsSite, 1, function(X) {
^~~~~~~~~
R/weight_allom.R:56:47: style: Variable and function name style should be snake_case.
koppenObs <- apply(rcoordsSite, 1, function(X) {
^
R/weight_allom.R:64:14: style: Variable and function name style should be snake_case.
dfequation$wE <- vapply(dfequation$koppen, compare_koppen, FUN.VALUE = 0.9)
^~
R/weight_allom.R:86:14: style: Variable and function name style should be snake_case.
dfequation$wT <- 1e-6
^~
R/weight_allom.R:91:3: style: Variable and function name style should be snake_case.
eqtaxaG <- data.table::tstrsplit(dfequation$taxa1, " ")[[1]]
^~~~~~~
R/weight_allom.R:92:3: style: Variable and function name style should be snake_case.
eqtaxaS <- data.table::tstrsplit(dfequation$taxa1, " ")[[2]]
^~~~~~~
R/weight_allom.R:152:3: style: Variable and function name style should be snake_case.
vecW <- dfequation$w
^~~~
tests/testthat/test-est_params.R:5:11: style: Opening curly braces should never go on their own line and should always be followed by a new line.
{
^
tests/testthat/test-est_params.R:10:39: style: Commas should always have a space after.
long = c(-78,-85),
^
tests/testthat/test-get_biomass.R:5:11: style: Opening curly braces should never go on their own line and should always be followed by a new line.
{
^
tests/testthat/test-get_biomass.R:25:11: style: Opening curly braces should never go on their own line and should always be followed by a new line.
{
^
tests/testthat/test-get_biomass.R:92:11: style: Opening curly braces should never go on their own line and should always be followed by a new line.
{
^
tests/testthat/test-get_biomass.R:106:11: style: Opening curly braces should never go on their own line and should always be followed by a new line.
{
^
tests/testthat/test-get_biomass.R:122:11: style: Opening curly braces should never go on their own line and should always be followed by a new line.
{
^
tests/testthat/test-illustrate_allodb.R:10:11: style: Opening curly braces should never go on their own line and should always be followed by a new line.
{
^
tests/testthat/test-illustrate_allodb.R:31:11: style: Opening curly braces should never go on their own line and should always be followed by a new line.
{
^
tests/testthat/test-new_equations.R:4:11: style: Opening curly braces should never go on their own line and should always be followed by a new line.
{
^
tests/testthat/test-new_equations.R:127:3: style: Opening curly braces should never go on their own line and should always be followed by a new line.
{
^
tests/testthat/test-resample_agb.R:19:3: style: Opening curly braces should never go on their own line and should always be followed by a new line.
{
^
tests/testthat/test-weight_allom.R:5:11: style: Opening curly braces should never go on their own line and should always be followed by a new line.
{
^ |
|
There is a note about UTF-8 strings as well that I'm seeing that you'll need to address as well. * checking data for non-ASCII characters ... NOTE
Note: found 156 marked UTF-8 strings |
|
Thanks @adamhsparks. Most of the suggestion after running |
|
Ah, sorry, I didn’t pay attention and realise you’d applied the changes to the repo yet. I’d need to refresh my local branch and rerun. I thought you were having issues identifying and applying fixes to these initial problems.
…
On 5 Apr 2021, at 09:47, Erika Gonzalez-Akre ***@***.***> wrote:
Thanks @adamhsparks. Most of the suggestion after running lint_package() have already been applied, see for example here and here. I am not sure why they don't show to you. I still need to work on some thought. Let me know if after installing the package again you still see those.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
|
OK, this is what I get now. > lintr::lint_package()
................
R/new_equations.R:62:1: style: functions should have cyclomatic complexity of less than 15, this has 24.
new_equations <- function(subset_taxa = "all",
^
R/new_equations.R:115:65: style: Variable and function name style should be snake_case.
eq_jansen$equation_allometry <- apply(to_merge, 1, function(X) {
^
R/new_equations.R:273:51: style: Variable and function name style should be snake_case.
koppen_zones <- apply(rcoords_eq, 1, function(X) {
^
R/resample_agb.R:78:47: style: Variable and function name style should be snake_case.
list_dbh <- apply(dfsub[, 1:3], 1, function(X) {
^
R/resample_agb.R:89:5: warning: local variable ‘sampled_dbh’ assigned but may not be used
sampled_dbh <- list_dbh
^~~~~~~~~~~
R/resample_agb.R:97:7: warning: local variable ‘sampled_dbh’ assigned but may not be used
sampled_dbh <- list_dbh[[j]]
^~~~~~~~~~~
R/weight_allom.R:45:31: style: Variable and function name style should be snake_case.
suppressWarnings(dfequation$wN <-
^~
R/weight_allom.R:53:3: style: Variable and function name style should be snake_case.
coordsSite <- t(as.numeric(coords))
^~~~~~~~~~
R/weight_allom.R:54:3: style: Variable and function name style should be snake_case.
rcoordsSite <- round(coordsSite * 2 - 0.5) / 2 + 0.25
^~~~~~~~~~~
R/weight_allom.R:56:3: style: Variable and function name style should be snake_case.
koppenObs <- apply(rcoordsSite, 1, function(X) {
^~~~~~~~~
R/weight_allom.R:56:47: style: Variable and function name style should be snake_case.
koppenObs <- apply(rcoordsSite, 1, function(X) {
^
R/weight_allom.R:64:14: style: Variable and function name style should be snake_case.
dfequation$wE <- vapply(dfequation$koppen, compare_koppen, FUN.VALUE = 0.9)
^~
R/weight_allom.R:86:14: style: Variable and function name style should be snake_case.
dfequation$wT <- 1e-6
^~
R/weight_allom.R:91:3: style: Variable and function name style should be snake_case.
eqtaxaG <- data.table::tstrsplit(dfequation$taxa1, " ")[[1]]
^~~~~~~
R/weight_allom.R:92:3: style: Variable and function name style should be snake_case.
eqtaxaS <- data.table::tstrsplit(dfequation$taxa1, " ")[[2]]
^~~~~~~
R/weight_allom.R:152:3: style: Variable and function name style should be snake_case.
vecW <- dfequation$w
^~~~
tests/testthat/test-est_params.R:9:39: style: Commas should always have a space after.
long = c(-78,-85),Are there any that I can provide assistance with? |
|
Hi @adamhsparks. Most issues brought out by In In |
|
Thanks, @gonzalezeb, for addressing these changes. I think that both of these are acceptable given the reasons stated. I'll begin looking for reviewers now. |
|
@gonzalezeb, can you please update your README badges for allodb using |
|
@gonzalezeb, can you also please address the errors that are encountered when running checks? ==> devtools::check()
ℹ Updating allodb documentation
ℹ Loading allodb
Writing NAMESPACE
Writing NAMESPACE
── Building ────────────────────────────────────────────────────────── allodb ──
Setting env vars:
● CFLAGS : -Wall -pedantic -fdiagnostics-color=always
● CXXFLAGS : -Wall -pedantic -fdiagnostics-color=always
● CXX11FLAGS: -Wall -pedantic -fdiagnostics-color=always
────────────────────────────────────────────────────────────────────────────────
✓ checking for file ‘/Users/adamsparks/Development/GitHub/Reviews/allodb/DESCRIPTION’ ...
─ preparing ‘allodb’:
✓ checking DESCRIPTION meta-information ...
─ installing the package to build vignettes
E creating vignettes (5.3s)
--- re-building ‘allodb-vignette.Rmd’ using rmarkdown
Quitting from lines 81-87 (allodb-vignette.Rmd)
Error: processing vignette 'allodb-vignette.Rmd' failed with diagnostics:
arguments imply differing number of rows: 9991, 19
--- failed re-building ‘allodb-vignette.Rmd’
SUMMARY: processing the following file failed:
‘allodb-vignette.Rmd’
Error: Vignette re-building failed.
Execution halted
Error: 'col_ed' is not an exported object from 'namespace:cli'
Execution halted
Exited with status 1.Once that's dealt with, I'll look for reviewers. 🙏 |
adamhsparks
added
holding
topic:reproducibility
package
and removed
2/seeking-reviewer(s)
labels
|
Hi @adamhsparks, devtools::check(remote = TRUE, manual = TRUE)
i Updating allodb documentation
i Loading allodb
Writing NAMESPACE
Writing NAMESPACE
-- Building --------------------------------------------------- allodb --
Setting env vars:
* CFLAGS : -Wall -pedantic -fdiagnostics-color=always
* CXXFLAGS : -Wall -pedantic -fdiagnostics-color=always
* CXX11FLAGS: -Wall -pedantic -fdiagnostics-color=always
-------------------------------------------------------------------------
√ checking for file 'C:\Users\erikab\Dropbox (Smithsonian)\GitHub\allodb/DESCRIPTION' (349ms)
- preparing 'allodb': (11.3s)
√ checking DESCRIPTION meta-information ...
- installing the package to build vignettes
√ creating vignettes (39.3s)
- checking for LF line-endings in source and make files and shell scripts (11.1s)
- checking for empty or unneeded directories
- looking to see if a 'data/datalist' file should be added
- building 'allodb_0.0.0.9000.tar.gz'
-- Checking --------------------------------------------------- allodb --
Setting env vars:
* _R_CHECK_CRAN_INCOMING_USE_ASPELL_: TRUE
* _R_CHECK_CRAN_INCOMING_REMOTE_ : TRUE
* _R_CHECK_CRAN_INCOMING_ : TRUE
* _R_CHECK_FORCE_SUGGESTS_ : FALSE
* NOT_CRAN : true
-- R CMD check ----------------------------------------------------------
- using log directory 'C:/Users/erikab/AppData/Local/Temp/Rtmp6dUAvx/allodb.Rcheck' (1s)
- using R version 3.6.3 (2020-02-29)
- using platform: x86_64-w64-mingw32 (64-bit)
- using session charset: ISO8859-1
- using option '--as-cran' (402ms)
√ checking for file 'allodb/DESCRIPTION'
- this is package 'allodb' version '0.0.0.9000'
- package encoding: UTF-8
N checking CRAN incoming feasibility (35.1s)
Maintainer: 'Erika Gonzalez-Akre <[email protected]>'
New submission
Version contains large components (0.0.0.9000)
√ checking package namespace information ...
√ checking package dependencies (2.3s)
√ checking if this is a source package ...
√ checking if there is a namespace
√ checking for executable files (1.2s)
√ checking for hidden files and directories ...
√ checking for portable file names ...
√ checking whether package 'allodb' can be installed (11.2s)
√ checking installed package size ...
√ checking package directory (894ms)
√ checking for future file timestamps (624ms)
√ checking 'build' directory ...
√ checking DESCRIPTION meta-information (718ms)
√ checking top-level files ...
√ checking for left-over files ...
√ checking index information (353ms)
√ checking package subdirectories (392ms)
√ checking R files for non-ASCII characters ...
√ checking R files for syntax errors ...
√ checking whether the package can be loaded (355ms)
√ checking whether the package can be loaded with stated dependencies ...
√ checking whether the package can be unloaded cleanly (336ms)
√ checking whether the namespace can be loaded with stated dependencies (360ms)
√ checking whether the namespace can be unloaded cleanly (459ms)
√ checking loading without being on the library search path (780ms)
√ checking use of S3 registration (2.5s)
√ checking dependencies in R code (2s)
√ checking S3 generic/method consistency (783ms)
√ checking replacement functions (335ms)
√ checking foreign function calls (802ms)
√ checking R code for possible problems (5.4s)
√ checking Rd files (681ms)
√ checking Rd metadata ...
√ checking Rd line widths ...
√ checking Rd cross-references (343ms)
√ checking for missing documentation entries (473ms)
√ checking for code/documentation mismatches (1.5s)
√ checking Rd \usage sections (1.1s)
√ checking Rd contents ...
√ checking for unstated dependencies in examples ...
√ checking contents of 'data' directory (696ms)
√ checking data for non-ASCII characters (1s)
√ checking data for ASCII and uncompressed saves (588ms)
√ checking R/sysdata.rda (478ms)
√ checking installed files from 'inst/doc' ...
√ checking files in 'vignettes' ...
√ checking examples (4.7s)
√ checking for unstated dependencies in 'tests' ...
- checking tests (429ms)
√ Running 'testthat.R' (8.7s)
√ checking for unstated dependencies in vignettes (9.1s)
√ checking package vignettes in 'inst/doc' ...
√ checking re-building of vignette outputs (23.6s)
√ checking PDF version of manual (3.6s)
√ checking for detritus in the temp directory
See
'C:/Users/erikab/AppData/Local/Temp/Rtmp6dUAvx/allodb.Rcheck/00check.log'
for details.
-- R CMD check results --------------------------- allodb 0.0.0.9000 ----
Duration: 2m 0.2s
> checking CRAN incoming feasibility ... NOTE
Maintainer: 'Erika Gonzalez-Akre <[email protected]>'
New submission
Version contains large components (0.0.0.9000)
0 errors √ | 0 warnings √ | 1 note x |
|
Hi @gonzalezeb, I'm getting a NOTE when I run Note: found 156 marked UTF-8 stringsCan you check into this? As it's just a note, not an error in the checks, I'll start looking for reviewers now. |
|
Hi @adamhsparks. Sorry, I am a little behind with this and have solved only minor issues. There are some comments on the conceptual aspects of the package (i.e. here) where I get the point but changing our approach will require some (lot's!) of work, so I am giving it a careful thought. How should I tackle issues like those? |
|
Hi @gonzalezeb, which specific aspect were you linking to and asking for guidance on? Your link only links to the whole issue here, not one of the reviewers' comments or issue that they may have opened in your repository? |
|
Hi @gonzalezeb, do you need more time to work on this? We can put a holding tag on it and revisit it after three months to see where things are for up to a year. Would that be useful to you? |
|
Hi @adamhsparks, thanks for checking. We have made some substantial progress but I still need to work in some minor stuff. The plan is to finish this before Sep 15 as we want to submit our revised manuscript to MEE also by then. Will that be ok with you all? |
|
Sounds good, Erika! Thanks for updating me. |
|
Hi @adamhsparks, I wasn’t sure how to respond to the multiples reviews so I hope the following is sufficient. We are very grateful for the thoughtful suggestions by reviewers, and have implemented most. Initial suggestions by @jeffreyhanson (5/19/21), have been addressed:
Comprehensive review by @jeffreyhanson (6/17/21), has also been addressed:
Suggestions by @jstillh (6/14/21), have been addressed:
|
|
The reviewers advice to avoid the call Instead they suggest storing the equations as objects such as expressions or functions, and manipulate them with We thank the advice and, after considering it carefully, we would like to share our updated opinion. We first discuss the problems of To make our discussion concrete we offer some code examples. library(dplyr, warn.conflicts = FALSE)
library(allodb)tl;dr
UnsafeOnline we found that parsing text would be a risk if we hosted an application on a server we own and allow users to input arbitrary text. A malicious user could damage our server. But this is not our case. The allodb package is to be used as an analysis tool, directly on the users’s computers. And the input text comes mainly not from users but from a dataset we built. Error proneWe acknowledge that bugs can hide in the unevaluated text, as invalid code is still a valid text object. valid <- "dbh + 1"
eval(parse(text = valid), envir = list(dbh = 1))
#> [1] 2
invalid <- "1 <- dbh"
try(eval(parse(text = invalid), envir = list(dbh = 1)))
#> Error in 1 <- dbh : invalid (do_set) left-hand side to assignmentBut bugs can also hide in unevaluated expressions. Using invalid <- quote(1 <- dbh)
substitute(x, list(x = invalid))
#> 1 <- dbh
try(eval(substitute(x, list(x = invalid))))
#> Error in 1 <- dbh : invalid (do_set) left-hand side to assignmentWe are comfortable storing and manipulating text. We can inspect equations stored as text directly from the dataframe, and we can manipulate them with familiar tools. # Storing text
equation <- "1.2927 * (dbh^2)^1.36723"
dbh <- "(sampled_dbh * 0.393701)"
# + We can inspect the equations directly from the dataframe
tibble(equation, dbh)
#> # A tibble: 1 x 2
#> equation dbh
#> <chr> <chr>
#> 1 1.2927 * (dbh^2)^1.36723 (sampled_dbh * 0.393701)
# - To manipulate the equations we use familiar tools
modified <- gsub("dbh", dbh, equation)
modified
#> [1] "1.2927 * ((sampled_dbh * 0.393701)^2)^1.36723"
eval(parse(text = modified), list(sampled_dbh = 1))
#> [1] 0.1010408In contrast, we are not comfortable storing or manipulating expressions with # We need to create an escape hatch because substitute() doesn't work if we
# already have an expression saved in a variable
# http://adv-r.had.co.nz/Computing-on-the-language.html#substitute
substitute_q <- function(x, env) {
call <- substitute(substitute(y, env), list(y = x))
eval(call)
}
equation <- quote(1.2927 * (dbh^2)^1.36723)
dbh <- quote((sampled_dbh * 0.393701))
# - We can't inspect the equations directly from the dataframe :-(
tibble(equation = list(equation), dbh = list(dbh))
#> # A tibble: 1 x 2
#> equation dbh
#> <list> <list>
#> 1 <language> <language>
# - To manipulate the equations we need unfamiliar tools
modified <- substitute_q(equation, list(dbh = dbh))
modified
#> 1.2927 * ((sampled_dbh * 0.393701)^2)^1.36723
eval(modified, list(sampled_dbh = 1))
#> [1] 0.1010408As @jeffreyhanson advised, instead of using expressions and validate_equation <- function(text, envir = parent.frame()) {
tryCatch(
eval(parse(text = text), envir = envir),
error = function(e) {
stop("`text` must be valid R code.\nx Invalid: ", text, call. = FALSE)
}
)
invisible(text)
}
dbh <- 99
validate_equation(valid)
try(validate_equation(invalid))
#> Error : `text` must be valid R code.
#> x Invalid: <-1dbh
f <- function(text) {
validate_equation(text, envir = list(dbh = 1))
# Some useful code
text
}
f(valid)
#> [1] "dbh + 1"
try(f(invalid))
#> Error : `text` must be valid R code.
#> x Invalid: <-1dbh |
adamhsparks
added
5/awaiting-reviewer(s)-response
and removed
3/reviewer(s)-assigned
labels
|
@gonzalezeb, thank you for your comprehensive overview of the changes that you have made to the allodb package. @jeffreyhanson and @jstillh could you please let me know if you are satisfied with the changes to the package or have any further suggestions that may improve it? |
|
Awesome - thank you very much @gonzalezeb. @adamhsparks, I'll try and take get back to you next week. |
|
@gonzalezeb thanks a lot for the overview of the changes - i'll try to go through them in the next week as well and get back to you, @adamhsparks . |
|
Thank you @gonzalezeb for providing such detailed responses to the points I raised earlier. I especially found your discussion on the costs and benefits of Suggestions
Minor comments
1 Please note that I still (unsurprisingly) lack the forestry experience needed to evaluate the novelty and methodology of this package, and so continue to limit my reviews to code quality and documentation. 2 Just to be clear, I do think that -- where possible, feasible, and reasonable -- R package developers should ideally -- if only in principle -- minimize security risks, even if software licenses absolve developers of responsibility. It's just I think that |
|
Thanks @jeffreyhanson, I have adopted most of your comments, they are very helpful! |
|
Thank you @gonzalezeb for the very detailed response to the reviews. As @jeffreyhanson, I like your detailed response regarding the use of
Thanks for this contribution. |
|
@ropensci-review-bot approve |
|
Could not approve. Please, specify the name of the package. |
|
@ropensci-review-bot approve allodb |
|
Approved! Thanks @gonzalezeb for submitting and @jeffreyhanson, @jstillh for your reviews! 😁 To-dos:
Should you want to acknowledge your reviewers in your package DESCRIPTION, you can do so by making them 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 @stefaniebutland in your reply. She will get in touch about timing and can answer any questions. We've put together an online book with our best practice and tips, this chapter starts the 3d section that's about guidance for after onboarding. Please tell us what could be improved, the corresponding repo is here. Last but not least, you can volunteer as a reviewer via filling a short form. |
ropensci-review-bot
added
6/approved
and removed
5/awaiting-reviewer(s)-response
labels
This fulfills a requirement in [this comment](ropensci/software-review#436 (comment)) as per instructions in https://devguide.ropensci.org/redirect.html#redirect
> Delete your current code of conduct file if you had one since rOpenSci's default one will apply, see https://devguide.ropensci.org/collaboration.html#coc-file --ropensci/software-review#436 (comment)
> We're starting to roll out software metadata files to all ropensci packages via the Codemeta initiative, see https://github.com/ropensci/codemetar/#codemetar for how to include it in your package, after installing the package - should be easy as running codemetar::write_codemeta() in the root of your package. --ropensci/software-review#436 (comment)
Date accepted: 2021-09-27
Due date for @jeffreyhanson: 2021-06-14Submitting Author: Erika Gonzalez-Akre (@gonzalezeb)
Other Authors: Camille Piponiot (@cpiponiot), Mauro Lepore (@maurolepore), Kristina Teixeira (@teixeirak)
Repository: https://github.com/forestgeo/allodb
Version submitted: 0.0.0.9000
Editor: @adamhsparks
Reviewers: @jeffreyhanson, @jstillh
Due date for @jstillh: 2021-06-24
Archive: TBD
Version accepted: TBD
Scope
Please indicate which category or categories from our package fit policies this package falls under: (Please check an appropriate box below. If you are unsure, we suggest you make a pre-submission inquiry.):
Explain how and why the package falls under these categories (briefly, 1-2 sentences):
allodbintegrates compiled data and built-in functions to calculate tree above-ground biomass (AGB) which is a critical value used by forest ecologists to estimate the amount of carbon sequester and resealed by forests.allodbfalls in the "field and laboratory reproducibility tools" category as our package intents to improve and standardize the calculation of a widely used forest metric.Who is the target audience and what are scientific applications of this package?
allodbis designed to be used by forest ecologists with basic or advanced knowledge of R. This scientific application is intended to be used by individuals interested in calculating forest metrics such as AGB in extratropical forest (subtropical, temperate, and boreal forests).Are there other R packages that accomplish the same thing? If so, how does yours differ or meet our criteria for best-in-category?
The BIOMASS package is designed to estimate biomass in tropical forest. The main difference between
allodband BIOMASS is the set of equations used to estimate AGB. There is an existing calibrated equation used globally for tropical environments but not for areas outside the tropics. Our goal is to provide a set of curated equations and the ability to add new and additional equations in order to have a more reliable value of AGB outside the tropics. We do not duplicate any functionality of the BIOMASS package and only agree on the ecological scope.(If applicable) Does your package comply with our guidance around Ethics, Data Privacy and Human Subjects Research?
N/A
If you made a pre-submission enquiry, please paste the link to the corresponding issue, forum post, or other discussion, or @tag the editor you contacted.
@melvidoni or issue 427
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?
Do you wish to submit an Applications Article about your package to Methods in Ecology and Evolution? If so:
MEE Options
Code of conduct
The text was updated successfully, but these errors were encountered: