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

    XMLWordPrintable

    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

            No builds found.
            pschella Pim Schellart [X] (Inactive) created issue -
            pschella Pim Schellart [X] (Inactive) made changes -
            Field Original Value New Value
            Description This RFC proposes to use pybind11 (https://github.com/pybind/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 (https://community.lsst.org/t/using-pybind11-instead-of-swig-to-wrap-c-code/1096).

            The main arguments for pybind11 are:

            * wrappers are explicit, what you see is what you get (no magic);
            * wrappers are just C++11, 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 (https://community.lsst.org/t/using-pybind11-instead-of-swig-to-wrap-c-code/1096) as well as DMTN-014.

            This RFC intends to build consensus for a decision ultimately to be made by the System Architect.
            This RFC proposes to use [pybind11|https://github.com/pybind/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|https://community.lsst.org/t/using-pybind11-instead-of-swig-to-wrap-c-code/1096].

            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|https://community.lsst.org/t/using-pybind11-instead-of-swig-to-wrap-c-code/1096] as well as [DMTN-014|http://dmtn-014.lsst.io/en/latest/].

            This RFC intends to build consensus for a decision ultimately to be made by the System Architect.
            pschella Pim Schellart [X] (Inactive) made changes -
            Description This RFC proposes to use [pybind11|https://github.com/pybind/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|https://community.lsst.org/t/using-pybind11-instead-of-swig-to-wrap-c-code/1096].

            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|https://community.lsst.org/t/using-pybind11-instead-of-swig-to-wrap-c-code/1096] as well as [DMTN-014|http://dmtn-014.lsst.io/en/latest/].

            This RFC intends to build consensus for a decision ultimately to be made by the System Architect.
            This RFC proposes to use [pybind11|https://github.com/pybind/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|https://community.lsst.org/t/using-pybind11-instead-of-swig-to-wrap-c-code/1096].

            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|https://community.lsst.org/t/using-pybind11-instead-of-swig-to-wrap-c-code/1096] as well as [DMTN-014|http://dmtn-014.lsst.io/en/latest/].

            This RFC intends to build consensus for a decision ultimately to be made by the System Architect.
            Hide
            Parejkoj John Parejko added a comment -

            +1 (million) Considering the trouble that @rowen and I are having with wrapping VisitInfo in SWIG right now, I'm wholeheartedly in favor of this proposal.

            Show
            Parejkoj John Parejko added a comment - +1 (million) Considering the trouble that @rowen and I are having with wrapping VisitInfo in SWIG right now, I'm wholeheartedly in favor of this proposal.
            swinbank John Swinbank made changes -
            Description This RFC proposes to use [pybind11|https://github.com/pybind/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|https://community.lsst.org/t/using-pybind11-instead-of-swig-to-wrap-c-code/1096].

            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|https://community.lsst.org/t/using-pybind11-instead-of-swig-to-wrap-c-code/1096] as well as [DMTN-014|http://dmtn-014.lsst.io/en/latest/].

            This RFC intends to build consensus for a decision ultimately to be made by the System Architect.
            This RFC proposes to use [pybind11|https://github.com/pybind/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|https://community.lsst.org/t/using-pybind11-instead-of-swig-to-wrap-c-code/1096].

            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|https://community.lsst.org/t/using-pybind11-instead-of-swig-to-wrap-c-code/1096] as well as [DMTN-014|http://dmtn-014.lsst.io/en/latest/].

            This RFC intends to build consensus for a decision ultimately to be made by the System Architect.
            Hide
            swinbank John Swinbank added a comment -

            This RFC mentions the perpetual concern of "more Pythonic interfaces", but the work you've done to date has effectively been to replicate the existing Swig-based interface. If this RFC is accepted, would you suggest going ahead and reimplementing our entire Swig interface in pybind11 and then making another pass through the code to make it more idiomatic, or would you go back and reconsider the work done so far?

            Show
            swinbank John Swinbank added a comment - This RFC mentions the perpetual concern of "more Pythonic interfaces", but the work you've done to date has effectively been to replicate the existing Swig-based interface. If this RFC is accepted, would you suggest going ahead and reimplementing our entire Swig interface in pybind11 and then making another pass through the code to make it more idiomatic, or would you go back and reconsider the work done so far?
            Hide
            rowen Russell Owen added a comment -

            I'm in favor. I'm not keen on all the repetition, but simple repetition is better than magic.

            Show
            rowen Russell Owen added a comment - I'm in favor. I'm not keen on all the repetition, but simple repetition is better than magic.
            Hide
            pschella Pim Schellart [X] (Inactive) added a comment -

            John Swinbank I would consider the ticket to implement this RFC (if adopted and given the go-ahead by the System Architect) to be restricted to replicating the existing interfaces wherever possible. Thus continuing the work already done (conversion up to, and including part of, afw). This is done primarily to reduce the risk associated with changing unit test code at the same time as changing the code it tests. Then more Pythonic interfaces can be added over time when and where desired (subject to another RFC).

            That said, there are a few cases where modifying the existing interfaces is far easier then trying to replicate existing behaviour. The prime example is for code accepting sequence types . For those cases, and only those cases, I have opted to change the interface as well (changing the test type from tuple to list). I would suggest to continue with this strategy. If this is not desired, a separate decision is needed on how to deal with such cases.

            While the switch to pybind11 would not immediately give us more Pythonic interfaces (except for cases where keyword arguments are actually required to deal with default arguments in C++) it would make it far easier to generate them in the future. Gradually and as needed.

            Show
            pschella Pim Schellart [X] (Inactive) added a comment - John Swinbank I would consider the ticket to implement this RFC (if adopted and given the go-ahead by the System Architect) to be restricted to replicating the existing interfaces wherever possible. Thus continuing the work already done (conversion up to, and including part of, afw). This is done primarily to reduce the risk associated with changing unit test code at the same time as changing the code it tests. Then more Pythonic interfaces can be added over time when and where desired (subject to another RFC). That said, there are a few cases where modifying the existing interfaces is far easier then trying to replicate existing behaviour. The prime example is for code accepting sequence types . For those cases, and only those cases, I have opted to change the interface as well (changing the test type from tuple to list). I would suggest to continue with this strategy. If this is not desired, a separate decision is needed on how to deal with such cases. While the switch to pybind11 would not immediately give us more Pythonic interfaces (except for cases where keyword arguments are actually required to deal with default arguments in C++) it would make it far easier to generate them in the future. Gradually and as needed.
            Hide
            tjenness Tim Jenness added a comment -

            The big questions are going to be:

            • How much effort to do the rest?
            • How do we plan for the switch over if we can't have SWIG and pybind11 coexisting?

            Putting everything on the same ticket branch as you do now sounds like the only option but there's going to be a lot of rebasing as you try to keep up with people changing the packages from under you (as you've learned already as many of the packages that have been modified will need work to get them synced up with current master).

            Show
            tjenness Tim Jenness added a comment - The big questions are going to be: How much effort to do the rest? How do we plan for the switch over if we can't have SWIG and pybind11 coexisting? Putting everything on the same ticket branch as you do now sounds like the only option but there's going to be a lot of rebasing as you try to keep up with people changing the packages from under you (as you've learned already as many of the packages that have been modified will need work to get them synced up with current master).
            tjenness Tim Jenness made changes -
            Watchers Gregory Dubois-Felsmann, John Parejko, John Swinbank, Pim Schellart, Russell Owen, Tim Jenness [ Gregory Dubois-Felsmann, John Parejko, John Swinbank, Pim Schellart, Russell Owen, Tim Jenness ] Gregory Dubois-Felsmann, John Parejko, John Swinbank, Kian-Tat Lim, Pim Schellart, Russell Owen, Tim Jenness [ Gregory Dubois-Felsmann, John Parejko, John Swinbank, Kian-Tat Lim, Pim Schellart, Russell Owen, Tim Jenness ]
            Hide
            jbecla Jacek Becla added a comment -

            The wording "in the LSST stack" might be read as stack only, but I believe we want to do it for all LSST DM software (including Qserv, for example), and TimJ confirmed it in a side conversation in the qserv HC room. So I'd tweak the RFC description to make it clear it is for all DM software.

            Show
            jbecla Jacek Becla added a comment - The wording "in the LSST stack" might be read as stack only , but I believe we want to do it for all LSST DM software (including Qserv, for example), and TimJ confirmed it in a side conversation in the qserv HC room. So I'd tweak the RFC description to make it clear it is for all DM software.
            Hide
            gpdf Gregory Dubois-Felsmann added a comment - - edited

            It is also a question for the Python bindings for the OCS middleware. I do not recall whether Dave Mills investigated pybind11 in addition to swig and boost.python. I'll inquire.

            This is of course not an issue for the RFC per se.

            Show
            gpdf Gregory Dubois-Felsmann added a comment - - edited It is also a question for the Python bindings for the OCS middleware. I do not recall whether Dave Mills investigated pybind11 in addition to swig and boost.python. I'll inquire. This is of course not an issue for the RFC per se .
            Hide
            swinbank John Swinbank added a comment -

            If this RFC is accepted, I think it will be necessary to convert all of the Science Pipelines code to pybind11: even if some sort of coexistence with, or partial conversion from, Swig were possible, it would likely be confusing and chaotic. If & when the time comes, I'd expect the pipelines group to handle conversion of at least all of lsst_apps, and probably _distrib as well.

            It's less obvious what's necessary or desirable for Qserv or other parts of the codebase. As far as I know, the coupling with the Science Pipelines code is weaker, so it might not be necessary to convert them at the same time (or, indeed, at all). On the other hand, presumably many of the same arguments in favour of conversion would apply to them as to the Pipelines code. I wouldn't expect Pipelines developers to handle the conversion, though, so we'd have to negotiate with Fritz Mueller et al as to whether this was something which could be scheduled.

            Show
            swinbank John Swinbank added a comment - If this RFC is accepted, I think it will be necessary to convert all of the Science Pipelines code to pybind11: even if some sort of coexistence with, or partial conversion from, Swig were possible, it would likely be confusing and chaotic. If & when the time comes, I'd expect the pipelines group to handle conversion of at least all of lsst_apps, and probably _distrib as well. It's less obvious what's necessary or desirable for Qserv or other parts of the codebase. As far as I know, the coupling with the Science Pipelines code is weaker, so it might not be necessary to convert them at the same time (or, indeed, at all). On the other hand, presumably many of the same arguments in favour of conversion would apply to them as to the Pipelines code. I wouldn't expect Pipelines developers to handle the conversion, though, so we'd have to negotiate with Fritz Mueller et al as to whether this was something which could be scheduled.
            Hide
            tjenness Tim Jenness added a comment -

            It seems to me that it is highly desirable for all of DM to converge on a single binding solution so we do not have to split our expertise and support amongst multiple schemes. Operations will thank us. Of course, Qserv does not need to be done at the same time as everything else so the timelines can be decoupled. As far as I can tell we have to do a big bang (or at least "big merge") conversion because of pex_exceptions and having to pass afw objects into meas_algorithms calls (etc). Packages that do not accept objects from outside their package and which do not raise exceptions from other packages can presumably be ported independenctly.

            I did arrange for Pim Schellart [X] and Dave Mills to talk about pybind11 at the all hands meeting. Dave is using boost.python at the moment and I think was amenable to switching to pybind11 (it should be a trivial conversion).

            Show
            tjenness Tim Jenness added a comment - It seems to me that it is highly desirable for all of DM to converge on a single binding solution so we do not have to split our expertise and support amongst multiple schemes. Operations will thank us. Of course, Qserv does not need to be done at the same time as everything else so the timelines can be decoupled. As far as I can tell we have to do a big bang (or at least "big merge") conversion because of pex_exceptions and having to pass afw objects into meas_algorithms calls (etc). Packages that do not accept objects from outside their package and which do not raise exceptions from other packages can presumably be ported independenctly. I did arrange for Pim Schellart [X] and Dave Mills to talk about pybind11 at the all hands meeting. Dave is using boost.python at the moment and I think was amenable to switching to pybind11 (it should be a trivial conversion).
            Hide
            fritzm Fritz Mueller added a comment -

            Qserv's use of SWIG is pretty minimal – we have only two .i files in the source tree that wrap to Python that would need to be addressed.

            We do have a third .i file which is used to wrap an interface from C++ to LUA (sorry!) This is a very small, procedural interface. We could either carry SWIG forward as a dependency of qserv for this purpose, or consider just manually coding these LUA bindings.

            Show
            fritzm Fritz Mueller added a comment - Qserv's use of SWIG is pretty minimal – we have only two .i files in the source tree that wrap to Python that would need to be addressed. We do have a third .i file which is used to wrap an interface from C++ to LUA (sorry!) This is a very small, procedural interface. We could either carry SWIG forward as a dependency of qserv for this purpose, or consider just manually coding these LUA bindings.
            Hide
            dmills Dave Mills added a comment -

            I am amenable to the use of pybind11 to wrap the SAL generated interfaces. The OCS group
            has discussed it , as well as the desire to move to Python 3 as soon as is convenient. We intend
            to follow DM's lead as to the timeline in both cases.

            Show
            dmills Dave Mills added a comment - I am amenable to the use of pybind11 to wrap the SAL generated interfaces. The OCS group has discussed it , as well as the desire to move to Python 3 as soon as is convenient. We intend to follow DM's lead as to the timeline in both cases.
            Hide
            jbosch Jim Bosch added a comment -

            As absolutely anyone could have guessed, I'm strongly in favor. There will be work up-front (I'll leave it to Pim Schellart [X] to estimate how much), but if we spread that out well I think it might serve a secondary purpose of getting a lot of developers more familiar with parts of the stack they don't interact with much. Unfortunately, like the Python 3 conversion, I don't think it will be possible to do that fully in parallel, as we'll need to follow the dependency tree.

            Using pybind11-wrapped classes from within Swig is in fact possible (the reverse is not, at least not with out a lot of fragile reverse-engineering of Swig), but it's enough extra work to provide that for a class that we should only try it if we have a particularly thin in interface where we need to keep Swig on one side (and it sounds like we don't have that anywhere).

            Show
            jbosch Jim Bosch added a comment - As absolutely anyone could have guessed, I'm strongly in favor. There will be work up-front (I'll leave it to Pim Schellart [X] to estimate how much), but if we spread that out well I think it might serve a secondary purpose of getting a lot of developers more familiar with parts of the stack they don't interact with much. Unfortunately, like the Python 3 conversion, I don't think it will be possible to do that fully in parallel, as we'll need to follow the dependency tree. Using pybind11-wrapped classes from within Swig is in fact possible (the reverse is not, at least not with out a lot of fragile reverse-engineering of Swig), but it's enough extra work to provide that for a class that we should only try it if we have a particularly thin in interface where we need to keep Swig on one side (and it sounds like we don't have that anywhere).
            Hide
            tjenness Tim Jenness added a comment -

            I'm strongly in favor of a switch and I'm strongly in favor of us dropping SWIG everywhere. My only concern is the amount of work in tracking master if we take a year to do the migration. It's already the case that with pytest, python3 and pex_logging refactorings that the pybind11 branches are way out of date and this will get worse as we head up to packages that are getting more active development (we don't refactor the lower levels as often). We will need to bring the older packages up-to-date with master otherwise the higher packages won't build.

            Show
            tjenness Tim Jenness added a comment - I'm strongly in favor of a switch and I'm strongly in favor of us dropping SWIG everywhere. My only concern is the amount of work in tracking master if we take a year to do the migration. It's already the case that with pytest, python3 and pex_logging refactorings that the pybind11 branches are way out of date and this will get worse as we head up to packages that are getting more active development (we don't refactor the lower levels as often). We will need to bring the older packages up-to-date with master otherwise the higher packages won't build.
            Hide
            ktl Kian-Tat Lim added a comment -

            Someone said that I didn't explicitly weigh in on this; I will do so: +1.

            Show
            ktl Kian-Tat Lim added a comment - Someone said that I didn't explicitly weigh in on this; I will do so: +1.
            Hide
            tjenness Tim Jenness added a comment -

            3 hours to go then. Can we adopt this before John Swinbank and Simon Krughoff have worked out how to do the migration? Or would that be one of the triggering tickets?

            Show
            tjenness Tim Jenness added a comment - 3 hours to go then. Can we adopt this before John Swinbank and Simon Krughoff have worked out how to do the migration? Or would that be one of the triggering tickets?
            Hide
            swinbank John Swinbank added a comment -

            For the record, given Kian-Tat Lim's comment above, I've encouraged Pim Schellart [X] to simply adopt this RFC without any further approval being needed.

            As for implementation tickets, I propose to have this triggering two epics: DM-7330 and one yet-to-be-created in S17 (but with the expectation that we'll work on it before then). I hope that's sufficient; if you want to push back, this would be a good time.

            Show
            swinbank John Swinbank added a comment - For the record, given Kian-Tat Lim 's comment above, I've encouraged Pim Schellart [X] to simply adopt this RFC without any further approval being needed. As for implementation tickets, I propose to have this triggering two epics : DM-7330 and one yet-to-be-created in S17 (but with the expectation that we'll work on it before then). I hope that's sufficient; if you want to push back, this would be a good time.
            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.
            swinbank John Swinbank made changes -
            Link This issue is triggering DM-7717 [ DM-7717 ]
            swinbank John Swinbank made changes -
            Link This issue is triggering DM-7330 [ DM-7330 ]
            swinbank John Swinbank made changes -
            Resolution Done [ 10000 ]
            Status Proposed [ 10805 ] Adopted [ 10806 ]
            tjenness Tim Jenness made changes -
            Link This issue is triggering DM-8467 [ DM-8467 ]
            pschella Pim Schellart [X] (Inactive) made changes -
            Status Adopted [ 10806 ] Implemented [ 11105 ]

              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:

                  Jenkins Builds

                  No builds found.