Chromatogram fdata #290
Chromatogram fdata #290
Conversation
- Add featureData slot to Chromatograms class. - Add getter/setter method for featureData. - Add precursorMz, productMz and mz,Chromatograms method. - Add related unit tests and documentation.
| #' | ||
| #' @noRd | ||
| .mz_chromatograms <- function(x, mz = "mz") { | ||
| mz <- match.arg(mz, c("mz", "precursorMz", "productMz")) |
There was a problem hiding this comment.
This is just my personal preference. I like to see all choices for an argument in the definition of a function (especially for an exported and documented function):
.mz_chromatograms <- function(x, mz = c("mz", "precursorMz", "productMz")) {
mz <- match.arg(mz) ## will automatically use "mz" as default
## ...
}| ## If we've got the values in the featureData, use these. | ||
| if (mz %in% c("precursorMz", "productMz")) | ||
| vl <- rep(paste0(sub(mz, pattern = "Mz", replacement = ""), | ||
| "IsolationWindowTargetMZ"), 2) |
There was a problem hiding this comment.
paste0 is superfluous here: vl <- rep(sub("Mz", "IsolationWindowTargetMZ", mz), 2)
| vl <- c("mzmin", "mzmax") | ||
| if (all(vl %in% fvarLabels(x))) { | ||
| ## Want to return a matrix, not a data.frame | ||
| cbind(mzmin = fData(x)[, vl[1]], mzmax = fData(x)[, vl[2]]) |
There was a problem hiding this comment.
Why not explicitly convert into a matrix (the return value of cbind depends on its input, would be a numeric matrix in our use case but who knows ...).
m <- as.matrix(fData(x)[, vl])
dimnames(m) <- list(NULL, c("mzmin", "mzmax"))
mOr in one line:
as.matrix(setNames(fData(x)[, vl], c("mzmin", "mzmax")), rownames.force=FALSE)There was a problem hiding this comment.
I'll check - I think there was something behind using cbind here (performance wise).
There was a problem hiding this comment.
cbind is slightly faster than as.matrix. Not that it matters here, I just prefer using cbind and extract individual columns from a data.frame instead of anything that involves accessing multiple columns at once in a data.frame, as that might/can involve copying of the data, while accessing single columns of a data.frame never copies the data.
There was a problem hiding this comment.
Oh, cool. I wasn't aware of that. You are right:
library("microbenchmark")
set.seed(2017)
n <- 1e5
d <- data.frame(a=sample(n), b=sample(n), c=sample(n))
f1 <- function(x, vl=c("a", "b"))cbind(mzmin=x[, vl[1L]], mzmax=x[, vl[2L]])
f2 <- function(x, vl=c("a", "b"))matrix(c(x[, vl[1L]], x[, vl[2L]]), ncol=2L,
dimnames=list(NULL, c("mzmin", "mzmax")))
all.equal(f1(d), f2(d))
# [1] TRUE
microbenchmark(f1(d), f2(d))
# Unit: microseconds
# expr min lq mean median uq max neval
# f1(d) 112.814 122.4080 203.0495 126.7465 139.109 3693.441 100
# f2(d) 505.302 523.0355 638.2103 526.0990 536.168 4133.861 100| ## the values in one row are not identical | ||
| mzr <- matrix(nrow = nrow(x), ncol = 2, | ||
| dimnames = list(NULL, c("mzmin", "mzmax"))) | ||
| for (i in 1:nrow(mzr)) { |
There was a problem hiding this comment.
Use seq_len(nrow(mzr)) instead of 1:nrow(mzr): https://bioconductor.org/developers/how-to/efficient-code/#avoid--style-iterations
| #' @rdname Chromatograms-class | ||
| #' | ||
| #' @description \code{fData}: return the feature data as a \code{data.frame}. | ||
| setMethod("fData", "Chromatograms", function(object) pData(object@featureData)) |
There was a problem hiding this comment.
Is pData correct (instead of fData)?
There was a problem hiding this comment.
This is correct, although confusing. pData,AnnotationDataFrame accesses the adf@data slot, but is also the accessor for object@phenoData@data slot where object is an eSet type instance.
|
@jotsetung - is this something that needs to be pushed to Bioc quickly? (sorry, initially commented on wrong PR) |
|
Nope, no need to push to Bioc now. This is some preparatory work for the future I'm confused - you did already merge? Then I'll change the requested minor stuff above in the |
|
Yes, I already merged, sorry. What about |
|
yes, I believe SRM and MRM are the same thing - with SRM being the correct term. Having a |
featureDataslot to theChromatogramsobject.mz,Chromatograms,precursorMz,ChromatogramsandproductMz,Chromatograms.Short description: each row of an
Chromatogramsobject should contain chromatogram data for the same ion or m/z range (+ eventually rt range). Having this data in afeatureDataallows users a quick way to access such data.