BiologicalRecordsCentre / sparta

Species Presence/Absence R Trends Analyses
http://biologicalrecordscentre.github.io/sparta/index.html
MIT License
21 stars 24 forks source link

Testing site_match & sites_included #194

Closed drnickisaac closed 3 years ago

drnickisaac commented 4 years ago

I am looking at the code for occDetFunc() and the sites_included element of out. I want to be 100% confident that the order of sites in sites_included matches exactly with the order in which they appear in the bugs_data object.

I can't quite tell whether the current code does what we want. I was concerned about the use of unique() of line 282, but this function seems to retain the order in which items first appear. I'm a bit concerned about the multiple uses of as.numeric() on lines 281, 283 & 653, although quick tests do not raise obvious alarms.

@AugustT I would be really grateful if you would check out these sections of code to look for potential gremlins.

It would also be good to know who wrote this part of the code, and whether it has been tested.

AugustT commented 4 years ago

Hmmm.... is there some way to definitively test this? I feel we need another sprint day to push some of these things forward.

drnickisaac commented 3 years ago

I have spent a heap of time today moving code around in occDetFunc(). The sites_to_include object is now never changed from character to numeric, and it's only edited once. So the risk of a problem is now gone. The conversion of character (for the bugs_data object is now explicit, here: site_match <- unique(data.frame(name = occDetdata$site, id = as.numeric(as.factor(occDetdata$site)))) occDetdata <- merge(occDetdata, site_match, by.x='site', by.y="name") Also, we're now returning the data frame site_match rather than just the vector of site names.