R-Lum / Luminescence

Development of the R package 'Luminescence'
https://r-lum.github.io/Luminescence/
GNU General Public License v3.0
15 stars 7 forks source link

extract_IrradiationTimes() may fail to update an existing BIN file #445

Closed mcol closed 2 hours ago

mcol commented 2 hours ago

This way of subsetting will produce the wrong result if results[,"STEP"] == "irradiation (NA)" is always FALSE: https://github.com/R-Lum/Luminescence/blob/cc0b78100bba1d55ee091f2eed777ae0a0e0bf89/R/extract_IrradiationTimes.R#L364

In such a case, which() would return integer(0), and subset() would remove all rows, when actually we'd want to keep them all. Using a logical expression in subset() is safer.

When this happens, we may wrongly refuse to update the BIN file.

RLumSK commented 2 hours ago

@mcol We should not use subset() in the package, this causes all kinds of trouble, instead with should check whether there is any irradiation step at all in present. For the moment the function simply relies in the existence of such step (which is not good, I know).

mcol commented 2 hours ago

I don't know why I wrote subset(), what's happening is a simple subsetting of a data frame.