Skip to content
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 extracMsData and plotMsData functions #312

Merged
merged 2 commits into from
Feb 18, 2018
Merged

Add extracMsData and plotMsData functions #312

merged 2 commits into from
Feb 18, 2018

Conversation

jorainer
Copy link
Collaborator

  • Add extractMsData and plotMsData function to extract MS data as a data.frame
    and visualize the data.
  • Fix removePeaks,Spectrum after recent changes in IRanges (introduced in
    Bioconductor 3.7).

The extractMsData and plotMsData are two functions I had defined in xcms but they might be better placed in MSnbase because they provide some more low level functionality. I will need these then later in the examples and vignette related to the centroiding that I am currently developing in the centroiding branch.

- Add extractMsData and plotMsData function to extract MS data as a data.frame
  and visualize the data.
- Fix removePeaks,Spectrum after recent changes in IRanges (introduced in
  Bioconductor 3.7).
R/AllGenerics.R Outdated
@@ -147,3 +147,6 @@ setGeneric("splitByFile", function(object, f, ...) standardGeneric("splitByFile"
setGeneric("productMz", function(object, ...) standardGeneric("productMz"))
## setGeneric("aggregationFun", function(object, ...)
## standardGeneric("aggregationFun"))

setGeneric("extractMsData", function(object, ...)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm using here a method instead of a function, because in xcms I have a extractMsData,XCMSnExp method that has some additional functionality (ensures that adjusted retention times are used if present).

@codecov-io
Copy link

codecov-io commented Feb 16, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@bd5515e). Click here to learn what that means.
The diff coverage is 83.33%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #312   +/-   ##
=========================================
  Coverage          ?   75.97%           
=========================================
  Files             ?       78           
  Lines             ?     8350           
  Branches          ?        0           
=========================================
  Hits              ?     6344           
  Misses            ?     2006           
  Partials          ?        0
Impacted Files Coverage Δ
R/utils.R 71.74% <100%> (ø)
R/methods-MSnExp.R 75.37% <60%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bd5515e...6aa53a1. Read the comment docs.

@lgatto
Copy link
Owner

lgatto commented Feb 16, 2018

I haven't looked into it in details nor run the code, but my first question is, why extractMsData and not as(, "data.frame"). The setAs functions don't support extra arguments (in your case rt, mz and msLevel, but couldn't this be done using appropriate filtering functions filterRt, filterMz and filterMsLevel?

Also, re using as, see as(Spectrum, "data.frame") to possibly use within the MSnExp coercion method.

@lgatto
Copy link
Owner

lgatto commented Feb 16, 2018

I haven't looked at the output of plotMsData, but why have it use a data.frame and not an MSnExp?

@jorainer
Copy link
Collaborator Author

Re using as(Spectrum, "data.frame"): this one does not return the rt values that I need. Also, I'm directly accessing the slots in the function and am not using the mz and intensity methods. I've found that calling the methods takes, if applied to a large number of spectra, considerably longer because R has to look up the method.

I do however like the idea of a as(<MSnExp>, "data.frame"). This should then return a data.frame with columns "file", "rt", "mz" and "i". This could replace the proposed extractMsData method.

@jorainer
Copy link
Collaborator Author

Re plotMsData: the reason was that I used it along the extractMsData, so, extract data with extractMsData, which returns a data.frame and plot that data with plotMsData. In the end calling it on a MSnExp would/should also work - ideally by specifying a file argument to plot the data for a single file. The name of the function is also not the best I have to admit.

@jorainer
Copy link
Collaborator Author

Thinking it over I think I'll reduce the pull request to a single setAs method that converts a MSnExp to a data.frame and drop the plotMsData altogether.

@lgatto
Copy link
Owner

lgatto commented Feb 16, 2018

A plot,MSnExp would be a useful addition, if you have something general and useful enough. If you think the plotMsData function could play such a role, could you paste the produced figure in here.

@jorainer
Copy link
Collaborator Author

That is what plotMsData produces:

kc_3_alanine_3

so, it's the m/z against retention time at bottom (intensity color-coded) and on top of that the base peak chromatogram (highest signal per spectrum). Makes mostly sense for MS1 and if you have LC-MS data.

@lgatto lgatto merged commit 061c324 into master Feb 18, 2018
@lgatto lgatto mentioned this pull request Feb 18, 2018
@jorainer jorainer deleted the extractMsData branch February 19, 2018 04:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants