Just a couple of comments: - it would be really nice is the other packages used the logging support built into the IMP kernel. This makes controlling the complexity and where it goes much easier.
- is there a reason that other modules don't use the imp kernel exception wrappers? Is this something to do with SWIG failing or a design decision? It seems to me uniformity in exception returns would also be quite nice.
Notification of IMP commits wrote: > Author: ben@SALILAB.ORG > Date: 2008-09-09 19:31:05 -0700 (Tue, 09 Sep 2008) > New Revision: 715 > > Added: > trunk/domino/test/test_restraints.py > Modified: > trunk/domino/include/SimpleDiscreteRestraint.h > trunk/domino/pyext/DOMINO.i > trunk/domino/src/SimpleDiscreteRestraint.cpp > Log: > - Have SimpleDiscreteRestraint's constructor throw a proper exception on reading > an invalid format restraints file. > - Add a test case for this exception. > - Have the Python wrapper translate exceptions to Python. > > > Modified: trunk/domino/include/SimpleDiscreteRestraint.h > =================================================================== > --- trunk/domino/include/SimpleDiscreteRestraint.h 2008-09-10 01:34:59 UTC (rev 714) > +++ trunk/domino/include/SimpleDiscreteRestraint.h 2008-09-10 02:31:05 UTC (rev 715) > @@ -27,6 +27,9 @@ > class IMPDOMINOEXPORT SimpleDiscreteRestraint : public Restraint > { > public: > + //! Constructor. > + /** \exception ErrorException the restraint file is of an invalid format. > + */ > SimpleDiscreteRestraint(Model& model_, std::string restraint_filename, > Particle *p1, Particle *p2); > > > Modified: trunk/domino/pyext/DOMINO.i > =================================================================== > --- trunk/domino/pyext/DOMINO.i 2008-09-10 01:34:59 UTC (rev 714) > +++ trunk/domino/pyext/DOMINO.i 2008-09-10 02:31:05 UTC (rev 715) > @@ -11,6 +11,7 @@ > %} > > %include "kernel/pyext/IMP_macros.i" > +%include "kernel/pyext/IMP_exceptions.i" > > /* Ignore shared object import/export stuff */ > #define DOMINODLLEXPORT > @@ -28,9 +29,6 @@ > /* Get definitions of kernel base classes (but do not wrap) */ > %import "kernel/pyext/IMP.i" > > -/* Don't use the exception handlers defined in the kernel */ > -%exception; > - > /* Wrap our own classes */ > %include "IMP/domino/DiscreteSampler.h" > %include "IMP/domino/CombState.h" > > Modified: trunk/domino/src/SimpleDiscreteRestraint.cpp > =================================================================== > --- trunk/domino/src/SimpleDiscreteRestraint.cpp 2008-09-10 01:34:59 UTC (rev 714) > +++ trunk/domino/src/SimpleDiscreteRestraint.cpp 2008-09-10 02:31:05 UTC (rev 715) > @@ -34,9 +34,10 @@ > atoi(v[1].c_str()))] > = atof(v[2].c_str()); > } else { > - std::cout << "SimpleDiscreteRestraint::load_restraints the line : " > - << line << " is of the wrong format" << std::endl; > - throw(1); > + std::ostringstream msg; > + msg << "SimpleDiscreteRestraint::load_restraints the line : " > + << line << " is of the wrong format"; > + IMP_failure(msg.str().c_str(), ErrorException); > } > } > } > > Added: trunk/domino/test/test_restraints.py > =================================================================== > --- trunk/domino/test/test_restraints.py (rev 0) > +++ trunk/domino/test/test_restraints.py 2008-09-10 02:31:05 UTC (rev 715) > @@ -0,0 +1,25 @@ > +import unittest > +import os > +import IMP > +import IMP.domino > +import IMP.test > + > +class RestraintTests(IMP.test.TestCase): > + """Tests for the SimpleDiscreteRestraint""" > + > + def test_invalid_restraints_file(self): > + """Loading an invalid restraints file should give an error""" > + fname = "invalid" > + m = IMP.Model() > + p1 = IMP.Particle() > + p2 = IMP.Particle() > + m.add_particle(p1) > + m.add_particle(p2) > + > + print >> file(fname, "w"), "garbage" > + self.assertRaises(RuntimeError, IMP.domino.SimpleDiscreteRestraint, > + m, fname, p1, p2) > + os.unlink(fname) > + > +if __name__ == '__main__': > + unittest.main() > > _______________________________________________ > IMP-commits mailing list > IMP-commits@salilab.org > https://salilab.org/mailman/listinfo/imp-commits >