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

Wrap pex_exceptions with pybind11

    XMLWordPrintable

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: pex_exceptions
    • Labels:
      None
    • Story Points:
      4
    • Epic Link:
    • Sprint:
      DRP F16-1, DRP F16-2
    • Team:
      Data Release Production

      Description

      While wrapping these packages, pay particular attention to exception translation (see Jim's bullet point 3: https://jira.lsstcorp.org/browse/RFC-182?focusedCommentId=48644&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-48644).

        Attachments

          Issue Links

          blocks

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-9187 port jointcal to pybind11

          • Done
          is blocked by

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-6783 Add support for deriving from Python exception types to pybind11

          • Done
          is parent task of

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-6591 Implement exception translators in upstream pybind11

          • Done

            Activity

            Ascending order - Click to sort in descending order
            No builds found.
            swinbank John Swinbank created issue -
            swinbank John Swinbank made changes -
            Field Original Value New Value
            Epic Link DM-6171 [ 24684 ]
            swinbank John Swinbank made changes -
            Epic Link DM-6171 [ 24684 ] DM-6168 [ 24680 ]
            pschella Pim Schellart [X] (Inactive) made changes -
            Link This issue is parent task of DM-6591 [ DM-6591 ]
            Hide
            Permalink
            pschella Pim Schellart [X] (Inactive) added a comment -

            DM-6591 design complete and reviewed. Can now use this design to try to wrap pex_exceptions.

            Show
            pschella Pim Schellart [X] (Inactive) added a comment - DM-6591 design complete and reviewed. Can now use this design to try to wrap pex_exceptions.
            pschella Pim Schellart [X] (Inactive) made changes -
            Status To Do [ 10001 ] In Progress [ 3 ]
            Hide
            Permalink
            pschella Pim Schellart [X] (Inactive) added a comment -

            Now passes all pex_exception unit tests, except for those that depend on deriving from Python Exception. So this has to be looked at next.

            Show
            pschella Pim Schellart [X] (Inactive) added a comment - Now passes all pex_exception unit tests, except for those that depend on deriving from Python Exception. So this has to be looked at next.
            pschella Pim Schellart [X] (Inactive) made changes -
            Watchers John Swinbank, Pim Schellart [ John Swinbank, Pim Schellart ] Jim Bosch, John Swinbank, Pim Schellart [ Jim Bosch, John Swinbank, Pim Schellart ]
            pschella Pim Schellart [X] (Inactive) made changes -
            Link This issue is blocked by DM-6783 [ DM-6783 ]
            pschella Pim Schellart [X] (Inactive) made changes -
            Story Points 12 6
            pschella Pim Schellart [X] (Inactive) made changes -
            Story Points 6 8
            pschella Pim Schellart [X] (Inactive) made changes -
            Summary Wrap base, pex_exceptions, utils with pybind11 Wrap pex_exceptions with pybind11
            pschella Pim Schellart [X] (Inactive) made changes -
            Story Points 8 4
            Hide
            Permalink
            pschella Pim Schellart [X] (Inactive) added a comment -

            Reduced scope to be just pex_exceptions for easier review.

            Show
            pschella Pim Schellart [X] (Inactive) added a comment - Reduced scope to be just pex_exceptions for easier review.
            Hide
            Permalink
            pschella Pim Schellart [X] (Inactive) added a comment -

            Now passes all unit tests with just pybind11 wrappers.
            Uses a similar workaround as with Swig to have a pure Python wrapper take care of deriving from Python Exception.
            Includes a new convenience function python::declare_exception to register custom exception types in other packages (similar in functionality to what we provided with the Swig %declareException macro).
            This all works in concert with the new exception translator support added to Python as part of DM-6783.

            Show
            pschella Pim Schellart [X] (Inactive) added a comment - Now passes all unit tests with just pybind11 wrappers. Uses a similar workaround as with Swig to have a pure Python wrapper take care of deriving from Python Exception. Includes a new convenience function python::declare_exception to register custom exception types in other packages (similar in functionality to what we provided with the Swig %declareException macro). This all works in concert with the new exception translator support added to Python as part of DM-6783 .
            pschella Pim Schellart [X] (Inactive) made changes -
            Reviewers Andy Salnikov, Jim Bosch [ salnikov, jbosch ]
            Status In Progress [ 3 ] In Review [ 10004 ]
            swinbank John Swinbank made changes -
            Sprint DRP F16-1 [ 225 ] TCAMS 2016_03, DRP F16-1 [ 209, 225 ]
            swinbank John Swinbank made changes -
            Rank Ranked higher
            swinbank John Swinbank made changes -
            Sprint TCAMS 2016_03, DRP F16-1 [ 209, 225 ] DRP F16-1, DRP F16-2 [ 225, 231 ]
            Hide
            Permalink
            salnikov Andy Salnikov added a comment -

            Pim, I finished my review, all comments are on PR. This is the first time I had to look at pex_exceptions, and I was a bit confused by multi-level wrapping. I'm still not sure I understand everything so some of my comments may be nonsense.

            Show
            salnikov Andy Salnikov added a comment - Pim, I finished my review, all comments are on PR. This is the first time I had to look at pex_exceptions , and I was a bit confused by multi-level wrapping. I'm still not sure I understand everything so some of my comments may be nonsense.
            Hide
            Permalink
            pschella Pim Schellart [X] (Inactive) added a comment - - edited

            Thanks for all your helpful suggestions. Definitely not nonsense I will try to clean things up a bit. I ported over too much of the direct Python API stuff from the Swig solution. This can be done much nicer with pybind11 RAII indeed.

            Show
            pschella Pim Schellart [X] (Inactive) added a comment - - edited Thanks for all your helpful suggestions. Definitely not nonsense I will try to clean things up a bit. I ported over too much of the direct Python API stuff from the Swig solution. This can be done much nicer with pybind11 RAII indeed.
            Hide
            Permalink
            pschella Pim Schellart [X] (Inactive) added a comment - - edited

            Merged (onto epic branch instead of master) after implementing comments.

            Show
            pschella Pim Schellart [X] (Inactive) added a comment - - edited Merged (onto epic branch instead of master) after implementing comments.
            pschella Pim Schellart [X] (Inactive) made changes -
            Resolution Done [ 10000 ]
            Status In Review [ 10004 ] Done [ 10002 ]
            Parejkoj John Parejko made changes -
            Link This issue blocks DM-9187 [ DM-9187 ]
            tjenness Tim Jenness made changes -
            Component/s pex_exceptions [ 10728 ]
            Component/s base [ 10716 ]

              People

              Assignee:
              pschella Pim Schellart [X] (Inactive)
              Reporter:
              swinbank John Swinbank
              Reviewers:
              Andy Salnikov, Jim Bosch
              Watchers:
              Andy Salnikov, Jim Bosch, John Swinbank, Pim Schellart [X] (Inactive)
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.