Landscape-Data-Commons / terradactyl

Tools for using terrestrial core methods data
Other
3 stars 4 forks source link

pct_cover(by_line = TRUE) fails #101

Closed nstauffer closed 2 years ago

nstauffer commented 2 years ago

Currently, attempting to use pct_cover(by_line = TRUE) will result in the error message: Error in `dplyr::mutate()`: ! Problem while computing `..1 = dplyr::across(c(2:ncol(.)), round, digits = 2)`. Caused by error in `across()`: ! Problem while computing column `LineKey`. Caused by error in `fn()`: ! non-numeric argument to mathematical function

It looks like this is due to the step labeled "constrain to two digits" at the end of the function. I think that it'd be fixable if the column indices weren't hardcoded using something like mutate_if(is.numeric, round(digits = 2)). Alternatively, maybe something like mutate_at(vars(-matches()), round(digits = 2)) would be more appropriate in case one or more grouping variables were numeric. I'm not positive that that's how mutate_if() or mutate_at() actually work, but it's pretty close.

nstauffer commented 2 years ago

Preliminary solution: I added an argument for the number of digits to round to (digits) and replaced the rounding step with the following:

# Round results
# Be warned! This doesn't care if about grouping variables and will round them
# too if they're numeric
summary <- summary %>%
  dplyr::mutate(dplyr::across(where(is.numeric),
                              round,
                              digits = digits))

This appears to fail if digits isn't provided as a named argument though, which is weird. I'll dig in deeper.

nstauffer commented 2 years ago

This works fine when there aren't grouping variables supplied, but if there are then all arguments need to be provided even if you want to use the default values. Not related to this patch, but something to tackle later. Probably lower priority since I think it's common behavior with multiple functions in the package.

On another note: @smccord, does it make more sense to use signif() instead of round()?

josephbrehm commented 2 years ago

All the rounding code has been disabled. We might need to reopen this if/when rounding is reintroduced