rformassspectrometry / Spectra

Low level infrastructure to handle MS spectra
https://rformassspectrometry.github.io/Spectra/
34 stars 24 forks source link

MzBackendMzR misleading error with empty file list and misleading behavior with `files` argument #267

Closed meowcat closed 1 year ago

meowcat commented 1 year ago

When constructing MzBackendMzR with an empty file list, the returned error message is misleading:

fls <- c()
sp <- Spectra(fls, source = MsBackendMzR())
# Error in .local(object, ...) : 
#   Parameter 'files' is mandatory for 'MsBackendMzR'

This lets me think that I should be using Spectra(files = fls, ...) instead, which in fact does not error:

sp <- Spectra(files = fls, source = MsBackendMzR())
# no error
sp
# MSn data (Spectra) with 0 spectra in a MsBackendMemory backend:

However files is not the right argument, in fact sp <- Spectra(files = fls, source = MsBackendMzR()) completely ignores fls:

fls <- dir(system.file("sciex", package = "msdata"), full.names = TRUE)
sp <- Spectra(files = fls, source = MsBackendMzR())
sp
# MSn data (Spectra) with 0 spectra in a MsBackendMemory backend:

Passing fls as anonymous parameter works as expected:

sp <- Spectra(fls, source = MsBackendMzR())
sp
# MSn data (Spectra) with 1862 spectra in a MsBackendMzR backend:

Expected behaviour: fls <- c(); sp <- Spectra(fls, source = MsBackendMzR()) should

Note that no-input MsBackendMzR (and also MsBackendMgf) objects seem to be invalid anyway. I don't know if this is intended, so this would influence what behaviour we expect from a no-file MzBackendMzR.

sp <- Spectra(source = MsBackendMzR())
sp2 <- setBackend(sp, MsBackendMemory())
# Error in validObject(object) : 
# invalid class “MsBackendMemory” object: The following columns have a wrong data type: mz, intensity.
# The expected data type(s) is/are: NumericList, NumericList

(Discovered accidentally because I did not have package "msdata" installed when building the Spectra.Rmd vignette.)

meowcat commented 1 year ago

I spun out the last part into a separate issue #268

jorainer commented 1 year ago

Good point Michele. Will fix.

jorainer commented 1 year ago

this should be fixed with the PR above.