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

Upgrade pybind11 to version 2.x

    XMLWordPrintable

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None
    • Story Points:
      2
    • Sprint:
      DRP S17-2, DRP S17-3, DRP S17-4
    • Team:
      Data Release Production

      Description

      Pybind11 version 2, with it's latest bugfix update (2.0.1), is out.
      This is a big release with some API incompatibilities (most of which we already depend on). But we have been using pybind11 master so far so we won't have to change much.
      Nevertheless this is slightly more work than normal (normally about 0.1 SP) because py::metaclass() is now a required attribute for all classes that use .def_static which requires a lot of (trivial) changes. And of course there may be other hidden problems.

      Now that a major fraction of the porting work is done, and we have the upstream features we need merged into the release version, the intention is to stick to fixed releases from now on if possible (instead of tracking master).

        Attachments

          Issue Links

            Activity

            pschella Pim Schellart [X] (Inactive) created issue -
            pschella Pim Schellart [X] (Inactive) made changes -
            Field Original Value New Value
            Epic Link DM-7717 [ 26925 ]
            pschella Pim Schellart [X] (Inactive) made changes -
            Status To Do [ 10001 ] In Progress [ 3 ]
            swinbank John Swinbank made changes -
            Team Data Release Production [ 10301 ]
            pschella Pim Schellart [X] (Inactive) made changes -
            Link This issue blocks DM-8580 [ DM-8580 ]
            Hide
            pschella Pim Schellart [X] (Inactive) added a comment -

            Encountered a problem. When a derived class MroDerived needs to expose a static attribute (which necessitates using py::metaclass), but also inherits from multiple bases Python raises the following error:

            ImportError: MroDerived: PyType_Ready failed (TypeError: mro() returned base with unsuitable layout ('pybind11_tests.MroBaseB'))!

            This has been bugreported upstream as https://github.com/pybind/pybind11/issues/597 and further work on this ticket is blocked until the issue is resolved.

            Show
            pschella Pim Schellart [X] (Inactive) added a comment - Encountered a problem. When a derived class MroDerived needs to expose a static attribute (which necessitates using py::metaclass ), but also inherits from multiple bases Python raises the following error: ImportError: MroDerived: PyType_Ready failed (TypeError: mro() returned base with unsuitable layout ('pybind11_tests.MroBaseB'))! This has been bugreported upstream as https://github.com/pybind/pybind11/issues/597 and further work on this ticket is blocked until the issue is resolved.
            swinbank John Swinbank made changes -
            Sprint DRP S17-2 [ 356 ] DRP S17-2, DRP S17-3 [ 356, 360 ]
            swinbank John Swinbank made changes -
            Rank Ranked higher
            pschella Pim Schellart [X] (Inactive) made changes -
            Epic Link DM-7717 [ 26925 ] DM-9155 [ 29718 ]
            pschella Pim Schellart [X] (Inactive) made changes -
            Sprint DRP S17-2, DRP S17-3 [ 356, 360 ] DRP S17-2 [ 356 ]
            pschella Pim Schellart [X] (Inactive) made changes -
            Epic Link DM-9155 [ 29718 ] DM-7717 [ 26925 ]
            pschella Pim Schellart [X] (Inactive) made changes -
            Sprint DRP S17-2 [ 356 ] DRP S17-2, DRP S17-3 [ 356, 360 ]
            swinbank John Swinbank made changes -
            Sprint DRP S17-2, DRP S17-3 [ 356, 360 ] DRP S17-2, DRP S17-3, DRP S17-4 [ 356, 360, 363 ]
            swinbank John Swinbank made changes -
            Rank Ranked higher
            pschella Pim Schellart [X] (Inactive) made changes -
            Summary Upgrade pybind11 to version 2.0.x Upgrade pybind11 to version 2.x
            Hide
            pschella Pim Schellart [X] (Inactive) added a comment -

            Likely to be version 2.1.0

            Show
            pschella Pim Schellart [X] (Inactive) added a comment - Likely to be version 2.1.0
            Hide
            pschella Pim Schellart [X] (Inactive) added a comment -

            Unclear if we are going to get a tagged upstream release before our merge. But it will likely be close to this version (latest upstream master).
            Changes should work against our current version (some random earlier master version) too though.

            Show
            pschella Pim Schellart [X] (Inactive) added a comment - Unclear if we are going to get a tagged upstream release before our merge. But it will likely be close to this version (latest upstream master). Changes should work against our current version (some random earlier master version) too though.
            Hide
            pschella Pim Schellart [X] (Inactive) added a comment -

            The only other change is a required pybind11 to python namespace, to avoid a clash with upstream.
            This was scheduled as cleanup already and is (partly) applied here.

            Show
            pschella Pim Schellart [X] (Inactive) added a comment - The only other change is a required pybind11 to python namespace, to avoid a clash with upstream. This was scheduled as cleanup already and is (partly) applied here.
            pschella Pim Schellart [X] (Inactive) made changes -
            Reviewers Jim Bosch [ jbosch ]
            Status In Progress [ 3 ] In Review [ 10004 ]
            Hide
            jbosch Jim Bosch added a comment -

            Looks good. Only comment is that I think the afw::image constructors that take FIts objects can all be deleted (I noticed you've moved one around here), since Fits itself isn't available from Python. But I think I've already done that on DM-9182 anyway and I'll just make sure it isn't lost when I rebase that.

            Show
            jbosch Jim Bosch added a comment - Looks good. Only comment is that I think the afw::image constructors that take FIts objects can all be deleted (I noticed you've moved one around here), since Fits itself isn't available from Python. But I think I've already done that on DM-9182 anyway and I'll just make sure it isn't lost when I rebase that.
            jbosch Jim Bosch made changes -
            Status In Review [ 10004 ] Reviewed [ 10101 ]
            Hide
            pschella Pim Schellart [X] (Inactive) added a comment -

            Merged against presumed 2.1.0 version. Will open new ticket once final release is tagged upstream.

            Show
            pschella Pim Schellart [X] (Inactive) added a comment - Merged against presumed 2.1.0 version. Will open new ticket once final release is tagged upstream.
            pschella Pim Schellart [X] (Inactive) made changes -
            Resolution Done [ 10000 ]
            Status Reviewed [ 10101 ] Done [ 10002 ]

              People

              Assignee:
              pschella Pim Schellart [X] (Inactive)
              Reporter:
              pschella Pim Schellart [X] (Inactive)
              Reviewers:
              Jim Bosch
              Watchers:
              Jim Bosch, Pim Schellart [X] (Inactive)
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved: