-
Notifications
You must be signed in to change notification settings - Fork 80
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add chromPeakSummary function and a fix #772
base: devel
Are you sure you want to change the base?
Changes from all commits
2c57df3
dacd665
188541c
e5aa3ff
b59b15e
c017cba
fe7509c
670de04
c581cde
f9d4b0d
e7ee120
dd68e2f
a49539e
13cfaf3
156788a
fd9cf65
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2182,3 +2182,8 @@ setClass("FilterIntensityParam", | |
msg | ||
else TRUE | ||
}) | ||
|
||
setClass("BetaDistributionParam", | ||
contains = "Param" | ||
) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -279,6 +279,7 @@ dropGenericProcessHistory <- function(x, fun) { | |
valsPerSpect = valsPerSpect, rtrange = rtr, | ||
mzrange = mzr) | ||
if (length(mtx)) { | ||
## mtx: time, mz, intensity | ||
if (any(!is.na(mtx[, 3]))) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I usually recommend avoiding hardcoded column numbers, imagine someone came up with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree - generally. Here, we load the I did some timings on that: mtx <- cbind(time = 1:5, mz = 1:5, intensity = c(2342.2, 123.1, 231.1, 23.1, 123.23))
int_col <- 3L
library(microbenchmark)
> microbenchmark(mtx[, 3L], mtx[, "intensity"], mtx[, int_col])
Unit: nanoseconds
expr min lq mean median uq max neval cld
mtx[, 3L] 354 363.5 437.30 380.5 477.5 948 100 a
mtx[, "intensity"] 389 404.0 543.07 412.0 474.0 5036 100 a
mtx[, int_col] 381 399.5 469.45 407.0 484.0 1135 100 a |
||
## How to calculate the area: (1)sum of all intensities / (2)by | ||
## the number of data points (REAL ones, considering also NAs) | ||
|
@@ -290,21 +291,20 @@ dropGenericProcessHistory <- function(x, fun) { | |
## as e.g. centWave. Using max(1, ... to avoid getting Inf in | ||
## case the signal is based on a single data point. | ||
## (3) rtr[2] - rtr[1] | ||
res[i, "into"] <- sum(mtx[, 3], na.rm = TRUE) * | ||
res[i, "into"] <- sum(mtx[, 3L], na.rm = TRUE) * | ||
((rtr[2] - rtr[1]) / | ||
max(1, (sum(rtim >= rtr[1] & rtim <= rtr[2]) - 1))) | ||
maxi <- which.max(mtx[, 3]) | ||
maxi <- which.max(mtx[, 3L]) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, above was not the only hardcoded |
||
res[i, c("rt", "maxo")] <- mtx[maxi[1], c(1, 3)] | ||
res[i, c("rtmin", "rtmax")] <- rtr | ||
## Calculate the intensity weighted mean mz | ||
meanMz <- do.call(mzCenterFun, list(mtx[, 2], mtx[, 3])) | ||
if (is.na(meanMz)) meanMz <- mtx[maxi[1], 2] | ||
res[i, "mz"] <- meanMz | ||
|
||
if ("beta_cor" %in% cn) { | ||
if ("beta_cor" %in% cn) | ||
res[i, c("beta_cor", "beta_snr")] <- .get_beta_values( | ||
mtx[, 3L], mtx[, 1L]) | ||
} | ||
vapply(split(mtx[, 3L], mtx[, 1L]), sum, NA_real_), | ||
unique(mtx[, 1L])) | ||
} else { | ||
res[i, ] <- rep(NA_real_, ncols) | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @wkumler , I needed to read that a few times. So a value of 5 means symmetric ? Why is it not zero centered, so that
abs()
tells you the skewedness (regardless in what direction) and you can<0
or>0
if you only want the direction ?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good point, thank you for the review. I'm using the language inherent to the
dbeta()
function where the values of 2-5 correspond to the "positive parameters" alpha and beta (or, in R, shape1 and shape2). I'm absolutely open to rescaling these and I agree that a positive/negative skew would be more intuitive. If we're open to rescaling and we have a parameter to pass now inchromPeakSummary
then we also may want to allow a wider variety of numbers - the values of 5 matched my intuition for peak shape corners but other folks may want more/less tail and more/less skew.