Uploaded image for project: 'Request For Comments'
  1. Request For Comments
  2. RFC-216

Use pybind11 instead of swig to generate Python wrappers for C++ code

    Details

    • Type: RFC
    • Status: Implemented
    • Resolution: Done
    • Component/s: DM
    • Labels:
      None

      Description

      This RFC proposes to use pybind11, instead of swig, to generate Python wrappers for C++ code in the LSST stack.

      This was originally motivated by the desire to have more Pythonic interfaces for C++ code (see RFC-81).
      In addition the Swig wrapper code is generally perceived to be complex and thus difficult to understand, extend, debug and maintain.

      To investigate alternative wrapper solutions DM-5470 created an example codebase with some tricky details likely to be encountered in the stack.
      This codebase was wrapped with CFFI (DM-5677), Cython (DM-5471) and pybind11 (DM-5676) with the latter found to be best by far (see http://dmtn-014.lsst.io/en/latest/ for more information).

      It was decided, in RFC-182, to attempt a trial conversion of the LSST stack (up to afw) which was done in DM-6168.
      The current status as well as the pros and cons of a potential switch from swig to pybind11 were presented on community.

      The main arguments for pybind11 are:

      • wrappers are explicit, what you see is what you get (no magic);
      • wrappers are just C++11 with no intermediate language so developers only need to know C++ (and Python);
      • easier to support more Pythonic interfaces, for instance with attributes and keyword arguments;
      • natural integration of docstrings describing the interface (as exposed to Python) into the wrapper file itself (if desired);
      • pybind11 is small and header only, which makes it relatively easy to maintain, debug and extend if necessary (which would probably be considerably harder with swig).

      Arguments against are:

      • significant conversion effort;
      • wrappers are verbose and require maintenance when the C++ interfaces change;
      • pybind11 is a younger and smaller project (then swig) and the risk of abandonment by its maintainers is therefore probably higher.

      For further technical background please see the community discussion as well as DMTN-014.

      This RFC intends to build consensus for a decision ultimately to be made by the System Architect.

        Attachments

          Issue Links

            Activity

            Hide
            rowen Russell Owen added a comment -

            I'd like to suggest that we find a time soon to have most of us stop working on anything in the stack that will interfere with the pybind11 migration (e.g. C++ code) and focus on a mass migration to pybind11. One question is whether we ought to finish the port to python 3 first (e.g. will that simplify the pybind11 transition?). If so, then I'll suggest we do that as well.

            Show
            rowen Russell Owen added a comment - I'd like to suggest that we find a time soon to have most of us stop working on anything in the stack that will interfere with the pybind11 migration (e.g. C++ code) and focus on a mass migration to pybind11. One question is whether we ought to finish the port to python 3 first (e.g. will that simplify the pybind11 transition?). If so, then I'll suggest we do that as well.
            Hide
            swinbank John Swinbank added a comment -

            We have some concerns about how to most effectively parallelize this work, especially since the bulk of it falls in one package (afw). One of the first tickets in the implementation epic will be for Pim to work with a small group (possibly just one other person) to figure out how easy it is for them to work on the same package without falling over each other. Once that's done, we'll be better able to figure out how to plan the longer-term implementation.

            If possible, I'd like this ticket to focus on whether this is technically the right thing to do rather than getting distracted into worrying about scheduling the work.

            Show
            swinbank John Swinbank added a comment - We have some concerns about how to most effectively parallelize this work, especially since the bulk of it falls in one package (afw). One of the first tickets in the implementation epic will be for Pim to work with a small group (possibly just one other person) to figure out how easy it is for them to work on the same package without falling over each other. Once that's done, we'll be better able to figure out how to plan the longer-term implementation. If possible, I'd like this ticket to focus on whether this is technically the right thing to do rather than getting distracted into worrying about scheduling the work.
            Hide
            tjenness Tim Jenness added a comment -

            Given the overwhelming positive response, can we add the implementation tickets and mark this as ADOPTED?

            Show
            tjenness Tim Jenness added a comment - Given the overwhelming positive response, can we add the implementation tickets and mark this as ADOPTED?
            Hide
            jbecla Jacek Becla added a comment -

            Pim should do this

            Show
            jbecla Jacek Becla added a comment - Pim should do this
            Hide
            swinbank John Swinbank added a comment -

            For reference – since I was muttering about adding new epics to cover the work etc, Pim has asked me to do that before we mark this as adopted. I'll do so as soon as I have brain cells to spare.

            Show
            swinbank John Swinbank added a comment - For reference – since I was muttering about adding new epics to cover the work etc, Pim has asked me to do that before we mark this as adopted. I'll do so as soon as I have brain cells to spare.

              People

              • Assignee:
                pschella Pim Schellart [X] (Inactive)
                Reporter:
                pschella Pim Schellart [X] (Inactive)
                Watchers:
                Dave Mills, Fritz Mueller, Gregory Dubois-Felsmann, Jacek Becla, Jim Bosch, John Parejko, John Swinbank, Joshua Meyers, Kian-Tat Lim, Pim Schellart [X] (Inactive), Russell Owen, Tim Jenness
              • Votes:
                4 Vote for this issue
                Watchers:
                12 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Planned End:

                  Summary Panel