fix: custom installation of DOMINO #235
Conversation
[this has the added surprise that the pypi version of DOMINO is different than the one on GitHub!] This installs DOMINO locally, resolving Reed-CompBio#137. However, I would like to tackle Reed-CompBio#119 and Reed-CompBio#234, and I'm still missing test cases to show the resolution of Reed-CompBio#137. This patches DOMINO with two patch files that separate runner.py (since before the behavior was reliant on setup.py) and move module imports outside of `src`.
|
you can try using the egfr test case |
|
I'll try that out! |
this works!
|
That was a great test case - I got a new |
There was a problem hiding this comment.
@agitter would using the egfr data as a test case cause the egfr data to be apart of the pilot data? The data is also used in the summary.py test cases.
The wording of the pilot data is: Can be included to establish proof of concept, effect size estimations, or feasibility of proposed methods. The feasibility of proposed methods is vague but my worry.
There was a problem hiding this comment.
I tried using a random graph earlier but that didn't work - however, since I know the size of a dataset to use, I can try to artificially induce node hotspots in a randomly generated graph and trick DOMINO into believing it is a biological network, which would work as a test case (as this isn't a test of "is DOMINO a good AMI algorithm," but rather "does DOMINO even run" - none of our test cases before actually made DOMINO run.
[To properly make the test cases work, I would need to read stderr but the stderr parsing is in #220 - for now, the tests require manual verification that its output does not include errors, which, locally for me, it doesn't.]
I wasn't aware that the EGFR dataset was pilot data - I can absolutely create a little tool to make artifical hotspots in a graph if you think that's a good idea 👍
There was a problem hiding this comment.
We don't want the egfr dataset to be pilot data, which is the worry. However, I would wait to add in a different test case for now.
also adjusts the docker container to move patch copies after requirements installation
|
I'll mark this PR as ready for review, though the data set should be swapped out. As of now, this PR enables singularity support for DOMINO [this could help circumvent #234 as one could use Singularity instead of Docker - I'll still try to replicate that issue locally in the meantime] |
|
I forgot to update the non-singularity tests. |
|
The tests for this worked locally, but that was because the generate inputs test suite never cleaned up its inputs before running the test. Pushing a commit to fix that 👍 |
[this has the added surprise that the pypi version of DOMINO is different than the one on GitHub!]
This installs DOMINO locally, which closes #137.
However, I would like to tackle #119 and #234, and I'm still missing test cases to show the resolution of #137. (I only tested it on the ridiculously large benchmark input, and I would love to have some medium-sized input to test this on.)This patches DOMINO with two patch files that separate runner.py (since before the behavior was reliant on setup.py) and move module imports outside of
src.This also makes a new change that allows for required inputs to have custom extensions.
though I have yet to test this on the snakemake side.