{"action":"created","issue":{"url":"https://api.github.com/repos/lgatto/MSnbase/issues/150","repository_url":"https://api.github.com/repos/lgatto/MSnbase","labels_url":"https://api.github.com/repos/lgatto/MSnbase/issues/150/labels{/name}","comments_url":"https://api.github.com/repos/lgatto/MSnbase/issues/150/comments","events_url":"https://api.github.com/repos/lgatto/MSnbase/issues/150/events","html_url":"https://github.com/lgatto/MSnbase/issues/150","id":170910205,"number":150,"title":"pickPeaks and estimateNoise use different `isCentroided` methods?!","user":{"login":"sgibb","id":1828443,"avatar_url":"https://avatars.githubusercontent.com/u/1828443?v=3","gravatar_id":"","url":"https://api.github.com/users/sgibb","html_url":"https://github.com/sgibb","followers_url":"https://api.github.com/users/sgibb/followers","following_url":"https://api.github.com/users/sgibb/following{/other_user}","gists_url":"https://api.github.com/users/sgibb/gists{/gist_id}","starred_url":"https://api.github.com/users/sgibb/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sgibb/subscriptions","organizations_url":"https://api.github.com/users/sgibb/orgs","repos_url":"https://api.github.com/users/sgibb/repos","events_url":"https://api.github.com/users/sgibb/events{/privacy}","received_events_url":"https://api.github.com/users/sgibb/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/lgatto/MSnbase/labels/bug","name":"bug","color":"e10c02"}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":2,"created_at":"2016-08-12T16:33:39Z","updated_at":"2016-08-12T22:06:25Z","closed_at":null,"body":"In `pickPeaks` you find the following lines to test for centroided mode:\r\n[functions-Spectrum.R:395](https://github.com/lgatto/MSnbase/blob/a942daa309c5f65180556c8d5399fe33b2e1fd68/R/functions-Spectrum.R#L395)\r\n```r\r\nif (!ignoreCentroided & isTRUE(object@centroided)) {\r\n```\r\nand in `estimateNoise`:\r\n[functions-Spectrum.R:376](https://github.com/lgatto/MSnbase/blob/a942daa309c5f65180556c8d5399fe33b2e1fd68/R/functions-Spectrum.R#L376)\r\n```r\r\nif (!ignoreCentroided & (object@centroided | is.na(object@centroided))) {\r\n```\r\n\r\nThese lines are the reason why the example of `?plot.Spectrum.Spectrum` is not working anymore:\r\n[man/plotSpectrumSpectrum-methods.Rd:62-73](https://github.com/lgatto/MSnbase/blob/28839adfcfd8df0dfee4a836e005192699853019/man/plotSpectrumSpectrum-methods.Rd#L62-L73)\r\n```r\r\nfile <- dir(system.file(package = \"MSnbase\", dir = \"extdata\"),\r\n            full.name = TRUE, pattern = \"mzXML$\")\r\n\r\n## create basic MSnExp\r\nmsexp <- readMSData(file)\r\n\r\n## centroid them\r\nmsexp <- pickPeaks(msexp)\r\n```\r\nWhile the test in `pickPeaks` results in a non-centroided spectrum, the test in `estimateNoise` assumes a centroided on and returns a matrix of `NA`. That's why SNR comparison fails and no peak is found, even worse an empty spectrum is returned.\r\n\r\nWe should replace both tests (and maybe more in other functions) by a `isCentroided` method that handle these cases. The current `isCentroided,Spectrum-method` uses the `quantile` function and doesn't investigate the `centroided` slot. Using the quantile method in all subroutines would unnecessarily slow down the processing. So we should rely on the slot, or?"},"comment":{"url":"https://api.github.com/repos/lgatto/MSnbase/issues/comments/239571236","html_url":"https://github.com/lgatto/MSnbase/issues/150#issuecomment-239571236","issue_url":"https://api.github.com/repos/lgatto/MSnbase/issues/150","id":239571236,"user":{"login":"lgatto","id":384198,"avatar_url":"https://avatars.githubusercontent.com/u/384198?v=3","gravatar_id":"","url":"https://api.github.com/users/lgatto","html_url":"https://github.com/lgatto","followers_url":"https://api.github.com/users/lgatto/followers","following_url":"https://api.github.com/users/lgatto/following{/other_user}","gists_url":"https://api.github.com/users/lgatto/gists{/gist_id}","starred_url":"https://api.github.com/users/lgatto/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lgatto/subscriptions","organizations_url":"https://api.github.com/users/lgatto/orgs","repos_url":"https://api.github.com/users/lgatto/repos","events_url":"https://api.github.com/users/lgatto/events{/privacy}","received_events_url":"https://api.github.com/users/lgatto/received_events","type":"User","site_admin":false},"created_at":"2016-08-12T22:06:25Z","updated_at":"2016-08-12T22:06:25Z","body":"Or, we could add a strict argument to `centroided`:\r\n\r\n```r\r\ncentroided(x) ## NA\r\ncentroided(x, strict = TRUE) ## error\r\n```\r\n\r\n\r\n\r\n"}}
{"actor":{"display_login":"lgatto"}}