Convert C++ exceptions into Python exceptions
Closed this issue · 11 comments
So right now, difficulties parsing VCF files raise unhandled C++ exceptions that eventually lead to program termination. Ideally, we could wrap catch some of these exceptions so that the outcome is a python exception (IOError?) rather than a full stop. Below are two test cases that I've encountered:
import vcfnp
# echo "hey" > test.txt
v = vcfnp.variants("./test.txt")
[vcfnp] 2015-09-03 09:58:24.050742 :: caching is disabled
[vcfnp] 2015-09-03 09:58:24.050779 :: building array
error: no VCF header
import vcfnp
v = vcfnp.variants(".")
[vcfnp] 2015-09-03 09:57:32.435456 :: caching is disabled
[vcfnp] 2015-09-03 09:57:32.435487 :: building array
terminate called after throwing an instance of 'std::out_of_range'
what(): basic_string::substr
~/opt/bin/python.app: line 3: 84448 Abort trap: 6
Thanks, good idea. I'll take a look, but PR or suggestions welcome.
On Thursday, September 3, 2015, Kyle Beauchamp notifications@github.com
wrote:
So right now, difficulties parsing VCF files raise unhandled C++
exceptions that eventually lead to program termination. Ideally, we could
wrap catch some of these exceptions so that the outcome is a python
exception (IOError?) rather than a full stop. Below are two test cases that
I've encountered:import vcfnp
echo "hey" > test.txt
v = vcfnp.variants("./test.txt")
[vcfnp] 2015-09-03 09:58:24.050742 :: caching is disabled
[vcfnp] 2015-09-03 09:58:24.050779 :: building array
error: no VCF headerimport vcfnp
v = vcfnp.variants(".")
[vcfnp] 2015-09-03 09:57:32.435456 :: caching is disabled
[vcfnp] 2015-09-03 09:57:32.435487 :: building array
terminate called after throwing an instance of 'std::out_of_range'
what(): basic_string::substr
~/opt/bin/python.app: line 3: 84448 Abort trap: 6—
Reply to this email directly or view it on GitHub
#48.
Alistair Miles
Head of Epidemiological Informatics
Centre for Genomics and Global Health http://cggh.org
The Wellcome Trust Centre for Human Genetics
Roosevelt Drive
Oxford
OX3 7BN
United Kingdom
Web: http://purl.org/net/aliman
Email: alimanfoo@googlemail.com alimanfoo@gmail.com
Tel: +44 (0)1865 287721
So the system exit calls are coming from inside vcflib. The simplest option is to convert all exit(1) calls within vcflib/Variant.cpp to throw an exception instead. Then in the Cython we can convert these to Python exceptions.
I've pushed a couple of changes to master that should do this for the cases you give: af0160d, alimanfoo/vcflib@d1f925f
However, vcflib is littered with exit(1) calls. I will work through and replace them all with an exception throw. This will diverge my vcflib even further from @ekg, but I don't think Erik has much time for maintaining vcflib, so it's probably not worth trying to bring them back together anyway.
It is absolutely worth working together and avoiding a fork. I just have
arranged funding to support vcflib (and freebayes), so I will continue to
work on it. I am also using it in several other projects.
I think it is a great idea to switch to using exceptions. Exit calls are a
real hack, sorry for the trouble they're causing. How can I help?
On Sep 4, 2015 10:36 AM, "Alistair Miles" notifications@github.com wrote:
So the system exit calls are coming from inside vcflib. The simplest
option is to convert all exit(1) calls within vcflib/Variant.cpp to throw
an exception instead. Then in the Cython we can convert these to Python
exceptions.I've pushed a couple of changes to master that should do this for the
cases you give: af0160d
af0160d,
alimanfoo/vcflib@d1f925f
alimanfoo/vcflib@d1f925fHowever, vcflib is littered with exit(1) calls. I will work through and
replace them all with an exception throw. This will diverge my vcflib even
further from @ekg https://github.com/ekg, but I don't think Erik has
much time for maintaining vcflib anyway, so it's probably not worth trying
to bring them back together anyway.—
Reply to this email directly or view it on GitHub
#48 (comment).
Thanks @ekg, that's great news. Would it be possible to replace the exit(1) calls in vcflib with an exception throw? If you could do that, I could rebase and send you PRs for the other changes I have (minor bug fixes and convenience, nothing substantial)?
Yes absolutely. Do you have a recommended pattern? Are there any specific
cases which you would like to handle?
On Sep 4, 2015 10:43 AM, "Alistair Miles" notifications@github.com wrote:
Thanks @ekg https://github.com/ekg, that's great news. Would it be
possible to replace the exit(1) calls in vcflib with an exception throw? If
you could do that, I could rebase and send you PRs for the other changes I
have (minor bug fixes and convenience, nothing substantial)?—
Reply to this email directly or view it on GitHub
#48 (comment).
I'm not a native C++ coder so don't have a recommended pattern, as long as some kind of exception is thrown then I'm good. In alimanfoo/vcflib@d1f925f I threw a runtime error but whatever you think is best.
This came up because of VCF parsing errors, so I'd like to be able to gracefully handle those. But I think the main issue is to get rid of all the exit() calls. They are very annoying for anyone trying to use this in an interactive environment, e.g., IPython notebook, where currently a VCF parsing error would cause the IPython kernel to exit, so the user loses everything in the current session.
Yeah that is not good for ipython or embedded environments. Point taken. At
least some find/replace action should resolve things.
On Sep 4, 2015 11:36 AM, "Alistair Miles" notifications@github.com wrote:
I'm not a native C++ coder so don't have a recommended pattern, as long as
some kind of exception is thrown then I'm good. In alimanfoo/vcflib@
d1f925f alimanfoo/vcflib@d1f925f I threw a
runtime error but whatever you think is best.This came up because of VCF parsing errors, so I'd like to be able to
gracefully handle those. But I think the main issue is to get rid of all
the exit() calls. They are very annoying for anyone trying to use this in
an interactive environment, e.g., IPython notebook, where currently a VCF
parsing error would cause the IPython kernel to exit, so the user loses
everything in the current session.—
Reply to this email directly or view it on GitHub
#48 (comment).
I will dig around for examples of how to catch c++ exeptions in python
wrappers via cython.
On Sep 4, 2015 3:43 AM, "Erik Garrison" notifications@github.com wrote:
Yeah that is not good for ipython or embedded environments. Point taken. At
least some find/replace action should resolve things.
On Sep 4, 2015 11:36 AM, "Alistair Miles" notifications@github.com
wrote:I'm not a native C++ coder so don't have a recommended pattern, as long
as
some kind of exception is thrown then I'm good. In alimanfoo/vcflib@
d1f925f alimanfoo/vcflib@d1f925f I threw a
runtime error but whatever you think is best.This came up because of VCF parsing errors, so I'd like to be able to
gracefully handle those. But I think the main issue is to get rid of all
the exit() calls. They are very annoying for anyone trying to use this in
an interactive environment, e.g., IPython notebook, where currently a VCF
parsing error would cause the IPython kernel to exit, so the user loses
everything in the current session.—
Reply to this email directly or view it on GitHub
#48 (comment).—
Reply to this email directly or view it on GitHub
#48 (comment).
Thanks Kyle. I did find
http://docs.cython.org/src/userguide/wrapping_CPlusPlus.html - it looks
like you can use declarations in the pxd to control how Cython maps c++
exceptions into python.
On Friday, September 4, 2015, Kyle Beauchamp notifications@github.com
wrote:
I will dig around for examples of how to catch c++ exeptions in python
wrappers via cython.
On Sep 4, 2015 3:43 AM, "Erik Garrison" <notifications@github.com
javascript:_e(%7B%7D,'cvml','notifications@github.com');> wrote:Yeah that is not good for ipython or embedded environments. Point taken.
At
least some find/replace action should resolve things.
On Sep 4, 2015 11:36 AM, "Alistair Miles" <notifications@github.com
javascript:_e(%7B%7D,'cvml','notifications@github.com');>
wrote:I'm not a native C++ coder so don't have a recommended pattern, as long
as
some kind of exception is thrown then I'm good. In alimanfoo/vcflib@
d1f925f alimanfoo/vcflib@d1f925f I threw a
runtime error but whatever you think is best.This came up because of VCF parsing errors, so I'd like to be able to
gracefully handle those. But I think the main issue is to get rid of
all
the exit() calls. They are very annoying for anyone trying to use this
in
an interactive environment, e.g., IPython notebook, where currently a
VCF
parsing error would cause the IPython kernel to exit, so the user loses
everything in the current session.—
Reply to this email directly or view it on GitHub
#48 (comment).—
Reply to this email directly or view it on GitHub
#48 (comment).—
Reply to this email directly or view it on GitHub
#48 (comment).
Alistair Miles
Head of Epidemiological Informatics
Centre for Genomics and Global Health http://cggh.org
The Wellcome Trust Centre for Human Genetics
Roosevelt Drive
Oxford
OX3 7BN
United Kingdom
Web: http://purl.org/net/aliman
Email: alimanfoo@googlemail.com alimanfoo@gmail.com
Tel: +44 (0)1865 287721
I've removed all exit(1) in the main vcflib source code. There are still some exit(1) calls in vcflib submodules (tabixpp, fastahack) but I'll leave those for now.
Btw I've done this on my vcflib fork as there are other differences between mine and upstream which I don't have time to resolve at the moment.