Skip to content

Commit

Permalink
Address reviewer comments (#17)
Browse files Browse the repository at this point in the history
* Address reviewer comments

* Vignette doesn't need kableExtra anymore

* Trip dplyr lifecycle warning outside tests

* Bump min R version
  • Loading branch information
mikemahoney218 authored Feb 2, 2023
1 parent 68416f7 commit 333cf42
Show file tree
Hide file tree
Showing 42 changed files with 255 additions and 89 deletions.
11 changes: 2 additions & 9 deletions .github/workflows/R-CMD-check.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,13 @@ jobs:

- {os: windows-latest, r: 'release'}

# Use older ubuntu to maximise backward compatibility
- {os: ubuntu-latest, r: 'devel', http-user-agent: 'release'}
- {os: ubuntu-latest, r: 'release'}
- {os: ubuntu-latest, r: 'oldrel-1'}
- {os: ubuntu-latest, r: 'oldrel-2'}
# Until dplyr::summarize is swapped for dplyr::reframe
# - {os: ubuntu-latest, r: 'oldrel-3'}
# Uncomment this in 2023 (once current R is >= 4.3)
# The package requires R 3.6, I don't really remember why
# - {os: ubuntu-latest, r: 'oldrel-3'}
# Uncomment this in 2024 (once current R is >= 4.4)
# - {os: ubuntu-latest, r: 'oldrel-4'}

env:
Expand All @@ -58,11 +56,6 @@ jobs:
extra-packages: any::rcmdcheck, CAST=?ignore-before-r=4.1.0
needs: check

- name: Update dependencies
run: |
install.packages("purrr", repos = 'https://cloud.r-project.org')
shell: Rscript {0}

- uses: r-lib/actions/check-r-package@v2
env:
_R_CHECK_FORCE_SUGGESTS_: false
Expand Down
4 changes: 2 additions & 2 deletions DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ URL: https://github.com/mikemahoney218/waywiser,
https://mikemahoney218.github.io/waywiser/
BugReports: https://github.com/mikemahoney218/waywiser/issues
Depends:
R (>= 3.6)
R (>= 4.0)
Imports:
dplyr,
fields,
Expand All @@ -58,7 +58,6 @@ Suggests:
CAST,
covr,
ggplot2,
kableExtra,
knitr,
modeldata,
recipes,
Expand All @@ -74,6 +73,7 @@ Suggests:
withr
Config/testthat/edition: 3
Config/testthat/parallel: true
Config/Needs/website: kableExtra
Encoding: UTF-8
LazyData: true
Roxygen: list(markdown = TRUE, roclets = c("namespace", "rd", "srr::srr_stats_roclet"))
Expand Down
2 changes: 1 addition & 1 deletion R/data.R
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@
#'
#' This data is adapted from the CAST vignette
#' `vignette("cast02-AOA-tutorial", package = "CAST")`.
#' The original data is derived from the Worldclim global climate variables/
#' The original data is derived from the Worldclim global climate variables.
#'
#' @srrstats {G5.1} Data used to test the package is exported.
#'
Expand Down
4 changes: 4 additions & 0 deletions R/global_geary.R
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
#' Calculate the global Geary's C statistic for model residuals.
#' `ww_global_geary_c()` returns the statistic itself, while
#' `ww_global_geary_pvalue()` returns the associated p value.
#' These functions are meant to help assess model predictions, for instance by
#' identifying if there are clusters of higher residuals than expected. For
#' statistical testing and inference applications, use
#' [spdep::geary.test()] instead.
#'
#' These functions can be used for geographic or projected coordinate reference
#' systems and expect 2D data.
Expand Down
4 changes: 4 additions & 0 deletions R/global_moran.R
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
#' Calculate the global Moran's I statistic for model residuals.
#' `ww_global_moran_i()` returns the statistic itself, while
#' `ww_global_moran_pvalue()` returns the associated p value.
#' These functions are meant to help assess model predictions, for instance by
#' identifying if there are clusters of higher residuals than expected. For
#' statistical testing and inference applications, use
#' [spdep::moran.test()] instead.
#'
#' These functions can be used for geographic or projected coordinate reference
#' systems and expect 2D data.
Expand Down
3 changes: 3 additions & 0 deletions R/local_geary.R
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
#' Calculate the local Geary's C statistic for model residuals.
#' `ww_local_geary_c()` returns the statistic itself, while
#' `ww_local_geary_pvalue()` returns the associated p value.
#' These functions are meant to help assess model predictions, for instance by
#' identifying clusters of higher residuals than expected. For statistical
#' testing and inference applications, use [spdep::localC_perm()] instead.
#'
#' These functions can be used for geographic or projected coordinate reference
#' systems and expect 2D data.
Expand Down
3 changes: 3 additions & 0 deletions R/local_getis.R
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
#' Calculate the local Getis-Ord G and G* statistic for model residuals.
#' `ww_local_getis_ord_g()` returns the statistic itself, while
#' `ww_local_getis_ord_pvalue()` returns the associated p value.
#' These functions are meant to help assess model predictions, for instance by
#' identifying clusters of higher residuals than expected. For statistical
#' testing and inference applications, use [spdep::localG_perm()] instead.
#'
#' These functions can be used for geographic or projected coordinate reference
#' systems and expect 2D data.
Expand Down
3 changes: 3 additions & 0 deletions R/local_moran.R
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
#' Calculate the local Moran's I statistic for model residuals.
#' `ww_local_moran_i()` returns the statistic itself, while
#' `ww_local_moran_pvalue()` returns the associated p value.
#' These functions are meant to help assess model predictions, for instance by
#' identifying clusters of higher residuals than expected. For statistical
#' testing and inference applications, use [spdep::localmoran_perm()] instead.
#'
#' These functions can be used for geographic or projected coordinate reference
#' systems and expect 2D data.
Expand Down
2 changes: 2 additions & 0 deletions inst/srr_template_spatial_yardstick.R
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
test_that("srr: expected failures for {{{name}}}", {
trip_dplyr_warning()
worldclim_predicted <- worldclim_simulation
worldclim_predicted$predicted <- predict(
lm(response ~ bio2 * bio10 * bio13 * bio19, data = worldclim_simulation),
Expand Down Expand Up @@ -226,6 +227,7 @@ test_that("srr: expected failures for {{{name}}}", {
})

test_that("other generic srr standards", {
trip_dplyr_warning()
skip_if_not_installed("withr")
worldclim_predicted <- worldclim_simulation
worldclim_predicted$predicted <- predict(
Expand Down
4 changes: 4 additions & 0 deletions man/global_geary_c.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions man/global_moran_i.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions man/local_geary_c.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions man/local_getis_ord_g.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions man/local_moran_i.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion man/worldclim_simulation.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions tests/helper.R

This file was deleted.

7 changes: 0 additions & 7 deletions tests/testthat/_snaps/local_geary.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,6 @@

Code
df_local_c <- ww_local_geary_c(guerry_modeled, Crm_prs, predictions)
Warning <lifecycle_warning_deprecated>
Returning more (or less) than 1 row per `summarise()` group was deprecated in dplyr 1.1.0.
i Please use `reframe()` instead.
i When switching from `summarise()` to `reframe()`, remember that `reframe()` always returns an ungrouped data frame and adjust accordingly.
i The deprecated feature was likely used in the yardstick package.
Please report the issue at <https://github.com/tidymodels/yardstick/issues>.
Code
df_local_c[1:3]
Output
# A tibble: 85 x 3
Expand Down
6 changes: 0 additions & 6 deletions tests/testthat/_snaps/srr-ww_local_geary_c.md
Original file line number Diff line number Diff line change
Expand Up @@ -726,12 +726,6 @@

Code
ww_local_geary_c(worldclim_simulation, response, response)
Warning <lifecycle_warning_deprecated>
Returning more (or less) than 1 row per `summarise()` group was deprecated in dplyr 1.1.0.
i Please use `reframe()` instead.
i When switching from `summarise()` to `reframe()`, remember that `reframe()` always returns an ungrouped data frame and adjust accordingly.
i The deprecated feature was likely used in the yardstick package.
Please report the issue at <https://github.com/tidymodels/yardstick/issues>.
Output
# A tibble: 10,000 x 3
.metric .estimator .estimate
Expand Down
11 changes: 11 additions & 0 deletions tests/testthat/helper.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
if (identical(Sys.getenv("NOT_CRAN"), "true")) {
Sys.setenv("waywiser_test_cast" = "true")
}

trip_dplyr_warning <- function() {
invisible(
suppressWarnings(
dplyr::summarise(dplyr::group_by(iris, Species), 1:2)
)
)
}
2 changes: 2 additions & 0 deletions tests/testthat/test-agreement_coefficient.R
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
test_that("agreement coefficients match Ji and Gallo", {
trip_dplyr_warning()
#' @srrstats {G5.7} Algorithm performs as expected
x <- c(6, 8, 9, 10, 11, 14)
y <- c(2, 3, 5, 5, 6, 8)
Expand Down Expand Up @@ -62,6 +63,7 @@ test_that("agreement coefficients match Ji and Gallo", {
})

test_that("agreement coefficients are the same across methods", {
trip_dplyr_warning()
x <- c(6, 8, 9, 10, 11, 14)
y <- c(2, 3, 5, 5, 6, 8)
df <- data.frame(x = x, y = y)
Expand Down
17 changes: 12 additions & 5 deletions tests/testthat/test-area_of_applicability.R
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,22 @@ importance <- vip::vi_permute(
)

test_that("`ww_area_of_applicability` is properly classed", {
trip_dplyr_warning()
model <- ww_area_of_applicability(y ~ ., train, test, importance)
expect_s3_class(model, "ww_area_of_applicability")
expect_s3_class(model, "hardhat_model")
})


test_that("`ww_area_of_applicability` is not defined for vectors", {
trip_dplyr_warning()
expect_snapshot_error(
ww_area_of_applicability(mtcars$mpg)
)
})

test_that("`ww_area_of_applicability` finds 0 distance between identical data", {
trip_dplyr_warning()
#' @srrstats {G3.0} Testing with appropriate tolerances.
expect_equal(
suppressWarnings(
Expand All @@ -43,7 +46,7 @@ test_that("`ww_area_of_applicability` finds 0 distance between identical data",
})

test_that("`ww_area_of_applicability` works with or without a testing set", {

trip_dplyr_warning()
expect_error(
ww_area_of_applicability(y ~ ., train, test, importance),
NA
Expand All @@ -57,7 +60,7 @@ test_that("`ww_area_of_applicability` works with or without a testing set", {
})

test_that("`ww_area_of_applicability` methods are equivalent", {

trip_dplyr_warning()
methods <- list(
ww_area_of_applicability(y ~ ., train, test, importance),
ww_area_of_applicability(train[2:11], test[2:11], importance),
Expand Down Expand Up @@ -117,7 +120,7 @@ test_that("`ww_area_of_applicability` methods are equivalent", {
})

test_that("`ww_area_of_applicability` can handle different column orders", {

trip_dplyr_warning()
#' @srrstats {G3.0} Testing with appropriate tolerances.
expect_equal(
ww_area_of_applicability(train[2:11], test[2:11], importance)$aoa_threshold,
Expand All @@ -133,7 +136,7 @@ test_that("`ww_area_of_applicability` can handle different column orders", {
})

test_that("NAs are handled", {

trip_dplyr_warning()
train[1, 2] <- NA
test[1, 2] <- NA
comb_rset <- rsample::make_splits(train, test)
Expand Down Expand Up @@ -216,6 +219,7 @@ test_that("NAs are handled", {
})

test_that("Expected errors", {
trip_dplyr_warning()
expect_snapshot(
ww_area_of_applicability(y ~ ., train, test[1:10], importance),
error = TRUE
Expand Down Expand Up @@ -252,7 +256,7 @@ importance <- vip::vi_permute(
aoa <- ww_area_of_applicability(y ~ ., train, test, importance)

test_that("normal use", {

trip_dplyr_warning()
expect_snapshot(
predict(aoa, test)
)
Expand All @@ -265,6 +269,7 @@ test_that("normal use", {
})

test_that("`new_ww_area_of_applicability` arguments are assigned correctly", {
trip_dplyr_warning()
x <- ww_area_of_applicability(y ~ ., train, test, importance)

skip_on_os("mac")
Expand All @@ -284,6 +289,7 @@ test_that("`new_ww_area_of_applicability` arguments are assigned correctly", {
#' @srrstats {G5.4c} Data is derived originally from CAST and associated paper
test_that("ww_area_of_applicability() is close-enough to CAST", {
skip_on_cran()
trip_dplyr_warning()
#' @srrstats {SP6.2} Testing with ~global data
relevant_data <- head(as.data.frame(worldclim_simulation)[c(1:4, 6)], 1000)

Expand Down Expand Up @@ -331,6 +337,7 @@ test_that("ww_area_of_applicability() is close-enough to CAST", {
})

test_that("loaded data is equivalent", {
trip_dplyr_warning()
importance <- data.frame(
term = c("bio2", "bio10", "bio13", "bio19"),
estimate = c(50.68727, 57.66859, 62.81009, 48.72391)
Expand Down
2 changes: 1 addition & 1 deletion tests/testthat/test-global_geary.R
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
test_that("Global Geary statistics are stable", {

trip_dplyr_warning()
guerry_modeled <- guerry
guerry_lm <- lm(Crm_prs ~ Litercy, guerry_modeled)
guerry_modeled$predictions <- predict(guerry_lm, guerry_modeled)
Expand Down
2 changes: 1 addition & 1 deletion tests/testthat/test-global_moran.R
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
test_that("Global Moran statistics are stable", {

trip_dplyr_warning()
guerry_modeled <- guerry
guerry_lm <- lm(Crm_prs ~ Litercy, guerry_modeled)
guerry_modeled$predictions <- predict(guerry_lm, guerry_modeled)
Expand Down
1 change: 1 addition & 0 deletions tests/testthat/test-local_geary.R
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
test_that("Local geary statistics are stable", {
trip_dplyr_warning()
set.seed(123)

guerry_modeled <- guerry
Expand Down
4 changes: 2 additions & 2 deletions tests/testthat/test-local_getis.R
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
set.seed(123)
test_that("Local Getis-Ord statistics are stable", {

trip_dplyr_warning()
guerry_modeled <- guerry
guerry_lm <- lm(Crm_prs ~ Litercy, guerry_modeled)
guerry_modeled$predictions <- predict(guerry_lm, guerry_modeled)
Expand Down Expand Up @@ -61,7 +61,7 @@ test_that("Local Getis-Ord statistics are stable", {
})

test_that("Local Getis-Ord statistics are stable", {

trip_dplyr_warning()
guerry_modeled <- guerry
guerry_lm <- lm(Crm_prs ~ Litercy, guerry_modeled)
guerry_modeled$predictions <- predict(guerry_lm, guerry_modeled)
Expand Down
2 changes: 1 addition & 1 deletion tests/testthat/test-local_moran.R
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
set.seed(123)
test_that("Local Moran statistics are stable", {

trip_dplyr_warning()
guerry_modeled <- guerry
guerry_lm <- lm(Crm_prs ~ Litercy, guerry_modeled)
guerry_modeled$predictions <- predict(guerry_lm, guerry_modeled)
Expand Down
Loading

0 comments on commit 333cf42

Please sign in to comment.