FRBCesab / funbiogeo

:package: R package to help with analyses in functional biogeography
https://frbcesab.github.io/funbiogeo/
GNU General Public License v2.0
10 stars 1 forks source link

Potential issue with the type of site/species column #53

Closed ahasverus closed 2 years ago

ahasverus commented 2 years ago

Different functions have the statement: if (identical(selected_sites, character(0))) { message(...) }. But if the site (or the species) column is considered as an integer by R, no message will be returned.

We should replace this statement by: if (length(selected_sites) == 0) { message(...) }

ahasverus commented 2 years ago

Also, the end of the fb_filter_*() functions:

returned_sites <- site_species[site_species[["site"]] %in% selected_sites, , drop = FALSE]

if (identical(selected_sites, character(0))) {
  message("No sites has the specified trait coverage threshold")
  returned_sites <- site_species[NULL,]
}

should be rewritten as:

if (length(selected_sites) == 0) {
  message("No sites has the specified trait coverage threshold")
  returned_sites <- site_species[NULL,]
} else {
  returned_sites <- site_species[site_species[["site"]] %in% selected_sites, , drop = FALSE]
}

This allows a better control, and avoid to subset data twice if no sites are selected.

Rekyt commented 2 years ago

Very true!

We should replace the code as you specified plus add tests with sites/species columns that are not characters.