software-review fireexposuR: Compute and Visualize Wildfire Exposure

Submitting Author Name: Air Forbes Submitting Author Github Handle: @heyairf Repository: https://github.com/heyairf/fireexposuR Version submitted: 1.0.0 Submission type: Standard Editor: @maurolepore Reviewers: TBD

Archive: TBD Version accepted: TBD Language: en

Paste the full DESCRIPTION file inside a code block below:

Type: Package
Package: fireexposuR
Title: Compute and Visualize Wildfire Exposure
Version: 1.0.0
Authors@R: c(person("Air", "Forbes", email = "[email protected]", 
    role = c("aut", "cre"), comment = c(ORCID = "0000-0002-9842-7648")),
    person("Jennifer", "Beverly", role = "aut"))
Description: This package computes wildfire exposure using methods from
    Beverly et al. (2010), Beverly et al. (2021), and Beverly and Forbes
    (2023).  It provides functions to standardize the mapping and
    visualization of wildfire exposure. This package requires
    pre-processing of data which is docomented in Forbes and Beverly
    (in preparation).
License: GPL (>= 3)
Imports: 
    dplyr,
    geosphere,
    ggplot2,
    ggspatial,
    magrittr,
    maptiles,
    MultiscaleDTM,
    rlang,
    terra,
    tidyr,
    tidyselect,
    tidyterra
Encoding: UTF-8
LazyData: true
RoxygenNote: 7.3.2
Roxygen: list(markdown = TRUE)
URL: https://github.com/heyairf/fireexposuR
BugReports: https://github.com/heyairf/fireexposuR/issues
Suggests: 
    knitr,
    rmarkdown,
    testthat (>= 3.0.0)
Config/testthat/edition: 3
Depends: 
    R (>= 2.10)
VignetteBuilder:
    knitr

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.):
- [ ] data retrieval
- [ ] data extraction
- [ ] data munging
- [ ] data deposition
- [x] data validation and testing
- [x] workflow automation
- [ ] version control
- [ ] citation management and bibliometrics
- [ ] scientific software wrappers
- [ ] field and lab reproducibility tools
- [ ] database software bindings
- [x] geospatial data
- [ ] text analysis
Explain how and why the package falls under these categories (briefly, 1-2 sentences):

This package was developed to share methodologies from existing research by modifying/expanding existing functions. The functions automate and standardize assessments to increase access and quality assurance in the calculation for interested users.

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

Users interested in wildfire risk assessments including but not limited to researchers, consultants, planners, land use decision makers.

Are there other R packages that accomplish the same thing? If so, how does yours differ or meet our criteria for best-in-category?

There are no other R packages that fill this role.

