epigen / RnBeads

Git working Repo synced to the Bioconductor: http://bioconductor.org/packages/devel/bioc/html/RnBeads.html
https://rnbeads.org/
7 stars 8 forks source link

pOOBAH and sesame by Nathan Steenbuck #12

Closed schmic05 closed 3 years ago

schmic05 commented 3 years ago

Hi @nathansteenbuck

I had a look into the code that you wrote. Thanks a lot for providing it. However, I found some issue:

nmasked = nmasked + length(mask) maskedIDs <- match(mask, probeIDs) raw.set@pval.sites[maskedIDs, i] <- NA raw.set@M[maskedIDs, i] <- NA raw.set@U[maskedIDs, i] <- NA raw.set@M0[maskedIDs, i]<- NA raw.set@U0[maskedIDs, i]<- NA

Did you also encounter these problems?

nathansteenbuck commented 3 years ago

Hi Michael,

thank you for your quick reply and your effort.

I think these issues have been caused by the version of the dependency sesame. I had oriented myself on sesame version 1.6, as available by Bioconductor version 3.11. For that, the wrapper has been working well for me. I will try to debug the code so that it has backward compatibility according to the oldest release that was available through Bioconductor (i.e. version 3.8), test it and then get back to you.

Best, Nathan

On Fri, 18 Dec 2020 at 13:54, Michael Scherer notifications@github.com wrote:

Hi @nathansteenbuck https://github.com/nathansteenbuck

I had a look into the code that you wrote. Thanks a lot for providing it. However, I found some issue:

  • detectionPoobEcdf does not have an parameter force
  • pvalues <- sigset.l[[i]]@pval$pOOBAH should be pvalues <- sigset.l[[i]]@pval
  • there are NAs in the vector maskedIDs, which causes these command to fail:

nmasked = nmasked + length(mask) maskedIDs <- match(mask, probeIDs) raw.set@pval.sites[maskedIDs, i] <- NA raw.set@M[maskedIDs, i] <- NA raw.set@U[maskedIDs, i] <- NA raw.set@M0[maskedIDs, i]<- NA raw.set@U0[maskedIDs, i]<- NA

Did you also encounter these problems?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/epigen/RnBeads/pull/12#issuecomment-748069221, or unsubscribe https://github.com/notifications/unsubscribe-auth/ARDACNH77I5JKAS3MM74D7LSVNGHVANCNFSM4VA4EGEA .

nathansteenbuck commented 3 years ago

Hi Michael,

I added some commits to fix the issues. I think it should work better now :)

Best regards and have a great start into 2021, Nathan

On Fri, 18 Dec 2020 at 13:54, Michael Scherer notifications@github.com wrote:

Hi @nathansteenbuck https://github.com/nathansteenbuck

I had a look into the code that you wrote. Thanks a lot for providing it. However, I found some issue:

  • detectionPoobEcdf does not have an parameter force
  • pvalues <- sigset.l[[i]]@pval$pOOBAH should be pvalues <- sigset.l[[i]]@pval
  • there are NAs in the vector maskedIDs, which causes these command to fail:

nmasked = nmasked + length(mask) maskedIDs <- match(mask, probeIDs) raw.set@pval.sites[maskedIDs, i] <- NA raw.set@M[maskedIDs, i] <- NA raw.set@U[maskedIDs, i] <- NA raw.set@M0[maskedIDs, i]<- NA raw.set@U0[maskedIDs, i]<- NA

Did you also encounter these problems?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/epigen/RnBeads/pull/12#issuecomment-748069221, or unsubscribe https://github.com/notifications/unsubscribe-auth/ARDACNH77I5JKAS3MM74D7LSVNGHVANCNFSM4VA4EGEA .

schmic05 commented 3 years ago

Hi @nathansteenbuck

Thanks for the fixes and for your contribution in general. I merged your branch into the master branch of RnBeads. The function will be available after the next Biocondutor release.