Monoisotopic masses can be calculated correctly. #32
Conversation
|
@sneumann I managed to amend the PR by the missing LazyData statement. Now all checks are passed. If you are fine with the changes, please accept, so I can take care of the next issue from the repository's list. |
NAMESPACE
Outdated
| @@ -8,6 +8,7 @@ export(getFormula) | |||
| export(getIsotope) | |||
| export(getMass) | |||
| export(getMolecule) | |||
| export(getMono) | |||
There was a problem hiding this comment.
Can I have getMonoisotopic ? The abbreviation looses meaning without context
| #' @param ele Character vector of elements to count particularly or counting all contained in string if NULL. | ||
| #' @return A named numeric with counts for all contained or specified elements. | ||
| #' @references This function is a copy from the `CorMID` package by Jan Lisec. | ||
| #' It is not (and should bot be) exported in Rdisop but only used internally. |
There was a problem hiding this comment.
What about starting internal function with a dot, .CountChemicalElements()
R/getMolecule.R
Outdated
| @@ -3,25 +3,29 @@ | |||
| #' @aliases getMass | |||
| #' | |||
| #' @description Parse the sum formula and calculate the theoretical exact mass | |||
| #' and the isotope distribution. | |||
| #' and the isotope distribution for an approximate MS resolution of 20,000. | |||
There was a problem hiding this comment.
Does the 20.000 come from the Boecker paper ? For sure there is no isotope fine structure.
Maybe mention that instead ?
There was a problem hiding this comment.
In enviPat it is possible to adjust the resolution. By comparing the results with Rdisop I estimated what is probably internally used. I feel it is important for the user to get an approximate idea of the resolution he can expect. However, if you feel uncomfortable to provide this number, feel free to change to 'no isotope fine structure' instead.
R/getMolecule.R
Outdated
| #' exact mass and the isotope distribution. | ||
| #' The exact mass is the mass of the most abundant isotope and is not | ||
| #' identical with the monoisotopic mass. The latter can be extracted using | ||
| #' the function `getMono()`. This function can also be supplied with a |
There was a problem hiding this comment.
if getMono() is changed, then here as well.
There was a problem hiding this comment.
Yes. There are more instances (e.g. the Changes-document and probably a testthat file). Please Search and replace the whole package.
|
@sneumann I am not sure about how we proceed. :/ Will you make the changes while accepting the PR or shall I open another PR where the modifications are incorporated? Bests, Jan |
|
Hi, no need for a separate PR. You can go ahead and fix/rename things, push to your janlisec:devel and these changed automagically go into this PR here. Worst case ping me again, and I can merge all in one go. Yours, Steffen |
|
Merged & Pushed as 1.67.1 to https://bioconductor.org/checkResults/devel/bioc-LATEST/Rdisop/ |
|
Hm, maybe my manual changes prevented GitHub from seeing it was merged in e70ef18 |
Suggested Version: 1.65.4
Changes:
* Rdisop-1.65.4
* !changed
getIsotopefunction to return a list of matrices to be consistent with the other getter functions* new exported function
getMono()which will return the mono-isotopic mass for a molecule object (also when a list of molecules as for decomposeMass) or alternatively for a vector of chemical formulas* modified documentation to clarify the difference of exactmass and mono-isotopic mass
*
testthattest forgetMonowere set up* new internal function to count chemical elements (imported/copied from the CorMID package) to be utilized in
getMono* two data sets included in package and appropriately documented:
*
isotopes(a data.frame version of the PSE info)*
mono_masses(a named vector containing only the most abundant isotope per element)