biocore-ntnu / epic

(DEPRECATED) epic: diffuse domain ChIP-Seq caller based on SICER
http://bioepic.readthedocs.io
MIT License
31 stars 6 forks source link

Improvements to file, tempdir, and command handling #41

Closed DarwinAwardWinner closed 7 years ago

DarwinAwardWinner commented 7 years ago

I've made a number of improvements to file handling, tempdir handling, and command handling. In the files I edited:

My original motivation for making these fixes was noticing that every invocation of epic used the same temporary directory for bedgraph files, which meant that if multiple instances of the script were running at once, it was possible that they could overwrite each others' temporary files, causing bigwig files to get associated with the wrong samples. This should no longer be an issue, since each invoocation creates its own randomly-named temporary directory.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.002%) to 64.787% when pulling 209a57f601642352db8907baff4dd3e26d4f9c87 on DarwinAwardWinner:bdg-tmpdir into a6387132fc1b68702e168813fb9b5c8dfcdf924e on endrebak:master.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-1.7%) to 63.049% when pulling 25b22c10bf2fc757d557a63419c70c58e492524e on DarwinAwardWinner:bdg-tmpdir into a6387132fc1b68702e168813fb9b5c8dfcdf924e on endrebak:master.

endrebak commented 7 years ago

Thanks, much appreciated, but I have been rewriting the bigwig code to use rtracklayer (from bioconda-bioconductor) so this might be superfluous :/

This isn't your typical bioinformatics package that does not get updated, so before PRs you might want to make an issue first :)

endrebak commented 7 years ago

Thanks for this, but in 0.1.19 I've improved the handling of bigwigs. Furthermore, your tests failed for 2.7 :)

DarwinAwardWinner commented 7 years ago

That's ok, I'm a heavy Bioconductor user, so I support that solution. But I see that your changes also removed the option for outputting single bigwigs for the sum of all inputs and all treatments. Is there a reason for that? I've been using that option in my workflow, and I'd like to know if there's a reason I shouldn't be using it.

endrebak commented 7 years ago

Sorry, I had no idea. I removed it because I thought I wasn't going to use it and that it was such a new feature that no-one else had either :)

Since the new code is much shorter and faster, I'll find a way to create the sums of bigwigs using it. For now, please use 0.1.18 for the time being.

On Mon, Oct 3, 2016 at 8:25 PM, Ryan C. Thompson notifications@github.com wrote:

That's ok, I'm a heavy Bioconductor user, so I support that solution. But I see that your changes also removed the option for outputting single bigwigs for the sum of all inputs and all treatments. Is there a reason for that? I've been using that option in my workflow, and I'd like to know if there's a reason I shouldn't be using it.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/endrebak/epic/pull/41#issuecomment-251185040, or mute the thread https://github.com/notifications/unsubscribe-auth/AQ9I0vvq_c0VLCn0N75MpTChQEKd93pvks5qwUibgaJpZM4KLnuS .

endrebak commented 7 years ago

pip install epic=0.1.20 should write sum of bigwigs. Not out in bioconda yet.