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

RMB_EIC_prescreen: Complain when RMB settings not loaded. #6

Open
MaliRemorker opened this issue Jun 21, 2019 · 5 comments
Open

RMB_EIC_prescreen: Complain when RMB settings not loaded. #6

MaliRemorker opened this issue Jun 21, 2019 · 5 comments

Comments

@MaliRemorker
Copy link

Right now, the thrown error sounds misleading:

Error in if (!is.na(deprofile.setting)) msPeaks <- deprofile.scan(msPeaks,  :
  argument is of length zero
@schymane
Copy link
Owner

My suggestion would be to add a test right at the top of the function and then either exit and ask people to load the settings first [then also need to update documentation] or invoke RmbDefaultSettings() and continue with a warning.
The problem with the latter is that people should ideally be doing the pre-screening using the settings they will then use for the RMassBank workflow.
Alternative: settings file is an input. Thoughts?

@MaliRemorker
Copy link
Author

MaliRemorker commented Jun 21, 2019

Absolutely the former. We must not assume default settings are the right settings.

@schymane
Copy link
Owner

@meowcat do you know offhand how to test whether settings are loaded in RMB?
Can't find it in the documentation

@schymane
Copy link
Owner

@meowcat
Copy link

meowcat commented Jun 22, 2019

yes, though that function is of course not exported, so either ::: or directly is.null(getOption("RMassBank")).

Without looking at your specific code, I suggest this pattern we use in RMassBank:

findMsMsHR.direct <- function(msRaw, cpdID, mode = "pH", confirmMode = 0, useRtLimit = TRUE, 
			ppmFine = getOption("RMassBank")$findMsMsRawSettings$ppmFine,
			mzCoarse = getOption("RMassBank")$findMsMsRawSettings$mzCoarse,
			fillPrecursorScan = getOption("RMassBank")$findMsMsRawSettings$fillPrecursorScan,
			rtMargin = getOption("RMassBank")$rtMargin,
			deprofile = getOption("RMassBank")$deprofile,
			headerCache = NULL) { do.stuff() }

In this way, it is never opaque that there are parameters being accessed in a specific function call. Doesn't need to be as granular as this, settings = getOption("RMassBank") would be simpler.

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

No branches or pull requests

3 participants