Correct Issues with Initial `scrg__get_geometry_in_bbox` functions

[brotherjack]

In working with the wind/water data that @georgeomics found, I have come across various problems with the simplisticscrg__get_geometry_in_bbox functions I wrote. Here are the issues that I'm going to work on fixing (note @Bvpboy: these would be good unit test cases to write up).

• R function overloading is not working as expected, so calling up source('utils.R') will result in only the last scrg__get_geometry_in_bbox function being loaded. I believe that this can be corrected with S4 dispatch, so I'll read up on that and implement it.
• The function should have the option to accept Lat/Lon as well as Lon/Lat.
• Currently the function only will work if the sf package explicitly switches off spherical geometry (ie. sf_use_s2(FALSE) is run). If this is not the case, the call to st_within(sf_object, bbox, sparse = FALSE) will fail with the following error: Error in wk_handle.wk_wkb(wkb, s2_geography_writer(oriented = oriented, : Loop 0 is not valid: Edge 1 is degenerate (duplicate vertex). If this needs to work on spherical geometries this will need to be improved, otherwise we should make it clear that s2 be deactivated for the function to work properly.

[chfleming]

• (long,lat) is fairly universal
• My understanding is that a dataset will either have planar geometry (older or very local datasets) or spherical geometry, and so when designing a function to extract values from a planar geometry dataset, you could run sf_use_s2(FALSE) at the beginning of the function and sf_use_s2(TRUE) at the end. That's not ideal behavior for a CRAN package, but sf_use_s2 itself is not ideally designed in a usual R way. Normally something like this would be an option and you could record the option at the beginning, set it to what you want, and then restore the option at the end.

[brotherjack]

Turns out this issue, and the functions under consideration, are unnecessary. @Bvpboy has found the functionality we need in the sf package. Instead we'll write up some documentation on where this is and how to use it.

We'll close this issue when that is completed and merged in.