(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 inquiry, please paste the link to the corresponding issue, forum post, or other discussion, or @tag the editor you contacted.

#652

Explain reasons for any pkgcheck items which your package is unable to pass.

all pkgcheck's are currently passing

Technical checks

Confirm each of the following by checking the box.

[x] I have read the rOpenSci packaging guide.
[x] I have read the author guide and I expect to maintain this package for at least 2 years or to find a replacement.

This package:

[x] does not violate the Terms of Service of any service it interacts with.
[ ] has a CRAN and OSI accepted license.
[x] contains a README with instructions for installing the development version.
[x] includes documentation with examples for all functions, created with roxygen2.
[x] contains a vignette with examples of its essential functions and uses.
[x] has a test suite.
[x] has continuous integration, including reporting of test coverage.

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

[ ] The package is novel and will be of interest to the broad readership of the journal.
[ ] The manuscript describing the package is no longer than 3000 words.
[ ] You intend to archive the code for the package in a long-term repository which meets the requirements of the journal (see MEE's Policy on Publishing Code)
(Scope: Do consider MEE's Aims and Scope for your manuscript. We make no guarantee that your manuscript will be within MEE scope.)
(Although not required, we strongly recommend having a full manuscript prepared when you submit here.)
(Please do not submit your package separately to Methods in Ecology and Evolution)

Code of conduct

[x] I agree to abide by rOpenSci's Code of Conduct during the review process and in maintaining my package should it be accepted.

Sep 25 '24 17:09 heyairf

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

Sep 25 '24 17:09 ropensci-review-bot

:rocket:

Editor check started

:wave:

Sep 25 '24 17:09 ropensci-review-bot

Checks for fireexposuR (v1.0.0)

git hash: aacfb397

:heavy_check_mark: Package name is available
:heavy_check_mark: has a 'codemeta.json' file.
:heavy_check_mark: has a 'contributing' file.
:heavy_check_mark: uses 'roxygen2'.
:heavy_check_mark: 'DESCRIPTION' has a URL field.
:heavy_check_mark: 'DESCRIPTION' has a BugReports field.
:heavy_check_mark: Package has at least one HTML vignette
:heavy_check_mark: All functions have examples.
:heavy_check_mark: Package has continuous integration checks.
:heavy_check_mark: Package coverage is 93.9%.
:heavy_check_mark: R CMD check found no errors.
:heavy_check_mark: R CMD check found no warnings.
:eyes: Function names are duplicated in other packages

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

Package License: GPL (>= 3)

1. 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	147
internal	fireexposuR	14
internal	grid	12
internal	stats	10
internal	utils	6
internal	graphics	4
internal	grDevices	2
imports	magrittr	95
imports	dplyr	49
imports	ggplot2	49
imports	terra	45
imports	tidyterra	15
imports	geosphere	8
imports	maptiles	6
imports	ggspatial	4
imports	tidyr	3
imports	MultiscaleDTM	2
imports	rlang	NA
imports	tidyselect	NA
suggests	knitr	NA
suggests	rmarkdown	NA
suggests	testthat	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.

base

c (27), exp (19), sum (11), cbind (9), data.frame (9), as.data.frame (8), paste (6), ifelse (5), by (4), levels (4), matrix (4), ncol (4), all (3), factor (3), labels (3), rep (3), seq (3), class (2), match (2), names (2), round (2), unique (2), as.factor (1), for (1), if (1), length (1), max (1), mean (1), missing (1), outer (1), plot (1), rbind (1), scale (1), table (1)

magrittr

%>% (95)

dplyr

mutate (43), count (4), case_when (1), summarise (1)

ggplot2

element_blank (9), ggplot (7), aes (6), element_text (5), coord_sf (4), geom_col (3), labs (3), element_rect (2), expansion (2), scale_fill_manual (2), geom_bar (1), geom_hline (1), geom_vline (1), scale_x_discrete (1), scale_y_continuous (1), theme (1)

terra

classify (7), crop (6), focal (4), mask (4), project (4), vect (4), extract (3), res (3), rescale (3), spatSample (2), as.polygons (1), buffer (1), crs (1), merge (1), perim (1)

tidyterra

geom_spatraster_rgb (6), geom_spatvector (3), rename (2), filter (1), geom_spatraster (1), scale_fill_whitebox_c (1), whitebox.colors (1)

fireexposuR

exposure (7), direxp (2), adjustexp (1), extractexp (1), mapexpclass (1), mapexpcont (1), multidirexp (1)

grid

unit (12)

stats

window (6), df (4)

geosphere

destPoint (8)

maptiles

get_credit (3), get_tiles (3)

utils

data (6)

ggspatial

annotation_scale (4)

graphics

title (4)

tidyr

pivot_wider (2), pivot_longer (1)

grDevices

palette (2)

MultiscaleDTM

annulus_window (2)

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

2. 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 11 files) and
2 authors
1 vignette
no internal data file
12 imported packages
9 exported functions (median 52 lines of code)
9 non-exported functions in R (median 76 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	11	60.1
files_vignettes	1	62.0
files_tests	10	87.6
loc_R	808	59.6
loc_vignettes	114	28.0
loc_tests	289	60.1
num_vignettes	1	59.0
n_fns_r	18	25.4
n_fns_r_exported	9	42.3
n_fns_r_not_exported	9	20.6
n_fns_per_file_r	1	1.9	TRUE
num_params_per_fn	4	51.1
loc_per_fn_r	60	92.4
loc_per_fn_r_exp	52	80.4
loc_per_fn_r_not_exp	76	95.3	TRUE
rel_whitespace_R	11	47.2
rel_whitespace_vignettes	32	25.7
rel_whitespace_tests	20	58.5
doclines_per_fn_exp	50	62.7
doclines_per_fn_not_exp	0	0.0	TRUE
fn_call_network_size	1	11.5

2a. Network visualisation

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

3. `goodpractice` and other checks

Details of goodpractice checks (click to open)

3a. Continuous Integration Badges

GitHub Workflow Results

id	name	conclusion	sha	run_number	date
11037129203	pkgcheck	success	aacfb3	7	2024-09-25
11037129172	R-CMD-check.yaml	success	aacfb3	7	2024-09-25

3b. `goodpractice` results

`R CMD check` with rcmdcheck

rcmdcheck found no errors, warnings, or notes

Test coverage with covr

Package coverage: 93.87

Cyclocomplexity with cyclocomp

The following function have cyclocomplexity >= 15:

function	cyclocomplexity
extractexp	17

Static code analyses with lintr

lintr found the following 10 potential issues:

message	number of times
Avoid 1:length(...) expressions, use seq_len.	1
Avoid library() and require() calls in packages	3
Lines should not be more than 80 characters. This line is 83 characters.	4
Lines should not be more than 80 characters. This line is 84 characters.	1
Lines should not be more than 80 characters. This line is 85 characters.	1

4. Other Checks

Details of other checks (click to open)

:heavy_multiplication_x: The following function name is duplicated in other packages:

- exposure from mds, mlxR, netdiffuseR, RsSimulx

Package Versions

package	version
pkgstats	0.1.6.17
pkgcheck	0.1.2.58

Editor-in-Chief Instructions:

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

Sep 25 '24 18:09 ropensci-review-bot

@ropensci-review-bot check package

Sep 26 '24 03:09 adamhsparks

Thanks, about to send the query.

Sep 26 '24 03:09 ropensci-review-bot

:rocket:

Editor check started

:wave:

Sep 26 '24 03:09 ropensci-review-bot

Checks for fireexposuR (v1.0.0)

git hash: aacfb397

:heavy_check_mark: Package name is available
:heavy_check_mark: has a 'codemeta.json' file.
:heavy_check_mark: has a 'contributing' file.
:heavy_check_mark: uses 'roxygen2'.
:heavy_check_mark: 'DESCRIPTION' has a URL field.
:heavy_check_mark: 'DESCRIPTION' has a BugReports field.
:heavy_check_mark: Package has at least one HTML vignette
:heavy_check_mark: All functions have examples.
:heavy_check_mark: Package has continuous integration checks.
:heavy_check_mark: Package coverage is 93.9%.
:heavy_check_mark: R CMD check found no errors.
:heavy_check_mark: R CMD check found no warnings.
:eyes: Function names are duplicated in other packages

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

Package License: GPL (>= 3)

1. 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	147
internal	fireexposuR	14
internal	grid	12
internal	stats	10
internal	utils	6
internal	graphics	4
internal	grDevices	2
imports	magrittr	95
imports	dplyr	49
imports	ggplot2	49
imports	terra	45
imports	tidyterra	15
imports	geosphere	8
imports	maptiles	6
imports	ggspatial	4
imports	tidyr	3
imports	MultiscaleDTM	2
imports	rlang	NA
imports	tidyselect	NA
suggests	knitr	NA
suggests	rmarkdown	NA
suggests	testthat	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.

base

c (27), exp (19), sum (11), cbind (9), data.frame (9), as.data.frame (8), paste (6), ifelse (5), by (4), levels (4), matrix (4), ncol (4), all (3), factor (3), labels (3), rep (3), seq (3), class (2), match (2), names (2), round (2), unique (2), as.factor (1), for (1), if (1), length (1), max (1), mean (1), missing (1), outer (1), plot (1), rbind (1), scale (1), table (1)

magrittr

%>% (95)

dplyr

mutate (43), count (4), case_when (1), summarise (1)

ggplot2

element_blank (9), ggplot (7), aes (6), element_text (5), coord_sf (4), geom_col (3), labs (3), element_rect (2), expansion (2), scale_fill_manual (2), geom_bar (1), geom_hline (1), geom_vline (1), scale_x_discrete (1), scale_y_continuous (1), theme (1)

terra

classify (7), crop (6), focal (4), mask (4), project (4), vect (4), extract (3), res (3), rescale (3), spatSample (2), as.polygons (1), buffer (1), crs (1), merge (1), perim (1)

tidyterra

geom_spatraster_rgb (6), geom_spatvector (3), rename (2), filter (1), geom_spatraster (1), scale_fill_whitebox_c (1), whitebox.colors (1)

fireexposuR

exposure (7), direxp (2), adjustexp (1), extractexp (1), mapexpclass (1), mapexpcont (1), multidirexp (1)

grid

unit (12)

stats

window (6), df (4)

geosphere

destPoint (8)

maptiles

get_credit (3), get_tiles (3)

utils

data (6)

ggspatial

annotation_scale (4)

graphics

title (4)

tidyr

pivot_wider (2), pivot_longer (1)

grDevices

palette (2)

MultiscaleDTM

annulus_window (2)

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

2. 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 11 files) and
2 authors
1 vignette
no internal data file
12 imported packages
9 exported functions (median 52 lines of code)
9 non-exported functions in R (median 76 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	11	60.1
files_vignettes	1	62.0
files_tests	10	87.6
loc_R	808	59.6
loc_vignettes	114	28.0
loc_tests	289	60.1
num_vignettes	1	59.0
n_fns_r	18	25.4
n_fns_r_exported	9	42.3
n_fns_r_not_exported	9	20.6
n_fns_per_file_r	1	1.9	TRUE
num_params_per_fn	4	51.1
loc_per_fn_r	60	92.4
loc_per_fn_r_exp	52	80.4
loc_per_fn_r_not_exp	76	95.3	TRUE
rel_whitespace_R	11	47.2
rel_whitespace_vignettes	32	25.7
rel_whitespace_tests	20	58.5
doclines_per_fn_exp	50	62.7
doclines_per_fn_not_exp	0	0.0	TRUE
fn_call_network_size	1	11.5

2a. Network visualisation

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

3. `goodpractice` and other checks

Details of goodpractice checks (click to open)

3a. Continuous Integration Badges

GitHub Workflow Results

id	name	conclusion	sha	run_number	date
11037129203	pkgcheck	success	aacfb3	7	2024-09-25
11037129172	R-CMD-check.yaml	success	aacfb3	7	2024-09-25

3b. `goodpractice` results

`R CMD check` with rcmdcheck

rcmdcheck found no errors, warnings, or notes

Test coverage with covr

Package coverage: 93.87

Cyclocomplexity with cyclocomp

The following function have cyclocomplexity >= 15:

function	cyclocomplexity
extractexp	17

Static code analyses with lintr

lintr found the following 10 potential issues:

message	number of times
Avoid 1:length(...) expressions, use seq_len.	1
Avoid library() and require() calls in packages	3
Lines should not be more than 80 characters. This line is 83 characters.	4
Lines should not be more than 80 characters. This line is 84 characters.	1
Lines should not be more than 80 characters. This line is 85 characters.	1

4. Other Checks

Details of other checks (click to open)

:heavy_multiplication_x: The following function name is duplicated in other packages:

- exposure from mds, mlxR, netdiffuseR, RsSimulx

Package Versions

package	version
pkgstats	0.1.6.17
pkgcheck	0.1.2.58

Editor-in-Chief Instructions:

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

Sep 26 '24 03:09 ropensci-review-bot

Hi @heyairf, local checks indicate that you may not be properly ignoring the codemeta.json file in .Rbuildignore. Can you check on that for me, please?

Sep 26 '24 03:09 adamhsparks

Hi @adamhsparks , I think that is now fixed?

Edit: Nevermind. Standby.

Sep 26 '24 16:09 heyairf

Thank you! I am still learning a lot about R and github.

Sep 27 '24 17:09 heyairf

@ropensci-review-bot assign @maurolepore as reviewer

Sep 28 '24 23:09 adamhsparks

Can't assign reviewer because there is no editor assigned for this submission yet

Sep 28 '24 23:09 ropensci-review-bot

@ropensci-review-bot assign @maurolepore as editor

Sep 28 '24 23:09 adamhsparks

Assigned! @maurolepore is now the editor

Sep 28 '24 23:09 ropensci-review-bot

Hi @heyairf, thanks a lot for your submission. It's my pleasure to be the handling editor.

Semantic tags for my comments

To help you track my comments I'll tag them with "ml" and a numbered sequence, e.g. ml01, ml02, and so on. Comments following bullets are for you to consider -- you may or may not respond to them. Comments following check-boxes are requests for some action -- please respond.

Reviewers

[x] ml01. Can you please suggest three reviewers? Following our guidelines I may appoint at most one; the other two will help me better understand the type of expertise you value.

Editor checks

I'll post my checks in a few days. I already explored the package and looks in good shape. I appreciate your kindness in sharing your work with the rOpenSci community, and your willingness to learn a bit more about R package development. In that spirit I'll share a number of suggestions to make the reviews as smooth as possible.

Oct 01 '24 00:10 maurolepore

Hi @maurolepore, I am very much looking forward to the review process and opportunity to learn. Please note that the world of R package development is very new to me so my reviewer suggestions will reflect that. The following suggestions are people that participate in wildfire research projects, are proficient in R, and are, at varying levels, familiar with the methods that are in this package.

Liz Chapman [email protected]
Jen Schmidt [email protected]
Brett Moore [email protected]

Oct 01 '24 17:10 heyairf

Hi @heyairf sorry for the delay. I'm close to finishing polishing the notes from my checks. I expect to finish it by Wed-Thu at the latest.

Oct 07 '24 00:10 maurolepore

Dear @heyairf,

Once again, thank for sharing your great work with the rOpenSci community.

Here are my checks and comments (based on the editor's template). Remember the items in bullets are just for your consideration, and the check-boxes require some action (sometimes just an explanation). The more items you can address before review the better. It will reduce the chance that reviewers will pick up in these relatively uninteresting structural issues and free them to focus on the more interesting aspects of the package.

Please feel free to ask for clarifications or help. If I don't hear from you before, I'll touch base in a couple of weeks.

Editor checks:

[ ] Documentation: The package has sufficient documentation available online (README, pkgdown docs) to allow for an assessment of functionality and scope without installing the package. In particular,
- [x] Is the case for the package well made?
- [ ] Is the reference index page clear (grouped by topic if necessary)?
- [x] Are vignettes readable, sufficiently detailed and not just perfunctory?
[ ] Fit: The package meets criteria for fit and overlap.
[x] Installation instructions: Are installation instructions clear enough for human users?
[ ] Tests: If the package has some interactivity / HTTP / plot production etc. are the tests using state-of-the-art tooling?
[x] Contributing information: Is the documentation for contribution clear enough e.g. tokens for tests, playgrounds?
[x] License: The package has a CRAN or OSI accepted license.
[ ] Project management: Are the issue and PR trackers in a good shape, e.g. are there outstanding bugs, is it clear when feature requests are meant to be tackled?

Editor comments

Documentation

ml01. The case for the package is made -- it provides an implementation in R of the methods described in three papers by Beverly et al. Consider giving a short description of each paper to help the users find the relevant one. For example, this clause leaves me wondering which paper explains how to prepare the data:

"an accompanying paper details suggestions for data acquisition and preparation in accordance with various budget limitations and user experience levels."

[x] ml02. Please add a website for the package.

We recommend creating a documentation website for your package using pkgdown. -- https://devdevguide.netlify.app/pkg_building.html#website

It sounds hard but all you need to do is to run usethis::use_pkgdown_github_pages() on the console:

use_pkgdown_github_pages(): implements the GitHub setup needed to automatically publish your pkgdown site to GitHub pages: -- https://usethis.r-lib.org/reference/use_pkgdown.html

This allows "for an assessment of functionality and scope without installing the package", which should help the reviewers and users.

ml03. The reference index (the list of all the exported functions and datasets of the package) can be made a little clearer by grouping functions. This package doesn't have many functions but it's still something to consider.

When your package has many functions, use grouping in the reference, which you can do more or less automatically. https://devdevguide.netlify.app/pkg_building.html#function-grouping

More importantly, it would be nice to use references to "exposure" consistently in the names of the functions. Right now I see three ways to refer to exposure:

In full: exposure
As a suffix: adjustexp
In the middle: mapexpclass

Consider using a prefix rather than the alternatives. That way users can type the prefix and see all related functions thanks to auto-complete.

Also "We strongly recommend snake_case over all other styles unless you are porting over a package that is already in wide use". For example, adjust_exp() (or better even exp_adjust()) instead of adjustexp() (or expadjust()).

See Function and argument naming at https://devdevguide.netlify.app/pkg_building.html#function-and-argument-naming

[x] ml04. Can you please explain if you tried to include the example data in the package? I see some example data is created from scratch in README, in the vignette, and in many examples and tests (see also ml08). This is hard to maintain. If you later decide to use a different type of example data, doing that consistently would require changes in many places. I think somewhere you suggested the example data was too large to host in the package? This is something worth thinking about, since smaller test data correlates with more frequent testing and fewer bugs. Please explain your challenge and I'm more than happy to try help you find a solution.

Fit and overlap

[x] ml05. At https://github.com/ropensci/software-review/issues/659 I see: "This package was developed to share methodologies from existing research by modifying/expanding existing functions". Can you please explain where those existing functions live? I'd like to better understand the potential for overlap with some other package.

Tests

The tests could improve in a number of ways. I don't have first-had experience testing specifically spatial data so I'll try to find a reviewer who has (thanks for being proactive in https://discuss.ropensci.org/t/s4-data-for-testthat-and-examples/4007/5). For now I'll share some feedback that applies to tests in general.

[x] ml06. Please try to make each test run in under a second and let me know what challenges you face. Sometimes it's a matter of figuring out how to create a tiny toy dataset that allow you to test the properties you care about. For example, if you only care about the class of the output, the specific values of the output are irrelevant. This means that the test data can be unrealistically small AND useful, so you get useful and fast tests.

These two test-files are the slowest:

✔ |         20 | direxp [18.0s] 
✔ |          7 | multidirexp [87.1s]

If you trully can't make tests run faster, then you may move them to a dedicated folder e.g. tests/testthat/slow and run those tests less frequently than every other test -- with testthat::test_dir("tests/testthat/slow") (or testthat::test_dir(testthat::test_path("slow"))).

References: https://testthat.r-lib.org/reference/test_dir.html and https://testthat.r-lib.org/reference/test_path.html

[x] ml07. In tests, try to find ways to run verbose functions in quiet mode -- unless the verbose output IS what you want to test (e.g. expect_warning()), but in those cases the expectations typically swallow the verbose output so that the test-report remains decluttered.

Here are the verbose functions and the output I see clutering the test-report:

adjustexp: 
Proceed with caution: any adjustments to transmission distances should be further validated with observed fire history

direxp: 
<SpatRaster> resampled to 500554 cells.
Value object provided has more than one feature, only the first point or polygon will be used.

mapexpcont:
<SpatRaster> resampled to 501264 cells.

See https://ropensci.org/blog/2024/02/06/verbosity-control-packages/

[x] ml08. (Overlaps with ml04 but may come with it's own challenges) Please see if there is a way to deduplicate the generation of test data. This will make it easier to maintain the package. If the tst data is useful to users, then see if you can provide it with the package or via a separate data-package. If it's not useful for users, then you may create test data in some other Files relevant to testing e.g. (tests/testthat/setup.R). It can be hard to get your head around all these options. Have a look and let me know. I'm happy to help.

Specifially, I see this code-chunk multiple times:

# test data ====================================================================
set.seed(0)
e <- c(45,55,495,505) * 10000
r <- terra::rast(resolution = 100, extent = terra::ext(e))
terra::values(r) <- sample(c(0,1), terra::ncell(r), replace = TRUE)
r <- terra::sieve(r, threshold = 50, directions = 4)
haz <- terra::sieve(r, threshold = 500, directions = 4)

[x] ml09. Please try to be specific about the type of errors you expect, e.g. expect_error(f(), "calculation failed") is better than expect_error(f()).

Here is one example from the package:

  expect_error(direxp(2, pt))

Note that the second argument to expect_error() is a regular expression — the goal is to find a short fragment of text that matches the error you expect and is unlikely to match errors that you don’t expect. -- https://mastering-shiny.org/scaling-testing.html?q=expect_error#key-expectations

ml10. Suggestions based on the code chunk below:
- It's best to be specific, and use expect_message(), expect_warning() rather than the catch-all `expect_condition().
- It's best to use the regexp argument to ensure you get the specific contition you want to test.
- The last empty call to expect_condition() seems like a mistake?

test_that("mapexpclass() input checks and function messages work", {
  expect_condition(mapexplass(2, "loc", aoicrs)) # stopifnot() L67
  expect_condition(mapexpclass(expvals, "loc", aoicrs)) # stopifnot() L69
  expect_condition(mapexpclass(exp, "loc", 2)) # stopifnot() L71
  expect_condition(mapexpclass(exp, "loc", aoibig)) # stopifnot() L73
  expect_condition(mapexpclass(exp, "blah", aoicrs)) # matchargs() L75
  expect_condition()
})

ml11. It's best for a function to always return an object of the same type.

If a function is type-stable ... you can predict the output type based only on the input types (not their values). -- https://design.tidyverse.org/out-type-stability.html

The code chunk below shows that multidirexp() is not type-stable. To predict whether the output is a "data.frame" or a "ggplot" we need to know the value of the argument plot. Consider creating separate functions for returning a "data.frame" and a "ggplot". For example:

Option 1: data |> multidirexp() |> plot_multidirexp() Option 2: data |> multidirexp() |> plot() or data |> multidirexp() |> ggplot2::autoplot(). This requres understanding a little of the S3 object-oriented system (https://adv-r.hadley.nz/s3.html). Here are some examples using autoplot: https://forestgeo.github.io/fgeo.plot/reference/index.html

# https://github.com/heyairf/fireexposuR/blob/main/tests/testthat/test-multidirexp.R
test_that("multidirexp() returns object with correct class", {
  expect_s3_class(multidirexp(exp, pts, plot = T), "ggplot")
  expect_s3_class(multidirexp(exp, pts), "data.frame")
})

ml12. The comments are likely to fall out of sync with the code. It's best to make an effort to turn those comments into a formal testthat expectation.


test_that("summexp() input checks work", {
  expect_condition(summexp(2)) #exp: right class
  expect_condition(summexp(exp, 2)) #aoi: right class
  expect_condition(summexp(exp, aoi, "blah")) # classify: match arg
})

Project management

[x] ml13. Please explain if you think it might be possible to reduce the size of the git repo. The entire directory is ~77M but the .git/ directory alone is ~76M. This means the current files are ~1M in total. This indicates previous versions of the repository did have large files that are no longer necessary. Do you really need those large files in the history? Removing files from the history of a repo is hard-core and irreversible but maybe there is a different, better place to store those large files so that the repo is light (and therefore faster to clone).

# The fireexposuR directory is quite large, so cloning is relatively slow
➜  git du -hd 1 fireexposuR 
240K    fireexposuR/.Rproj.user
80K     fireexposuR/R
20K     fireexposuR/inst
76M     fireexposuR/.git
16K     fireexposuR/vignettes
24K     fireexposuR/.github
736K    fireexposuR/man
48K     fireexposuR/tests
77M     fireexposuR

# Almost all that "weight" comes from the .git/ directory alone
➜  git du -hd 1 fireexposuR/.git
4.0K    fireexposuR/.git/branches
75M     fireexposuR/.git/objects
32K     fireexposuR/.git/logs
8.0K    fireexposuR/.git/info
28K     fireexposuR/.git/refs
64K     fireexposuR/.git/hooks
76M     fireexposuR/.git

`@ropensci-bot` check

ml14. The checks from the ropensci-bot show some noteworthy properties that may help find potential areas of improvement, such as removing unused dependencies, splitting or simplifying large or complex functions. In particular, the goodpractice results finds a call to the pattern 1:length(...) that is best replaced with a call to seq_len().

Next, beware of iterating over 1:length(x), which will fail in unhelpful ways if x has length 0. -- https://adv-r.hadley.nz/control-flow.html?q=1:leng#common-pitfalls

Oct 10 '24 13:10 maurolepore

Hi @maurolepore,

Thank you so much for such detailed feedback! I will work on addressing these comments over the next few days and ask for further clarification if I need to. I very much appreciate all the resources you've included.

Oct 15 '24 19:10 heyairf

Hi @maurolepore,

I have attempted to addressed your feedback within my package! My replies to each of your comments are below. I expect that some may need to be addressed further, please let me know.

ml01

The accompanying paper is currently in preparation and will be submitted after the R package review is complete so examples in the paper align with the r package after it has been reviewed. I have clarified this in the README and will be sure to update with the new citation when it is published. I have also added a short description of the three papers that are linked, thank you for your suggestion!

ml02

Thank you for the links. I have added a website for the package following your instructions!

ml03

I have changed the function names so they use snake case and standardized all names to a prefixed “fire_exp”. Some functions have been split into more than one to address your feedback comment ml11. I have not officially grouped them, but now the function names organize them in a more meaningful order.

ml04/ml08

This has absolutely been my biggest challenge in this process that I have struggled to find help with the most. Originally I tried to use real data that had very large file sizes but soon found that that would not be an option which is why I switched to generating the example data with code. It was most challenging to figure out because of the spatial data types and inability to save them as .Rdata files like almost every example on the internet. I have made some more changes to address this based on all your very helpful feedback! I am sure there is still lots of room for improvement, but I have now saved the main example data as a .tif file in inst/extdata instead of regenerating it on the fly in every single example and test document.

ml05

My apologies, to clarify, the existing functions in question were made and used by only me. I have been using them in my own research and to save myself time when helping others run assessments. The edits and modifications I made were to make them more robust to share with a larger audience via the R package (i.e. the documentation and input checks).

ml06

With your feedback I believe I have now reduced the testing time to under 4 seconds for each function while still maintaining test coverage. I will continue to think about ways to reduce the size/extent of the test data to speed these up further.

ml07

I have added suppressMessages() to the few tests that output non-error messages to remove the clutter.

ml08

see reply to ml04

ml09/ml10

This feedback was so helpful for my overall understanding of how tests should be designed! I believe I have made significant improvements to address this in all of the test files.

ml11

I have split some functions to address this, but perhaps it can be split further still. For example, fire_exp_dir() now returns a spatial feature, that can then be used in fire_exp_dir_plot() which has the option to return a plot or a map.

ml12

The comments have been removed now that the expected error messages (from feedback at ml09/ml10) are more meaningful

ml13

You are absolutely correct that there was originally large files saved with the package for the examples in early stages of the project. I will remove them from the repo this week with some support from the research and training support at my university.

ml14

I have updated this line to use seq_len()

Oct 21 '24 19:10 heyairf

@ropensci-review-bot check package

Oct 24 '24 18:10 maurolepore

Thanks, about to send the query.

Oct 24 '24 18:10 ropensci-review-bot

@heyairf thanks so much for your effort and your quick response. Great work.

I'll start reaching out to potential reviewers.

ml13 Great you're taking initiative. For now I suggest to keep this repo as is. Before you push a re-written Git repo to GitHub I would like to first discuss with the editor board to gather alternative opinions. Until it seems prudent to practice in a local copy of the repo.

Also ...

ml15. In the tests I see this warning:

Warning (test-fire_exp_extract_vis.R:43:3): fire_exp_extract_vis() runs when input conditions are met
Use of .data in tidyselect expressions was deprecated in tidyselect 1.2.0.
i Please use `"class"` instead of `.data$class`

The problem is in R/fire_exp_extract_vis.R#L137. You can fix using use "class" instead of .data$class.

      tidyr::drop_na("class")

[ ] Can you please confirm the potential reviewers you listed above do not have a conflic of interest? If that eliminates someone from your list, please replace them with a new suggestion. Also, in any case, it's handy to have their GitHub handle.

Oct 24 '24 19:10 maurolepore

Sorry @maurolepore and @heyairf , the check package service is currently not working. It'll be back online in about 12 hours, at which time I'll make sure that request gets processed. Thanks

Oct 24 '24 19:10 mpadge

@heyairf

ml13. I already discussed with the editorial board and we agree in that removing the large files would be beneficial, particularly before we engage the reviewers.

Here are some tools you may consider:

https://github.com/newren/git-filter-repo/
https://github.com/rtyley/bfg-repo-cleaner

Understanding the goal and knowing about a couple of tools should help you find examples and better support. If that's insufficient please let me know and I'm happy to help.

Remember to backup your original repo and play with copies of it until you're comfortable with the result. If

BTW

@mpadge kindly shared a script to detect the largest files in a repository

On fireexposuR I get this output:

$ git rev-list main | while read rev; do git ls-tree -lr $rev  | cut -c54- | sed -r 's/^ +//g;'; done  | sort -u | perl -e 'while (<>) { chomp; @stuff=split("\t");$sums{$stuff[1]} += $stuff[0];} print "$sums{$_} $_\n" for (keys %sums);' | sort -rn | head -n 20
71482479 inst/extdata/LExpAB2020.tif
8969108 inst/extdata/LHazAB2020.tif
4976880 inst/extdata/nonburnableAB2020.tif
861294 inst/extdata/forR.gdb/a0000000c.gdbtable
703342 man/figures/README-maplandscape-1.png
680608 inst/extdata/fpa.shp
658836 inst/extdata/WHITstruc.shp
645400 inst/extdata/WHITstruc.dbf
555028 inst/extdata/forR.gdb/a00000010.gdbtable
474724 inst/extdata/forR.gdb/a00000014.gdbtable
360794 inst/extdata/forR.gdb/a00000004.gdbtable
213432 inst/extdata/hazard.tif
204820 inst/extdata/WHITbuilt.shp
163292 inst/extdata/forestarea.shp
162201 R/direxp.R
159766 inst/extdata/forR.gdb/a00000010.spx
155670 inst/extdata/forR.gdb/a00000004.spx
142837 man/figures/README-aoivis-1.png
135190 inst/extdata/forR.gdb/a00000014.spx
109006 man/figures/README-mapdir-1.png

Oct 25 '24 08:10 maurolepore

@ropensci-review-bot check package

Oct 25 '24 09:10 maurolepore

Thanks, about to send the query.

Oct 25 '24 09:10 ropensci-review-bot

Checks for fireexposuR (v1.0.1)

git hash: 01b6686c

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

Package License: GPL (>= 3)

1. 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	150
internal	grid	12
internal	stats	12
internal	fireexposuR	8
internal	utils	8
internal	graphics	5
internal	grDevices	4
imports	magrittr	100
imports	ggplot2	53
imports	dplyr	51
imports	terra	47
imports	tidyterra	17
imports	geosphere	8
imports	maptiles	8
imports	tidyr	5
imports	ggspatial	4
imports	MultiscaleDTM	2
imports	rlang	NA
imports	tidyselect	NA
suggests	knitr	NA
suggests	rmarkdown	NA
suggests	testthat	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.

base

c (25), exp (20), sum (11), cbind (9), data.frame (9), as.data.frame (8), paste (7), ifelse (5), by (4), levels (4), matrix (4), ncol (4), factor (3), labels (3), rep (3), seq (3), all (2), class (2), match (2), max (2), mean (2), names (2), round (2), unique (2), as.factor (1), drop (1), for (1), if (1), missing (1), nrow (1), outer (1), rbind (1), scale (1), seq_len (1), t (1), table (1)

magrittr

%>% (100)

ggplot2

element_blank (9), aes (8), ggplot (8), element_text (5), coord_sf (4), geom_col (3), labs (3), element_rect (2), expansion (2), scale_fill_manual (2), geom_bar (1), geom_hline (1), geom_vline (1), scale_color_manual (1), scale_x_discrete (1), scale_y_continuous (1), theme (1)

dplyr

mutate (42), count (4), rename (3), case_when (1), summarise (1)

terra

classify (7), crop (6), extract (5), focal (4), mask (4), project (4), vect (4), res (3), rescale (3), spatSample (2), as.polygons (1), buffer (1), crs (1), merge (1), perim (1)

tidyterra

geom_spatraster_rgb (6), geom_spatvector (3), whitebox.colors (3), rename (2), filter (1), geom_spatraster (1), scale_fill_whitebox_c (1)

grid

unit (12)

stats

df (6), window (6)

fireexposuR

fire_exp_dir (2), fire_exp (1), fire_exp_adjust (1), fire_exp_dir_multi (1), fire_exp_dir_plot (1), fire_exp_extract (1), fire_exp_extract_vis (1)

geosphere

destPoint (8)

maptiles

get_tiles (5), get_credit (3)

utils

data (8)

graphics

title (5)

tidyr

pivot_wider (4), pivot_longer (1)

ggspatial

annotation_scale (4)

grDevices

palette (4)

MultiscaleDTM

annulus_window (2)

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

2. 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 13 files) and
2 authors
1 vignette
no internal data file
12 imported packages
11 exported functions (median 52 lines of code)
11 non-exported functions in R (median 76 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	13	66.0
files_vignettes	1	61.9
files_tests	11	88.9
loc_R	823	60.0
loc_vignettes	110	26.9
loc_tests	306	61.1
num_vignettes	1	58.8
n_fns_r	22	30.4
n_fns_r_exported	11	48.6
n_fns_r_not_exported	11	24.7
n_fns_per_file_r	1	1.9	TRUE
num_params_per_fn	3	29.3
loc_per_fn_r	60	92.6
loc_per_fn_r_exp	52	80.4
loc_per_fn_r_not_exp	76	95.3	TRUE
rel_whitespace_R	12	50.3
rel_whitespace_vignettes	34	26.2
rel_whitespace_tests	22	60.9
doclines_per_fn_exp	39	48.4
doclines_per_fn_not_exp	0	0.0	TRUE
fn_call_network_size	1	11.5

2a. Network visualisation

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

3. `goodpractice` and other checks

Details of goodpractice checks (click to open)

3a. Continuous Integration Badges

GitHub Workflow Results

id	name	conclusion	sha	run_number	date
11492044488	pages build and deployment	success	d24548	10	2024-10-24
11492024343	pkgcheck	success	01b668	20	2024-10-24
11492024335	pkgdown.yaml	success	01b668	9	2024-10-24
11492024339	R-CMD-check.yaml	success	01b668	21	2024-10-24

3b. `goodpractice` results

`R CMD check` with rcmdcheck

rcmdcheck found no errors, warnings, or notes

Test coverage with covr

Package coverage: 76.81

Cyclocomplexity with cyclocomp

No functions have cyclocomplexity >= 15

Static code analyses with lintr

lintr found no issues with this package!

Package Versions

package	version
pkgstats	0.2.0
pkgcheck	0.1.2.61

Editor-in-Chief Instructions:

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

Oct 25 '24 09:10 ropensci-review-bot

@ropensci-review-bot seeking reviewers

Oct 25 '24 12:10 maurolepore

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

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

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

Oct 25 '24 12:10 ropensci-review-bot

fireexposuR: Compute and Visualize Wildfire Exposure

Scope

Technical checks

Publication options

Code of conduct

Checks for fireexposuR (v1.0.0)

1. Package Dependencies

2. Statistical Properties

2a. Network visualisation

3. goodpractice and other checks

3a. Continuous Integration Badges

3b. goodpractice results

R CMD check with rcmdcheck

Test coverage with covr

Cyclocomplexity with cyclocomp

Static code analyses with lintr

4. Other Checks

Editor-in-Chief Instructions:

Checks for fireexposuR (v1.0.0)

1. Package Dependencies

2. Statistical Properties

2a. Network visualisation

3. goodpractice and other checks

3a. Continuous Integration Badges

3b. goodpractice results

R CMD check with rcmdcheck

Test coverage with covr

Cyclocomplexity with cyclocomp

Static code analyses with lintr

4. Other Checks

Editor-in-Chief Instructions:

Semantic tags for my comments

Reviewers

Editor checks

Editor checks:

Editor comments

Documentation

Fit and overlap

Tests

Project management

@ropensci-bot check

Checks for fireexposuR (v1.0.1)

1. Package Dependencies

2. Statistical Properties

2a. Network visualisation

3. goodpractice and other checks

3a. Continuous Integration Badges

3b. goodpractice results

R CMD check with rcmdcheck

Test coverage with covr

Cyclocomplexity with cyclocomp

Static code analyses with lintr

Editor-in-Chief Instructions:

3. `goodpractice` and other checks

3b. `goodpractice` results

`R CMD check` with rcmdcheck

3. `goodpractice` and other checks

3b. `goodpractice` results

`R CMD check` with rcmdcheck

`@ropensci-bot` check

3. `goodpractice` and other checks

3b. `goodpractice` results

`R CMD check` with rcmdcheck