Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-827

Reimplement C++/Python Exception Translation

    XMLWordPrintable

    Details

      Description

      I'd like to reimplement our Swig bindings for C++ exceptions to replace the "LsstCppException" class with a more user-friendly mechanism. We'd have a Python exception hierarchy that mirrors the C++ hierarchy (generated automatically with the help of a few Swig macros). These wrapped exceptions could be thrown in Python as if they were pure-Python exceptions, and could be caught in Python in the same language regardless of where they were thrown.

      We're doing this as part of a "Measurement" sprint because we'd like to define custom exceptions for different kinds of common measurement errors, and we want to be able to raise those exceptions in either language.

        Attachments

          Issue Links

            Activity

            Hide
            ktl Kian-Tat Lim added a comment -

            FYI, gcov shows that Tracepoint::Tracepoint(void) and Exception::getTraceback() are untested. Also the two Exception::addMessage() cases involving a plain semicolon (one strange, one requiring two addMessage() calls) are not tested.

            A Python coverage tool shows that wrappers.ExceptionMeta.__getattr__() is not called. The PyType is None case in translate() isn't called either, but I think that's mostly present for safety.

            It's not easy to determine test coverage of .i files, unfortunately.

            Show
            ktl Kian-Tat Lim added a comment - FYI, gcov shows that Tracepoint::Tracepoint(void) and Exception::getTraceback() are untested. Also the two Exception::addMessage() cases involving a plain semicolon (one strange, one requiring two addMessage() calls) are not tested. A Python coverage tool shows that wrappers.ExceptionMeta.__getattr__() is not called. The PyType is None case in translate() isn't called either, but I think that's mostly present for safety. It's not easy to determine test coverage of .i files, unfortunately.
            Hide
            ktl Kian-Tat Lim added a comment -

            To look at the gcov output (as processed by lcov), see http://www.slac.stanford.edu/~ktl/exceptions-coverage/

            Show
            ktl Kian-Tat Lim added a comment - To look at the gcov output (as processed by lcov ), see http://www.slac.stanford.edu/~ktl/exceptions-coverage/
            Hide
            ktl Kian-Tat Lim added a comment -

            And to look at the coverage.py output, see http://www.slac.stanford.edu/~ktl/exceptions-pycov/

            Show
            ktl Kian-Tat Lim added a comment - And to look at the coverage.py output, see http://www.slac.stanford.edu/~ktl/exceptions-pycov/
            Hide
            jbosch Jim Bosch added a comment -
            • utils: the "#if !defined(NO_SWIG_LSST_EXCEPTIONS)" went away; is this a problem?

            Pretty sure it isn't; this certainly wasn't enabled in any downstream packages, and I suspect it's a relic of a time when Swig's exception handling was less reliable. It'd be easy enough to add back in if you think we need it.

            • utilsLib.i: are you sure utils.version() wasn't used?

            Very. This code would have only worked with SVN anyhow, and this sort of boilerplate was replaced by a new mechanism in sconsUtils back in Winter 2012. I don't know how the stuff in utils survived that purge.

            FYI, gcov shows that Tracepoint::Tracepoint(void) and Exception::getTraceback() are untested. Also the two Exception::addMessage() cases involving a plain semicolon (one strange, one requiring two addMessage() calls) are not tested.

            I think I actually know enough about Swig to be able to remove the Tracepoint default ctor (i.e. I think Swig's std::vector wrappers was the only reason it was ever defined). I'll try to add tests for at least the two addMessage() calls edge cases; I don't think the other is worth the effort.

            A Python coverage tool shows that wrappers.ExceptionMeta._getattr_() is not called. The PyType is None case in translate() isn't called either, but I think that's mostly present for safety.

            Should be easy to add a test for the _getattr_ forwarding. I'll do it.

            Show
            jbosch Jim Bosch added a comment - utils : the " #if !defined(NO_SWIG_LSST_EXCEPTIONS) " went away; is this a problem? Pretty sure it isn't; this certainly wasn't enabled in any downstream packages, and I suspect it's a relic of a time when Swig's exception handling was less reliable. It'd be easy enough to add back in if you think we need it. utilsLib.i : are you sure utils.version() wasn't used? Very. This code would have only worked with SVN anyhow, and this sort of boilerplate was replaced by a new mechanism in sconsUtils back in Winter 2012. I don't know how the stuff in utils survived that purge. FYI, gcov shows that Tracepoint::Tracepoint(void) and Exception::getTraceback() are untested. Also the two Exception::addMessage() cases involving a plain semicolon (one strange, one requiring two addMessage() calls) are not tested. I think I actually know enough about Swig to be able to remove the Tracepoint default ctor (i.e. I think Swig's std::vector wrappers was the only reason it was ever defined). I'll try to add tests for at least the two addMessage() calls edge cases; I don't think the other is worth the effort. A Python coverage tool shows that wrappers.ExceptionMeta._ getattr _() is not called. The PyType is None case in translate() isn't called either, but I think that's mostly present for safety. Should be easy to add a test for the _ getattr _ forwarding. I'll do it.
            Hide
            jbosch Jim Bosch added a comment -

            I went ahead and tested all the addMessage() cases. It looks like __getattr__ actually was tested already (e.g. when we call err.what() instead of err.cpp.what()); the coverage tool must have had a hard time recognizing that.

            Show
            jbosch Jim Bosch added a comment - I went ahead and tested all the addMessage() cases. It looks like __getattr__ actually was tested already (e.g. when we call err.what() instead of err.cpp.what() ); the coverage tool must have had a hard time recognizing that.

              People

              Assignee:
              jbosch Jim Bosch
              Reporter:
              jbosch Jim Bosch
              Reviewers:
              Kian-Tat Lim
              Watchers:
              Jim Bosch, Kian-Tat Lim, Russell Owen, Simon Krughoff
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.