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.

Sign up for GitHub

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

Jump to bottom

removeReporters,OnDiskMSnExp #161

Closed
lgatto opened this issue Oct 7, 2016 · 8 comments
Closed

removeReporters,OnDiskMSnExp #161

lgatto opened this issue Oct 7, 2016 · 8 comments
Labels
MSnbase2

Comments

@lgatto
Copy link
Owner

lgatto commented Oct 7, 2016 •
edited
Loading

Added a removeReporters,OnDiskMSnExp that coerces to an MSnSet in bab8b0d. Needs to use lazy processing.
@lgatto lgatto added the MSnbase2 label Oct 7, 2016
@lgatto
Copy link
Owner Author

lgatto commented Oct 7, 2016

cc @jotsetung

@jorainer
Copy link
Collaborator

jorainer commented Oct 7, 2016

I'll have a look at it.

jorainer added a commit that referenced this issue Oct 7, 2016
@jorainer
Implement removePeaks,OnDiskMSnExp
cc91d58 
o Implement the removePeaks method for OnDiskMSnExp (issue #161) adding
  the operation to the processing queue.
o Unit test not yet implemented due to lack of test data.
jorainer added a commit that referenced this issue Oct 10, 2016
@jorainer
Tix removeReporters,OnDiskMSnExp
8d7ec82 
o Fix the removeReporters,OnDiskMSnExp method (see issue #161).
o Add unit test.
@jorainer
Copy link
Collaborator

Simply adding removeReporters to the lazy processing queue does not work for OnDiskMSnExp with MS1 and MSn spectra, because as soon as a Spectrum1 is processed an error is thrown by the removeReporters,Spectrum class. I applied the following modification for it to work (in commit 8d7ec82):

  • removeReporters,Spectrum does not throw an error is msLevel == 1, but a warning.
  • The removeReporters,Spectrum has an internal parameter suppressWarning that allows to suppress the above warning. This is because we don't want to get a warning message for each Spectrum1 in an OnDiskMSnExp object. I went for an internal parameter, as this parameter should not be visible to the user.
  • The removeReporters,MSnExp throws an error if all spectra are MS1.
  • The removeReporters,OnDiskMSnExp throws an error if all spectra are MS1.

Are you OK with that?

@lgatto
Copy link
Owner Author

lgatto commented Oct 10, 2016

Looks good, thanks.

@lgatto
Copy link
Owner Author

lgatto commented Oct 11, 2016

@jotsetung - can we close this?

@jorainer
Copy link
Collaborator

happy to do that

@lgatto
Copy link
Owner Author

lgatto commented Oct 11, 2016

Closing issues feels so rewarding :-)

@jorainer
Copy link
Collaborator

Yes, true; but sorry, I didn't wanted to sneak that from you, if you want I can re-open and you can close it ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
MSnbase2
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@lgatto @jorainer