Conversation
- Add parameter `rtCenterFun` to `PeakDensityParam()` allowing to specify the function to calculate the feature's retention time from the assigned chromatographic peaks' retention times (issue #823). - Ensure backward compatibility with `PeakDensityParam` not having that slot.
- Load test data from *MsDataHub* instead of *msdata* and remove dependency on *msdata*. - Use `Spectra::rbindlistWithRownames()` for a fast, `data.table::rbindlist()`-based way to row-bind `data.frame`s.
|
Note @sneumann : this PR includes now also:
|
|
Wait - why are there now merge conflicts. I will have a look at them |
|
|
||
| test_that("findChromPeaks,OnDiskMSnExp,MassifquantParam works", { | ||
| mzf <- system.file("microtofq/MM14.mzML", package = "msdata") | ||
| mzf <- faahko_3_files[1L] |
There was a problem hiding this comment.
Do the MsDataHub set of files have names instead of adressing by index ?
There was a problem hiding this comment.
yes, that's how I understood it. each file is one "data set", and they can be accessed by default through their name. But in this particular case I was defining the faahko_3_files variable in the main testthat.R file. It is 3 files from faahKO.
There was a problem hiding this comment.
Ah, now I got it. You wanted something like faahko_3_files["ko15.CDF"] instead of accessing by index. I think faahko_3_files does not have names, but I could directly use the MsDataHub::ko15.CDF() instead.
| ## Check that we've got all required columns | ||
| .reqCols <- c("mz", "rt", "sample") | ||
| if (sleep > 0) | ||
| if (sleep > 0 | rtCenterFun == "wMean") |
There was a problem hiding this comment.
Hm, sleep >0 was for displaying stuff during processing ?
Ah, we need intensity either when displaying stuff or when calculating weights ? I was initially surprised why these quite different conditions were ored.
| microtofq_fs <- c(system.file("microtofq/MM14.mzML", package = "msdata"), | ||
| system.file("microtofq/MM8.mzML", package = "msdata")) | ||
| microtofq_od <- readMSData(microtofq_fs, mode = "onDisk") | ||
| ## microtofq_fs <- c(system.file("microtofq/MM14.mzML", package = "msdata"), |
There was a problem hiding this comment.
Do we need to get them into MsDataHub ?
There was a problem hiding this comment.
I did not want to add all files to MsDataHub - we can use other files for testing purposes too, the microtofq files were handy because they were small. There was no particular need or reason to use them (apart from the size). So I did not add them to MsDataHub but I changed to other test files. It's anyway better to use less different test files as they need to be downloaded and cached by the testing machine - so we save bandwidth.
| MsFeatures, | ||
| MsExperiment (>= 1.5.4), | ||
| Spectra (>= 1.16.1), | ||
| Spectra (>= 1.21.5), |
| mgcv, | ||
| rhdf5 | ||
| rhdf5, | ||
| MsDataHub |
There was a problem hiding this comment.
good point. I will add that.
|
OK, @sneumann I fixed the merge conflicts. Also, I reverted to load the FAAH KO data files still from the faahKO R package. For now (this release) I think that's saver as we would need to change some additional test files, results files etc. |
|
Cool, I can confirm it works locally. Merging and pushing now. |
2 participants
This PR:
rtCenterFunto thePeakDensityParam.plotPrecursorIons()function.