rformassspectrometry / Spectra

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

New .peaksapply fails with MsBackendMassbankSql #187

Closed meowcat closed 3 years ago

meowcat commented 3 years ago

Hi,

filing this here, perhaps it's a MsBackendMassbankSql issue. Using addProcessing with the "new" .peaksapply, fails for me on a MsBackendMassbankSql backend at time of applying a lazy queue.

Error in (function (x, ...) : argument "x" is missing, with no default

It works if I convert the data to MsBackendDataFrame first. Everything worked with the old .peaksapply where arguments ms2Leveland centroided were expected.

I can't get far in debugging because the error handler obscures to me where the issue actually is coming from.

Below is a reprex with the minimassbank.sqlite:

library(tidyverse)
library(Spectra)
library(MsBackendMassbank)
library(RSQLite)
library(DBI)

con <- dbConnect(
  RSQLite::SQLite(),
  system.file("sql/minimassbank.sqlite", package="MsBackendMassbank")
)

be <- backendInitialize(MsBackendMassbankSql(), dbcon = con)
sp <- Spectra(be)

sp_selection <- sp[2:5]
sp_converted <- setBackend(sp_selection, MsBackendDataFrame())

print(peaksData(sp_selection)[[1]])
print(peaksData(sp_converted)[[1]])

ms2_normalize_spectrum <- function(x, ...) {
  scale <- 333
  x[,2] <- x[,2] / max(x[,2]) * scale
  x  
}

sp_converted_norm <- sp_converted %>% addProcessing(ms2_normalize_spectrum)
print(peaksData(sp_converted_norm)[[1]])
sp_selection_norm <- sp_selection %>% addProcessing(ms2_normalize_spectrum)
# Here we fail, when the processing function would be applied to get the actual peaks.
print(peaksData(sp_selection_norm)[[1]])

# It's not the pipe's fault:
test2 <- addProcessing(sp_selection, ms2_normalize_spectrum)
print(peaksData(test2))
jorainer commented 3 years ago

Can you please provide the output of your sessionInfo?

jorainer commented 3 years ago

Oh man. I fixed it now. Thanks so much @meowcat for reporting and the reproducible example!

The problem was that the list returned by peaksData,MsBackendMassbankSql is named. And the name is then used as the variable name. Anyway, I dropped now the names and it works. I will merge then change into the master branch once the unit tests run through.

jorainer commented 3 years ago

Can you please verify that it's working now @meowcat and eventually close the issue?

meowcat commented 3 years ago

Works, thanks!