EvolEcolGroup / tidysdm

R package to fit species distribution models (SDMs) using the 'tidymodels' framework
https://evolecolgroup.github.io/tidysdm/
GNU Affero General Public License v3.0
28 stars 8 forks source link

unexported functions in other packages #37

Closed topepo closed 9 months ago

topepo commented 9 months ago

I'm probably the last person to get preachy about this but in the process of #36 we did see:

$ grep getFromNamespace *.R
boyce_cont.R:  utils::getFromNamespace("abort_if_class_pred", "yardstick")(truth)
boyce_cont.R:  if (!utils::getFromNamespace("is_binary", "yardstick")(estimator)) {
grid_cellsize.R:  if (utils::getFromNamespace("is_longlat", "spatialsample")(data)) {
grid_cellsize.R:    grid_box <- utils::getFromNamespace("expand_grid", "spatialsample")(grid_box)
grid_offset.R:  if (utils::getFromNamespace("is_longlat", "spatialsample")(data)) {
grid_offset.R:    grid_box <- utils::getFromNamespace("expand_grid", "spatialsample")(grid_box)
kap_max.R:  utils::getFromNamespace("abort_if_class_pred", "yardstick")(truth)
kap_max.R:  if (!utils::getFromNamespace("is_binary", "yardstick")(estimator)) {
recipes_sf_methods.R:  utils::getFromNamespace("prep.recipe", "recipes")(
recipes_sf_methods.R:  utils::getFromNamespace("bake.recipe", "recipes")(object = object, ...,
tss_max.R:  utils::getFromNamespace("abort_if_class_pred", "yardstick")(truth)
tss_max.R:  if (!utils::getFromNamespace("is_binary", "yardstick")(estimator)) {

I can completely see that a similar issue will pop up in one of these.

I suggest putting in issues for the respective packages about your needs. I think that there are a few options to make this better:

dramanica commented 9 months ago

@topepo You are right, guilty as charged. These hacks should have not made it into the version on cran, they are mostly convenience two liners that were meant to be replaced by local copies/alternative, but somehow that did not happen. For spatial_recipes, using NextMethod is an elegant solution. For the moment, I'll go with that, but it would be nice indeed to make the spatial column work natively in recipes.