robinandeer / puzzle

Variant caller GUI + genetic disease analysis
https://robinandeer.gitbooks.io/puzzle/content/
MIT License
22 stars 7 forks source link

Added inheritance filter to gemini plugin #223

Open moonso opened 8 years ago

moonso commented 8 years ago

Finally works!! Please review @brainstorm @guillermo-carrasco @robinandeer

brainstorm commented 8 years ago

Cool stuff Måns! Just went through the PR, seems fine to me except the HapMapMany test/comment I left... It would make sense to have an exception for pulling the data from S3:

When tests are running on TravisCI, do not pull those in.

Other than that, excellent job! ;)

moonso commented 8 years ago

I have no idea why the tests fails, it's working on my local installation... Guess it has to do with different gemini versions, can anyone help out @brainstorm @guillermo-carrasco @robinandeer ??

moonso commented 8 years ago

@robinandeer would you like to have a last review of this? The tests fails because of this https://github.com/arq5x/gemini/issues/724#issuecomment-216905331, is it possible to get around in some way? Please merge so I can do a release

guillermo-carrasco commented 8 years ago

hmm I don't even get what's the problem exactly, the stacktrace is confusing :-/

moonso commented 8 years ago

@guillermo-carrasco think that the problem has to do with gemini.gim uses eval, that in combination with pytest cov make some wierd things happen

robinandeer commented 8 years ago

eval...

captain-picard-meme-dafuq

robinandeer commented 8 years ago

How are we standing on this?

Doesn't GEMINI do a test for this case or are they not using py.test?

I don't like the idea of merging in code that will make tests fail just bc having a "broken master branch" tag in the README is not a good thing for potential collaborators etc. to see

@moonso @brainstorm @guillermo-carrasco @henrikstranneheim ??

brainstorm commented 8 years ago

I had to deal with GEMINI's "testsuite" in PR https://github.com/arq5x/gemini/pull/719... TL;DR: Not fun.

Best chance you guys get is pinging BrentP on this if you are really stuck, IMHO...

robinandeer commented 8 years ago

Yeah let's bring in BrentP and see what he has to say

Otherwise I guess we'll drop that unit test and/or work around it 😕