search

tidymodels / spatialsample

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

Warn if CRS is NA, don't error #96

Closed mikemahoney218 closed 2 years ago

mikemahoney218 commented 2 years ago

Simulated data will often have an NA CRS, as there is not a real transformation to map the simulated data onto the real world. Right now, that means spatialsample can't easily be used with simulated data without bizarre workarounds. This PR changes that to enable using spatialsample with simulated data.

The original intent of the error was two-fold: NA CRS don't cooperate with unit conversions (because it's not clear what units to convert values into), and non-sf objects will have an NA CRS and therefore could mostly not be used by these functions. That said, it was possible to use tibbles and similar with spatial_buffer_vfold_cv() before, if a user manually set radius == NULL && buffer == NULL (not missing/default arguments, but fully specified by the user). That was a weird loophole that we already heavily discouraged -- all our documentation (and the error message) says that users should use vfold_cv() instead of spatial_buffer_vfold_cv() if they don't want buffering. As such, requiring sf objects is technically a breaking change, but I would be surprised if anyone was doing it.

mikemahoney218 commented 2 years ago

Addressed both the issue with radius and the error message punctuation. 2ccd06bceef32e021f3e45e2ec5a203f48d35ccb is probably overkill, but I realized I could simplify the code somewhat by factoring all the repeated checks we do into their own functions.

github-actions[bot] commented 2 years 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.