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

Check and fix scanrange sub-setting in findPeaks methods #62

Closed
jorainer opened this issue Sep 23, 2016 · 6 comments
Closed

Check and fix scanrange sub-setting in findPeaks methods #62

jorainer opened this issue Sep 23, 2016 · 6 comments
Assignees
Labels
Projects

Comments

@jorainer
Copy link
Collaborator

If scanrange argument is defined, the feature detection should only be performed in the specified scan range. This does not seem to be the case for all findPeaks.* methods!

@jorainer jorainer added the xcms3 label Sep 23, 2016
@jorainer jorainer self-assigned this Sep 23, 2016
@jorainer
Copy link
Collaborator Author

jorainer commented Sep 26, 2016

A [ method to subset xcmsRaw objects to specific scans/spectra has been added in commit f82432a . This can be used (instead of the split method) to subset the xcmsRaw object(s) prior to feature detection.

@jorainer
Copy link
Collaborator Author

findPeaks.matchedFilter does actually not subset on scanrange: see #63.

@jorainer
Copy link
Collaborator Author

jorainer commented Sep 28, 2016

There is also a problem with findPeaks.centWave, see #64

@sneumann
Copy link
Owner

It would be good to fix this for all the findPeaks.* methods.
What about fillPeaks() ? Since fillPeaks() uses information in seconds,
this should be fine. What about the getEIC() ? Likewise, they use seconds.
Would it make sense to deprecate the scanrange and replace by rtrange=...' to be consistent with other functions likeplotRaw()` ? Yours, Steffen

@jorainer
Copy link
Collaborator Author

Haven't looked specifically at fillPeaks yet, but there shouldn't be any problem there. As mentioned further above, I suggest to use the new [ subsetting method on the xcmsRaw object at the beginning of each method, if !missing(scanrange).
Regarding rtrange and/or scanrange: for now I would like to keep both, also because in MSnbase there is no sub-setting on rtrange implemented (yet).

@jorainer
Copy link
Collaborator Author

Closing: xcmsRaw objects are now sub-setted prior to feature detection for all findPeaks methods.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
3.0.0
Done
Development

No branches or pull requests

2 participants