Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Substituted sys.exit for corresponding exception, depending on the context. #719

Closed
wants to merge 3 commits into from
Closed

Conversation

brainstorm
Copy link
Contributor

Thanks @brentp for merging the previous related PR #712!

Here comes the followup on the exception system for GEMINI.

@robinandeer

@brentp
Copy link
Collaborator

brentp commented Apr 28, 2016

Do the tests pass? I had to make some changes after the last one

@brentp
Copy link
Collaborator

brentp commented Apr 28, 2016

Orherwise. I think this will be a great change for useability.

@brainstorm
Copy link
Contributor Author

Ugh, sorry @brentp, how does one run the tests in GEMINI? It's install-data.py and then master-test.sh? Naively tried with nosetests and assumed they were broken :_S

mba-3:gemini romanvg$ nosetests -v -s test/

----------------------------------------------------------------------
Ran 0 tests in 0.004s

OK

Will have a look at that soon, brb.

@brentp
Copy link
Collaborator

brentp commented Apr 28, 2016

bash master-test.sh

@brainstorm
Copy link
Contributor Author

Yeah, I'm running into some issues both running the gemini/install-data.py script as advised by master-test.sh and then running gemini update --dataonly which downloads a few beefy files... so it'll take a bit longer than I expected.

Making progress, will keep you posted.

Traceback (most recent call last):
  File "/home/rvalls/.miniconda/bin/gemini", line 6, in <module>
    exec(compile(open(__file__).read(), __file__, 'exec'))
  File "/home/rvalls/dev/gemini/gemini/scripts/gemini", line 6, in <module>
    gemini.gemini_main.main()
  File "/home/rvalls/dev/gemini/gemini/gemini_main.py", line 1186, in main
    args.func(parser, args)
  File "/home/rvalls/dev/gemini/gemini/gemini_main.py", line 198, in load_fn
Traceback (most recent call last):
  File "/home/rvalls/.miniconda/bin/gemini", line 6, in <module>
    exec(compile(open(__file__).read(), __file__, 'exec'))
  File "/home/rvalls/dev/gemini/gemini/scripts/gemini", line 6, in <module>
    gemini_load.load(parser, args)
  File "/home/rvalls/dev/gemini/gemini/gemini_load.py", line 42, in load
    gemini.gemini_main.main()
  File "/home/rvalls/dev/gemini/gemini/gemini_main.py", line 1186, in main
    annotations.load_annos(args)
  File "/home/rvalls/dev/gemini/gemini/annotations.py", line 199, in load_annos
    % anno_files[anno])
IOError: Gemini cannot open this annotation file: /home/rvalls/dev/gemini/anno_dir/gemini/data/hg19.gwas.bed.gz.
Have you installed the annotation files?  If so, have they been moved or deleted? Exiting...

For more details:
        http://gemini.readthedocs.org/en/latest/content/#installation.html\#installing-annotation-files

    args.func(parser, args)
  File "/home/rvalls/dev/gemini/gemini/gemini_main.py", line 198, in load_fn
    gemini_load.load(parser, args)
  File "/home/rvalls/dev/gemini/gemini/gemini_load.py", line 42, in load
    annotations.load_annos(args)
  File "/home/rvalls/dev/gemini/gemini/annotations.py", line 199, in load_annos
    % anno_files[anno])
IOError: Gemini cannot open this annotation file: /home/rvalls/dev/gemini/anno_dir/gemini/data/hg19.gwas.bed.gz.
Have you installed the annotation files?  If so, have they been moved or deleted? Exiting...

@brentp
Copy link
Collaborator

brentp commented Apr 28, 2016

I can pull it in and to the testing. I just use the gemini installer, then with the python that gemini installs, run python setup.py install from the git repo and then run the tests from there.

@brainstorm
Copy link
Contributor Author

Yes, same process here, it's just that those downloads will take a while to finish :/ Don't worry, I'll make tests pass no matter what.

