search

tidymodels / spatialsample

Create and summarize spatial resampling objects 🗺
https://spatialsample.tidymodels.org
Other
71 stars 5 forks source link

Expand bounding box in `spatial_block_cv()` by default #151

Closed mikemahoney218 closed 11 months ago

mikemahoney218 commented 11 months ago

This PR has two main changes to spatial_block_cv(), sparked by this question on StackOverflow. Fixes #150.

The first change is a new error that throws when observations are assigned to multiple assessment sets in spatial_block_cv() (before running the radius step, which might result in observations correctly being in multiple assessment sets). I don't think this could come up elsewhere in the package -- we don't do many explicit st_intersection operations -- so right now this check is only in spatial_block_cv() rather than being more generally applied.

The second change is a new argument to spatial_block_cv(), called expand_bbox, which is a slight "fudge factor" that expands the bounding box before we build a grid. Because sf::st_make_grid() starts building its grid on the bottom-left observation, and sizes its cells so that the grid stops at exactly the top-right observation, with regularly spaced data the grid lines will often overlap exactly with the data, causing this issue. By expanding the grid a tiny amount, we can avoid those intersections.

We already did this fudge for lat/lon data, because the grid has straight lines between points and lat/lon data might "curve" along the surface of the Earth. This argument lets the user control the degree of fudging that we do, and also applies it to projected data by default, which should help make this situation less common. This is a breaking change -- notice how a few observations in the boston_canopy plots have shifted to new folds -- and I've tried to call it out as such.

mikemahoney218 commented 11 months ago

@hfrick , any chance you'd be willing to check out this PR that's aiming to address a bug from StackOverflow? I see that the no suggests check is failing, but it seems to be unrelated; I'm going to try and address that separately.

mikemahoney218 commented 11 months ago

Think this is the cause of the failing no-suggests check: https://github.com/r-lib/testthat/issues/1880

github-actions[bot] commented 11 months ago

This pull request has been automatically locked. If you believe you have found a related problem, please file a new issue (with a reprex: https://reprex.tidyverse.org) and link to this issue.