-
-
Notifications
You must be signed in to change notification settings - Fork 104
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
waywiser: Ergonomic Methods for Assessing Spatial Models #571
Comments
Thanks for submitting to rOpenSci, our editors and @ropensci-review-bot will reply soon. Type |
🚀 The following problem was found in your submission template:
👋 |
Sorry! The instructions at the top of the issue told me to not change anything other than the repo URL and GitHub handles -- this might be something to update in the issue template: software-review/.github/ISSUE_TEMPLATE/F-submit-statistical-software-for-review.md Line 8 in 6c9722f
|
Note: The following R packages were unable to be installed/upgraded on our system: [tigris, spatialsample, spdep]; some checks may be unreliable. |
Oops, something went wrong with our automatic package checks. Our developers have been notified and package checks will appear here as soon as we've resolved the issue. Sorry for any inconvenience. |
Checks for waywiser (v0.2.0.9000)git hash: b8816249
Important: All failing checks above must be addressed prior to proceeding Package License: MIT + file LICENSE 1. rOpenSci Statistical Standards (
|
type | package | ncalls |
---|---|---|
internal | waywiser | 81 |
internal | base | 80 |
internal | utils | 33 |
internal | graphics | 2 |
imports | stats | 42 |
imports | yardstick | 23 |
imports | rlang | 11 |
imports | purrr | 7 |
imports | spdep | 4 |
imports | hardhat | 3 |
imports | sf | 3 |
imports | glue | 2 |
imports | rsample | 2 |
imports | tidyselect | 2 |
imports | dplyr | 1 |
imports | fields | 1 |
imports | FNN | 1 |
imports | Matrix | 1 |
imports | tibble | 1 |
suggests | applicable | NA |
suggests | caret | NA |
suggests | CAST | NA |
suggests | covr | NA |
suggests | ggplot2 | NA |
suggests | knitr | NA |
suggests | modeldata | NA |
suggests | recipes | NA |
suggests | rmarkdown | NA |
suggests | spatialsample | NA |
suggests | spelling | NA |
suggests | testthat | NA |
suggests | tidymodels | NA |
suggests | tidyr | NA |
suggests | tigris | NA |
suggests | vip | NA |
suggests | whisker | NA |
suggests | withr | NA |
linking_to | NA | NA |
Click below for tallies of functions used in each package. Locations of each call within this package may be generated locally by running 's <- pkgstats::pkgstats(<path/to/repo>)', and examining the 'external_calls' table.
waywiser
calc_ssd (4), check_for_missing (3), gmfr (3), calc_spdu (2), calc_spod (2), ww_area_of_applicability (2), calc_aoa (1), calc_d_bar (1), calc_di (1), calc_spds (1), check_di_columns_numeric (1), check_di_importance (1), check_di_testing (1), create_aoa (1), expand_grid (1), is_longlat (1), predict.ww_area_of_applicability (1), print.ww_area_of_applicability (1), spatial_yardstick_df (1), spatial_yardstick_vec (1), standardize_and_weight (1), tidy_importance (1), tidy_importance.data.frame (1), tidy_importance.default (1), tidy_importance.vi (1), ww_agreement_coefficient_impl (1), ww_agreement_coefficient_vec (1), ww_agreement_coefficient.data.frame (1), ww_area_of_applicability.data.frame (1), ww_area_of_applicability.default (1), ww_area_of_applicability.formula (1), ww_area_of_applicability.rset (1), ww_build_neighbors (1), ww_build_weights (1), ww_global_geary_c_impl (1), ww_global_geary_c_vec (1), ww_global_geary_c.data.frame (1), ww_global_geary_pvalue_impl (1), ww_global_geary_pvalue_vec (1), ww_global_geary_pvalue.data.frame (1), ww_global_moran_i_impl (1), ww_global_moran_i_vec (1), ww_global_moran_i.data.frame (1), ww_global_moran_pvalue_impl (1), ww_global_moran_pvalue_vec (1), ww_global_moran_pvalue.data.frame (1), ww_local_geary_c_impl (1), ww_local_geary_c_vec (1), ww_local_geary_c.data.frame (1), ww_local_geary_pvalue_impl (1), ww_local_geary_pvalue_vec (1), ww_local_geary_pvalue.data.frame (1), ww_local_getis_ord_g_impl (1), ww_local_getis_ord_g_pvalue_vec (1), ww_local_getis_ord_g_pvalue.data.frame (1), ww_local_getis_ord_g_vec (1), ww_local_getis_ord_g.data.frame (1), ww_local_getis_ord_pvalue_impl (1), ww_local_moran_i_impl (1), ww_local_moran_i_vec (1), ww_local_moran_i.data.frame (1), ww_local_moran_pvalue_impl (1), ww_local_moran_pvalue_vec (1), ww_local_moran_pvalue.data.frame (1), ww_make_point_neighbors (1), ww_make_polygon_neighbors (1), ww_multi_scale (1), ww_systematic_agreement_coefficient_impl (1), ww_systematic_agreement_coefficient_vec (1), ww_systematic_agreement_coefficient.data.frame (1), ww_systematic_mpd.data.frame (1)
base
c (10), call (7), data.frame (7), mean (7), list (6), class (4), sum (4), if (3), nrow (3), abs (2), all (2), identical (2), inherits (2), is.na (2), names (2), unlist (2), any (1), character (1), drop (1), get (1), integer (1), length (1), missing (1), ncol (1), paste0 (1), round (1), seq_len (1), setdiff (1), sign (1), sqrt (1), unique (1)
stats
resid (20), dt (10), na.fail (6), lm (2), predict (2), complete.cases (1), cor (1)
utils
data (33)
yardstick
new_numeric_metric (23)
rlang
caller_env (6), exec (2), expr (2), list2 (1)
purrr
map (4), chuck (1), map_dbl (1), map_lgl (1)
spdep
knearneigh (1), localC_perm (1), localG_perm (1), Szero (1)
hardhat
mold (2), default_formula_blueprint (1)
sf
st_bbox (1), st_geometry_type (1), st_intersects (1)
glue
glue (2)
graphics
grid (2)
rsample
analysis (1), assessment (1)
tidyselect
eval_select (2)
dplyr
summarise (1)
fields
rdist (1)
FNN
knn.dist (1)
Matrix
mean (1)
tibble
tibble (1)
3. Statistical Properties
This package features some noteworthy statistical properties which may need to be clarified by a handling editor prior to progressing.
Details of statistical properties (click to open)
The package has:
- code in R (100% in 15 files) and
- 1 authors
- 3 vignettes
- no internal data file
- 15 imported packages
- 95 exported functions (median 3 lines of code)
- 173 non-exported functions in R (median 11 lines of code)
Statistical properties of package structure as distributional percentiles in relation to all current CRAN packages
The following terminology is used:
loc
= "Lines of Code"fn
= "function"exp
/not_exp
= exported / not exported
All parameters are explained as tooltips in the locally-rendered HTML version of this report generated by the checks_to_markdown()
function
The final measure (fn_call_network_size
) is the total number of calls between functions (in R), or more abstract relationships between code objects in other languages. Values are flagged as "noteworthy" when they lie in the upper or lower 5th percentile.
measure | value | percentile | noteworthy |
---|---|---|---|
files_R | 15 | 73.0 | |
files_vignettes | 3 | 92.4 | |
files_tests | 39 | 98.8 | |
loc_R | 1602 | 79.6 | |
loc_vignettes | 345 | 68.1 | |
loc_tests | 6814 | 98.8 | TRUE |
num_vignettes | 3 | 94.2 | |
n_fns_r | 268 | 93.1 | |
n_fns_r_exported | 95 | 95.0 | TRUE |
n_fns_r_not_exported | 173 | 91.9 | |
n_fns_per_file_r | 9 | 84.5 | |
num_params_per_fn | 4 | 54.6 | |
loc_per_fn_r | 9 | 24.3 | |
loc_per_fn_r_exp | 3 | 1.5 | TRUE |
loc_per_fn_r_not_exp | 11 | 35.4 | |
rel_whitespace_R | 18 | 79.7 | |
rel_whitespace_vignettes | 22 | 54.6 | |
rel_whitespace_tests | 19 | 98.9 | TRUE |
doclines_per_fn_exp | 109 | 92.6 | |
doclines_per_fn_not_exp | 0 | 0.0 | TRUE |
fn_call_network_size | 99 | 79.1 |
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
GitHub Workflow Results
id | name | conclusion | sha | run_number | date |
---|---|---|---|---|---|
3898338974 | Lock Threads | success | b88162 | 199 | 2023-01-12 |
3744601924 | pages build and deployment | success | a23b40 | 50 | 2022-12-20 |
3744561926 | pkgdown | success | b88162 | 118 | 2022-12-20 |
3744561927 | R-CMD-check | success | b88162 | 116 | 2022-12-20 |
3744561920 | R-CMD-check-hard | success | b88162 | 112 | 2022-12-20 |
3744561918 | test-coverage | success | b88162 | 116 | 2022-12-20 |
3b. goodpractice
results
R CMD check
with rcmdcheck
R CMD check generated the following error:
- Error in proc$get_built_file() : Build process failed
Test coverage with covr
ERROR: Test Coverage Failed
Cyclocomplexity with cyclocomp
Error : Build failed, unknown error, standard output:
�[33m* checking for file ‘waywiser/DESCRIPTION’ ... OK
- preparing ‘waywiser’:
- checking DESCRIPTION meta-information ... OK
- installing the package to build vignettes
- creating vignettes ... ERROR
--- re-building ‘multi-scale-assessment.Rmd’ using rmarkdown
Quitting from lines 52-58 (multi-scale-assessment.Rmd)
Error: processing vignette 'multi-scale-assessment.Rmd' failed with diagnostics:
OGRCreateCoordinateTransformation(): transformation not available
--- failed re-building ‘multi-scale-assessment.Rmd’
--- re-building ‘residual-autocorrelation.Rmd’ using rmarkdown
Quitting from lines 78-94 (residual-autocorrelation.Rmd)
Error: processing vignette 'residual-autocorrelation.Rmd' failed with diagnostics:
OGRCreateCoordinateTransformation(): transformation not available
--- failed re-building ‘residual-autocorrelation.Rmd’
--- re-building ‘waywiser.Rmd’ using rmarkdown
--- finished re-building ‘waywiser.Rmd’
SUMMARY: processing the following files failed:
‘multi-scale-assessment.Rmd’ ‘residual-autocorrelation.Rmd’
Error: Vignette re-building failed.
Execution halted
double free or corruption (out)
Aborted (core dumped)
�[39m
Static code analyses with lintr
lintr found the following 601 potential issues:
message | number of times |
---|---|
Avoid library() and require() calls in packages | 14 |
Lines should not be more than 80 characters. | 585 |
unexpected input | 2 |
Package Versions
package | version |
---|---|
pkgstats | 0.1.3 |
pkgcheck | 0.1.0.32 |
srr | 0.0.1.186 |
Editor-in-Chief Instructions:
Processing may not proceed until the items marked with ✖️ have been resolved.
Hello @mikemahoney218 , The failures shown on the checks appear to be genuine and are pointing to some issues in the package. We encourage you to try and reproduce in a clean docker container by cloning the repo and running With respect to messages from statistical checks by |
BTW @mikemahoney218, regarding testing locally in a docker container, this section of our devguide has some useful links to info (just in case). |
@mikemahoney218 @annakrystalli The |
Hi @annakrystalli & @mpadge -- can I ask if there's more information about your CI server available anywhere? I'm wondering what results you get from I can't reproduce the issue on CRAN: On CI: I notice that your link suggests the R-Hub docker images; it has been my experience that R-Hub has not been able to install most spatial software for a few years now. I checked using the rocker images, via the command: docker run --rm -ti -v "$(pwd)":/home/rstudio rocker/geospatial R (The volume attaches my code folder as the home directory in order to check the package.) So it seems like I'm not able to reproduce this issue across a variety of environments. |
@mikemahoney218 It's our own docker image used specifically for package checks. Current version gives this: sf::sf_extSoftVersion()
#> GEOS GDAL proj.4 GDAL_with_GEOS USE_PROJ_H
#> "3.8.0" "3.0.4" "6.3.1" "true" "true"
#> PROJ
#> "6.3.1" Created on 2023-01-12 with reprex v2.0.2 ... but i can confirm that the issue is directly caused by |
Thanks! I think there's still likely going to be an issue from using PROJ 6 -- the vignettes assume you've got access to the PROJ CDN, which I believe was a PROJ 7/2020-release feature, so the resulting vignettes may be odd -- but it shouldn't segfault; glad to hear you've caught it. |
@ropensci-review-bot check package |
Thanks, about to send the query. |
🚀 Editor check started 👋 |
Note: The following R packages were unable to be installed/upgraded on our system: [tigris, spatialsample, spdep]; some checks may be unreliable. |
Hi @annakrystalli ! I'm not sure how long the checks should take, but we're a bit past two hours now. I believe I've fixed the srr issue, and it sounds like fixing the CI system may take a while, but the package works on non-rOpenSci systems. |
@mikemahoney218 The comment above was intended to imply that checks for your package would not work until the problem was rectified. As said,
But given that you've already called the checks, i'll just get them to dump updated versions here when they're done. Please bear with us, as this could take a few days to get around to. |
Ah sorry, I had assumed the package checks would just fail again and I'd be able to get the bot to verify I'd finished the srr. Apologies! |
Checks for waywiser (v0.2.0.9000)git hash: 6c57cc85
Package License: MIT + file LICENSE 1. rOpenSci Statistical Standards (
|
type | package | ncalls |
---|---|---|
internal | waywiser | 81 |
internal | base | 80 |
internal | utils | 33 |
internal | graphics | 2 |
imports | stats | 42 |
imports | yardstick | 23 |
imports | rlang | 11 |
imports | purrr | 7 |
imports | spdep | 4 |
imports | hardhat | 3 |
imports | sf | 3 |
imports | glue | 2 |
imports | rsample | 2 |
imports | tidyselect | 2 |
imports | dplyr | 1 |
imports | fields | 1 |
imports | FNN | 1 |
imports | Matrix | 1 |
imports | tibble | 1 |
suggests | applicable | NA |
suggests | caret | NA |
suggests | CAST | NA |
suggests | covr | NA |
suggests | ggplot2 | NA |
suggests | knitr | NA |
suggests | modeldata | NA |
suggests | recipes | NA |
suggests | rmarkdown | NA |
suggests | spatialsample | NA |
suggests | spelling | NA |
suggests | testthat | NA |
suggests | tidymodels | NA |
suggests | tidyr | NA |
suggests | tigris | NA |
suggests | vip | NA |
suggests | whisker | NA |
suggests | withr | NA |
linking_to | NA | NA |
Click below for tallies of functions used in each package. Locations of each call within this package may be generated locally by running 's <- pkgstats::pkgstats(<path/to/repo>)', and examining the 'external_calls' table.
waywiser
calc_ssd (4), check_for_missing (3), gmfr (3), calc_spdu (2), calc_spod (2), ww_area_of_applicability (2), calc_aoa (1), calc_d_bar (1), calc_di (1), calc_spds (1), check_di_columns_numeric (1), check_di_importance (1), check_di_testing (1), create_aoa (1), expand_grid (1), is_longlat (1), predict.ww_area_of_applicability (1), print.ww_area_of_applicability (1), spatial_yardstick_df (1), spatial_yardstick_vec (1), standardize_and_weight (1), tidy_importance (1), tidy_importance.data.frame (1), tidy_importance.default (1), tidy_importance.vi (1), ww_agreement_coefficient_impl (1), ww_agreement_coefficient_vec (1), ww_agreement_coefficient.data.frame (1), ww_area_of_applicability.data.frame (1), ww_area_of_applicability.default (1), ww_area_of_applicability.formula (1), ww_area_of_applicability.rset (1), ww_build_neighbors (1), ww_build_weights (1), ww_global_geary_c_impl (1), ww_global_geary_c_vec (1), ww_global_geary_c.data.frame (1), ww_global_geary_pvalue_impl (1), ww_global_geary_pvalue_vec (1), ww_global_geary_pvalue.data.frame (1), ww_global_moran_i_impl (1), ww_global_moran_i_vec (1), ww_global_moran_i.data.frame (1), ww_global_moran_pvalue_impl (1), ww_global_moran_pvalue_vec (1), ww_global_moran_pvalue.data.frame (1), ww_local_geary_c_impl (1), ww_local_geary_c_vec (1), ww_local_geary_c.data.frame (1), ww_local_geary_pvalue_impl (1), ww_local_geary_pvalue_vec (1), ww_local_geary_pvalue.data.frame (1), ww_local_getis_ord_g_impl (1), ww_local_getis_ord_g_pvalue_vec (1), ww_local_getis_ord_g_pvalue.data.frame (1), ww_local_getis_ord_g_vec (1), ww_local_getis_ord_g.data.frame (1), ww_local_getis_ord_pvalue_impl (1), ww_local_moran_i_impl (1), ww_local_moran_i_vec (1), ww_local_moran_i.data.frame (1), ww_local_moran_pvalue_impl (1), ww_local_moran_pvalue_vec (1), ww_local_moran_pvalue.data.frame (1), ww_make_point_neighbors (1), ww_make_polygon_neighbors (1), ww_multi_scale (1), ww_systematic_agreement_coefficient_impl (1), ww_systematic_agreement_coefficient_vec (1), ww_systematic_agreement_coefficient.data.frame (1), ww_systematic_mpd.data.frame (1)
base
c (10), call (7), data.frame (7), mean (7), list (6), class (4), sum (4), if (3), nrow (3), abs (2), all (2), identical (2), inherits (2), is.na (2), names (2), unlist (2), any (1), character (1), drop (1), get (1), integer (1), length (1), missing (1), ncol (1), paste0 (1), round (1), seq_len (1), setdiff (1), sign (1), sqrt (1), unique (1)
stats
resid (20), dt (10), na.fail (6), lm (2), predict (2), complete.cases (1), cor (1)
utils
data (33)
yardstick
new_numeric_metric (23)
rlang
caller_env (6), exec (2), expr (2), list2 (1)
purrr
map (4), chuck (1), map_dbl (1), map_lgl (1)
spdep
knearneigh (1), localC_perm (1), localG_perm (1), Szero (1)
hardhat
mold (2), default_formula_blueprint (1)
sf
st_bbox (1), st_geometry_type (1), st_intersects (1)
glue
glue (2)
graphics
grid (2)
rsample
analysis (1), assessment (1)
tidyselect
eval_select (2)
dplyr
summarise (1)
fields
rdist (1)
FNN
knn.dist (1)
Matrix
mean (1)
tibble
tibble (1)
3. Statistical Properties
This package features some noteworthy statistical properties which may need to be clarified by a handling editor prior to progressing.
Details of statistical properties (click to open)
The package has:
- code in R (100% in 15 files) and
- 1 authors
- 3 vignettes
- no internal data file
- 15 imported packages
- 95 exported functions (median 3 lines of code)
- 173 non-exported functions in R (median 11 lines of code)
Statistical properties of package structure as distributional percentiles in relation to all current CRAN packages
The following terminology is used:
loc
= "Lines of Code"fn
= "function"exp
/not_exp
= exported / not exported
All parameters are explained as tooltips in the locally-rendered HTML version of this report generated by the checks_to_markdown()
function
The final measure (fn_call_network_size
) is the total number of calls between functions (in R), or more abstract relationships between code objects in other languages. Values are flagged as "noteworthy" when they lie in the upper or lower 5th percentile.
measure | value | percentile | noteworthy |
---|---|---|---|
files_R | 15 | 73.0 | |
files_vignettes | 3 | 92.4 | |
files_tests | 39 | 98.8 | |
loc_R | 1602 | 79.6 | |
loc_vignettes | 345 | 68.1 | |
loc_tests | 6814 | 98.8 | TRUE |
num_vignettes | 3 | 94.2 | |
n_fns_r | 268 | 93.1 | |
n_fns_r_exported | 95 | 95.0 | TRUE |
n_fns_r_not_exported | 173 | 91.9 | |
n_fns_per_file_r | 9 | 84.5 | |
num_params_per_fn | 4 | 54.6 | |
loc_per_fn_r | 9 | 24.3 | |
loc_per_fn_r_exp | 3 | 1.5 | TRUE |
loc_per_fn_r_not_exp | 11 | 35.4 | |
rel_whitespace_R | 18 | 79.7 | |
rel_whitespace_vignettes | 22 | 54.6 | |
rel_whitespace_tests | 19 | 98.9 | TRUE |
doclines_per_fn_exp | 109 | 92.6 | |
doclines_per_fn_not_exp | 0 | 0.0 | TRUE |
fn_call_network_size | 99 | 79.1 |
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
GitHub Workflow Results
id | name | conclusion | sha | run_number | date |
---|---|---|---|---|---|
3898338974 | Lock Threads | success | b88162 | 199 | 2023-01-12 |
3903744451 | pages build and deployment | success | d4d305 | 51 | 2023-01-12 |
3903687426 | pkgdown | success | 6c57cc | 121 | 2023-01-12 |
3903687434 | R-CMD-check | success | 6c57cc | 119 | 2023-01-12 |
3903687436 | R-CMD-check-hard | success | 6c57cc | 115 | 2023-01-12 |
3903687429 | test-coverage | success | 6c57cc | 119 | 2023-01-12 |
3b. goodpractice
results
R CMD check
with rcmdcheck
R CMD check generated the following notes:
- checking Rd cross-references ... NOTE
Packages unavailable to check Rd xrefs: ‘raster’, ‘terra’ - checking data for non-ASCII characters ... NOTE
Note: found 1 marked UTF-8 string
R CMD check generated the following check_fail:
- rcmdcheck_non_ascii_characters_in_data
Test coverage with covr
Package coverage: 100
Cyclocomplexity with cyclocomp
Error : �[4m�[33m
Build failed, standard output:
�[39m�[24m�[33m* checking for file ‘waywiser/DESCRIPTION’ ... OK
- preparing ‘waywiser’:
- checking DESCRIPTION meta-information ... OK
- installing the package to build vignettes
- creating vignettes ... OK
- checking for LF line-endings in source and make files and shell scripts
- checking for empty or unneeded directories
- re-saving .R files as .rda
�[39m
�[4m�[31mStandard error:
�[39m�[24m�[31mError in loadNamespace(x) : there is no package called ‘waywiser’
Execution halted
�[39m
Static code analyses with lintr
lintr found the following 603 potential issues:
message | number of times |
---|---|
Avoid library() and require() calls in packages | 14 |
Lines should not be more than 80 characters. | 587 |
unexpected input | 2 |
Package Versions
package | version |
---|---|
pkgstats | 0.1.3 |
pkgcheck | 0.1.1.3 |
srr | 0.0.1.188 |
Editor-in-Chief Instructions:
This package is in top shape and may be passed on to a handling editor
@ropensci-review-bot assign @Paula-Moraga as editor |
Assigned! @Paula-Moraga is now the editor |
Couldn't find entry for becarioprecario in the reviews log |
@ropensci-review-bot submit review #571 (comment) time 3 |
Couldn't find entry for Nowosad in the reviews log |
Thank you for your comments, @Nowosad ! I believe I've addressed all your comments in the current development version of the package. I've responded to your points with a bit more detail below.
I moved it to be a standalone article (so on the pkgdown site, but not built on CRAN), which also let me move kableExtra out of Suggests. (Commit, rendered)
The main reason is because this package supports R >= 4, while the pipe was added in 4.1. Using the native pipe would make CI runs for R 4.0 fail, or only be run conditionally; in order for the vignettes to build on every supported version of R, I've kept the %>% pipe for now.
I added a long discussion about what this means at the top of the vignette, and also elaborated a tiny bit at the end. (Commit, rendered)
These should be "fixed" for now, by tripping the new lifecycle warning in dplyr::summarise() at the top of each test file. The long-term fix will depend on if yardstick changes to use
Thank you! I've gone ahead and added this documentation throughout. (Example of commit, example of rendered)
I'm not sure I know what spatial explainers are! That said, this is my broad vision for the package:
With all that said, I'm not actively looking for things to add -- I'm currently only adding things that are useful for my own work. But if there are requests or PRs for other features, that's my basic outline for whether something belongs in waywiser or not. Hope that answers the question!
Thanks! I'll add you as a reviewer in the DESCRIPTION once the package is accepted 😄 |
@Paula-Moraga I recorded the reviews information, sorry about the glitch. |
@mikemahoney218 thanks for all of the improvements made.
I've been thinking of something like https://geods.netlify.app/post/spatial-ml-model-diagnostics/. |
@Nowosad I think that would be in scope for waywiser. I'm not planning on implementing it in the near future (I need to focus on my dissertation 😅, so all the techniques I'm actively adding are things I'm going to use myself), but I could definitely see the package growing in that direction over time. |
📆 @jakub_nowosad you have 2 days left before the due date for your review (2023-02-06). |
📆 @Nowosad you have 2 days left before the due date for your review (2023-02-06). |
@mikemahoney218<https://github.com/mikemahoney218> thanks for taking into account my comments. I hope that they have been useful.
I think these comments reflect that a few functions (namely, the p-value functions) are maybe a bit out of scope for the package, but I'm interested in what you (and others) think.
My opinion is that if a functions is not going to be used by the user it is best to keep it hidden. You can always use ::: if needed (for some reason…). So, the p-value functions is called from other functions in the package but I do not think that it is meant to be used directly by the user.
The p-value functions are included as "model assessment" tools because I've seen modeling projects use p-values to ID areas of concern, with regards to autocorrelation: locations with more extreme p-values for local autocorrelation metrics were selected for further investigation, to see if model specifications could be improved. In that sense, p-values are included as an assessment metric for predictive modeling, and not so much for statistical testing purposes. As such, waywiser lets you return p-values without also returning test statistics themselves, as this approach doesn't really require looking at the underlying test statistic values; extreme p-values are areas of interest, no matter what their actual statistic is.
I see. But p-values are tied to the stats statistics so I am not sure it is a good idea to only report p-values
Given all this, I see two desirable ways to address this set of comments:
1. Remove p-value functions from the package. This removes a use-case for the package (looking for extreme p-values to identify areas which might help improve model specifications), but also makes it more clear that the package is designed for assessing predictive accuracy, and is not oriented towards inference.
2. Retain p-value functions, but add documentation clarifying that for inference users are recommended to use spdep equivalents directly.
I would go for (2) and compute statistics and associated p-values from a main function. Then the user can decide on what to plot in a map.
|
@becarioprecario Thank you -- your comments have been extremely useful 😄 My thinking (and experience) is that the p-value functions are called directly by users as a model diagnostic tool during the iteration process -- these p-values aren't being reported in a publication, but rather used to guide model development by highlighting hot-spots for model residuals (and hopefully helping to make a model misspecification clear, so it can be fixed before any publication). There's not really a great way for functions using the yardstick infrastructure to return two different statistics (so here, the test statistic and p-value). The idiomatic way to do so is to use For example, if you want to calculate (for instance) global Moran's I with a p-value, plus an agreement coefficient, you can run That's why the "combined" functions were removed before this package was submitted; they don't work in a lot of places that users would expect them to be useful, and explaining the reason they work in a very different way than the rest of the package is pretty hard to communicate. Instead, all of the metrics provided by this package are pure yardstick metrics, without any of the weird edge cases. That means they're restricted to each returning a single type of statistic. I've added documentation as described in (2) to these functions (see for instance ropensci/waywiser@333cf42#diff-45d2e91a37be2289564b4e1c987cbc8ac817ee874cc0ddcf19cfcdd8088c01feR6-R9 ). I could also add documentation about using Alternatively, I could remove the p-value functions, if you think that having them at all without a combination function is harmful. But I don't think it makes sense to add combination functions; they introduce too many weird edge cases and don't idiomatically fit into yardstick. |
Many thanks @becarioprecario and @Nowosad for your useful reviews, and @mikemahoney218 for taking into account all the comments and suggestions to improve the package. I would like to ask @becarioprecario and @Nowosad if you are happy with the new version of the package and the package can be approved or you have additional comments. |
@Paula-Moraga I am happy the current version of the package. |
Hi @becarioprecario @Paula-Moraga , I just wanted to bump this thread: are there additional comments still to be resolved? Thank you! |
@mikemahoney218<https://github.com/mikemahoney218> sorry for missing this. I am still concerned about not reporting together the test statistic and the p-value. Really, from a statistical point of view there is no point in reporting only one of them when making inference. You can have tiny p-values with really meaningless effects. I agree that not shooting yourself in the foot is most likely on the users’ side but still…
In any case, these two values can be extracted with the functions in the package, which is fine, I think. If you consider that this discussion is helpful you can include it somewhere in the documentation and/or vignettes.
|
@becarioprecario Not a problem -- thank you for taking the time to review the package! It's highly appreciated. I'll add a new section to the vignettes tomorrow and follow up here with the commit. |
@becarioprecario I added a summary of this discussion to the "residual autocorrelation" vignette: Thank you again for your time reviewing this package, it's been a real help. |
@mikemahoney218<https://github.com/mikemahoney218> many thanks for adding that note. I think that I have no more comments about the package. Thanks for contributing your package to the community!!
|
Many thanks @mikemahoney218 @becarioprecario @Nowosad for your time and work to improve the package. I am very pleased to approve it! |
@ropensci-review-bot approve waywiser |
Approved! Thanks @mikemahoney218 for submitting and @becarioprecario, @jakub_nowosad, @Nowosad 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 @ropensci/blog-editors in your reply. They will get in touch about timing and can answer any questions. We maintain an online book with our best practice and tips, this chapter starts the 3d section that's about guidance for after onboarding (with advice on releases, package marketing, GitHub grooming); the guide also feature CRAN gotchas. Please tell us what could be improved. Last but not least, you can volunteer as a reviewer via filling a short form. |
@ropensci-review-bot invite me to ropensci/waywiser |
Invitation sent! |
@ropensci-review-bot finalize transfer of waywiser |
Transfer completed. |
Date accepted: 2023-02-27
Due date for @becarioprecario: 2023-02-04Submitting Author Name: Mike Mahoney
Submitting Author Github Handle: @mikemahoney218
Repository: https://github.com/mikemahoney218/waywiser
Version submitted:
Submission type: Stats
Badge grade: silver
Editor: @Paula-Moraga
Reviewers: @becarioprecario, @jakub_nowosad, @Nowosad
Due date for @jakub_nowosad: 2023-02-06
Due date for @Nowosad: 2023-02-06
Archive: TBD
Version accepted: TBD
Language: en
Scope
Please indicate which of our statistical package categories this package falls under. (Please check one appropriate box below):
Statistical Packages
Pre-submission Inquiry
General Information
Anyone fitting models to spatial data, particularly (but not exclusively) people working within the tidymodels ecosystem. This includes a number of domains, and we've already been using it in our modeling practice.
Paste your responses to our General Standard G1.1 here, describing whether your software is:
Please include hyperlinked references to all other relevant software.
The waywiser R package makes it easier to measure the performance of models fit to 2D spatial data by implementing a number of well-established assessment methods in a consistent, ergonomic toolbox; features include new yardstick metrics for measuring agreement and spatial autocorrelation, functions to assess model predictions across multiple scales, and methods to calculate the area of applicability of a model.
Relevant software implementing similar algorithms include CAST for
ww_area_of_applicability()
. Several yardstick metrics implemented directly wrap spdep in a more consistent interface. Willmott's D is also implemented in hydroGOF. Other functions have (as far as I am aware) not been implemented elsewhere, such asww_multi_scale()
which implements the procedure from Riemann et al 2010, orww_agreement_coefficient()
which implements metrics from Ji and Gallo 2006.N/A
Badging
Silver
This is the primary aspect which I believe merits the silver status. The waywiser package implements routines which are useful for a wide variety of spatial models and integrates well with the tidymodels ecosystem, making it (hopefully!) of interdisciplinary interest.
Depending on what the editors think, I'd also potentially submit this for gold, based upon the following two aspects:
But I'm not familiar enough with the system to know if waywiser is likely to be in compliance with these two aspects, and am comfortable submitting for "silver" status if waywiser does not obviously meet both.
Technical checks
Confirm each of the following by checking the box.
autotest
checks on the package, and ensured no tests fail. (Sorry, both the release and CRAN versions of autotest fail immediately on my machine with internal errors -- that is, from autotest itself and not from my package -- and therefore I have not been able to use it).srr_stats_pre_submit()
function confirms this package may be submitted.pkgcheck()
function confirms this package may be submitted - alternatively, please explain reasons for any checks which your package is unable to pass.This package:
Publication options
Code of conduct
The text was updated successfully, but these errors were encountered: