-
Notifications
You must be signed in to change notification settings - Fork 11
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
Addition of function force_sorted()
#123
Conversation
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.
Thanks Phili for the contribution. I'm generally OK, only some minor fixes in the documentation.
Maybe also to give a bit more of a context to this function: in the alignment (retention time adjustment) of LC-MS data we sometimes observe that the order of adjusted retention times of spectra is no longer ordered - which in fact should not happen, because spectra should always be ordered as they were acquired in the LC-MS run - and this is ordered by retention time. So, in cases were some spectra have out-of-order adjusted retention times we want to force them into order again. At present, this code is used in |
R/force_sorted.R
Outdated
#' | ||
#' ## We can see the values were not interpolated but rather replaced by the | ||
#' ## last increasing value `2` and increasing by 0.1. | ||
#' sorted_vec |
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.
I think this should be sorted_rtime
. Also could you use another name, like x2 or y, to keep it general. Here, I guess rtime refers to retention time, but this isn't mentioned in the man page (and there's no need to), so it is a bit of an obscure name.
R/force_sorted.R
Outdated
warning("Found decreasing values at the end of the vector. ", | ||
"Interpolation is not possible in this region. Instead, ", | ||
"replacing these values with a sequence that starts from ", | ||
"the last increasing value and increments by ", by, |
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.
There are two spaces after by.
Thank you for the contribution! |
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.
Thanks Phili! green lights also from my side. Will merge and push to BioC.
Thanks!
Here I propose the function
force_sorted()
which takes as an input a vector of numeric values and interpolate the values that are not increasing.I wrote this function specifically for a retention alignment method that is being developed in xcms and at some point i had to deal with unsorted/non-increasing retention times values. However, I believe it could be used in the future or by user for some other reasons.
Also, would welcome inputs on the name, as the function do no
sort()
the values per se, but we do check for the results by checkingis.unsorted()
.I also thought about a possible extra parameter
keep.na = TRUE
which would (ifFALSE
) allow the user to decide if they want the NA values to be adjusted too or not.Welcoming feedback :)