Have you guys considered to run those tests on smaller datasets (i.e: https:/brainstorm/tiny-test-data, if applicable), and then running it in a CI environment like bcbio does?:

https:/chapmanb/bcbio-nextgen/blob/master/.travis.yml

Very handy to spot bugs early and keep the tests running automatically on each pullrequest.

@brentp
Copy link
Collaborator

brentp commented Apr 28, 2016

yes, I have been pulling gemini out into separate modules (inheritance, geneimpacts, etc) for easier testing and continuous integration. once we have the new loader and gemini isn't dependent on the specific annotations, it will be simpler to have it rely on a small subset.

that said, the functional tests in gemini are pretty comprehensive so it's nice to have them as a final defense.

@brainstorm
Copy link
Contributor Author

Wow, the output is quite sparse in that test... don't even know where to look at when tests fail :_S

[rvalls@node31 gemini]$ grep fail test_gemini.log
fail
fail
     annotate-tool.t15 ... fail

A bit more context:

< gene  impact  impact_severity biotype is_exonic       is_coding       is_lof
64a64
> gene  impact  impact_severity biotype is_exonic       is_coding       is_lof
fail
updated 10 variants
     annotate-tool.t1 ... ok
updated 10 variants
     annotate-tool.t2 ... ok
updated 2 variants
     annotate-tool.t3 ... ok
updated 2 variants
     annotate-tool.t4 ... ok
updated 2 variants
     annotate-tool.t5 ... ok
updated 2 variants
     annotate-tool.t6 ... ok
updated 2 variants
     annotate-tool.t7 ... ok
    annotate-tool.t8...\c
updated 2 variants
     annotate-tool.t8 ... ok
updated 2 variants
     annotate-tool.t9 ... ok
updated 2 variants
     annoate-tool.t10 ... ok
updated 2 variants
     annotate-tool.t11 ... ok
updated 2 variants
     annotate-tool.t12 ... ok
    annotate-tool.t13...\c
updated 2 variants
     annotate-tool.t13 ... ok
updated 2 variants
     annotate-tool.t14 ... ok
     annotate-tool.t15 ... fail
annotate-tool.t15

Any hints?

@brentp
Copy link
Collaborator

brentp commented Apr 28, 2016

so you can go and look at that test and see:

echo $'EXITING: The number of column names, numbers, types, and operations must match: [anno23], [4,5], [text,float], [last,mode]\n' > exp

so that's one that is now an exception. YOu can probably change that section to something like:

echo $'ValueError: EXITING: The number of column names, numbers, types, and operations must match: [anno23], [4,5], [text,float], [last,mode]\n' > exp

...[TEST HERE]

check <(tail -n1 obs) exp annotate-tool.t15

if a test fails you can just run that specific test file (annotate-tool.sh) and modify to see what happened).

@brainstorm
Copy link
Contributor Author

@brentp, test-effstring.sh and test-clinvar.sh still have some issues, not sure if related with the exceptions though, should investigate further.

@brentp
Copy link
Collaborator

brentp commented May 2, 2016

@brainstorm. I'm going to pull this in and figure out any remaining issues with the
tests unless you have any objections.

@brainstorm
Copy link
Contributor Author

Absolutely, go ahead. Sorry I couldn't figure out what's going on with clinvar/eff :/

@brentp
Copy link
Collaborator

brentp commented May 2, 2016

this is merged into master. thanks!

@brentp brentp closed this May 2, 2016
@brainstorm
Copy link
Contributor Author

Wow, thanks Brent! Wonder what was off with ClinVar et al :-S

@brainstorm brainstorm deleted the no_sys_exit branch May 3, 2016 09:53
@brainstorm
Copy link
Contributor Author

Now I see: 646109ce551a5

Thanks again!

@brentp
Copy link
Collaborator

brentp commented May 3, 2016

yeah, I introduced that change but didn't commit the change to tests. decide it was better to revert.

thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants