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

Wrap utils with pybind11

    XMLWordPrintable

    Details

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

      Description

      Split off from DM-6302.

        Attachments

          Activity

          No builds found.
          pschella Pim Schellart [X] (Inactive) created issue -
          pschella Pim Schellart [X] (Inactive) made changes -
          Field Original Value New Value
          Epic Link DM-6168 [ 24680 ]
          swinbank John Swinbank made changes -
          Sprint DRP F16-1 [ 225 ] DRP F16-2 [ 231 ]
          Hide
          pschella Pim Schellart [X] (Inactive) added a comment -

          Hmm, not sure what the right approach is here.
          There seems to be a lot of very Swig specific stuff in this package (e.g typemaps for numpy types and Swig macros for operators and function return types, as well as some workarounds for Swig problems).
          Some of it looks like we might need a pybind11 equivalent for, but we probably don't need everything and we won't quite know what we need until we encounter it.
          I see three options.

          1. Remove all Swig specific stuff and add things back in (either here or elsewhere) as needed when wrapping further packages.
          2. Create as much as possible equivalent functionality now, at the risk of not actually needing it (or needing a different version of it) later.
          3. Postpone work on this ticket until we know what we need.

          Either way some of the tests also need to be removed (since it makes no sense to test the Swig specific things). So it will be hard to check if the conversion is complete.

          Show
          pschella Pim Schellart [X] (Inactive) added a comment - Hmm, not sure what the right approach is here. There seems to be a lot of very Swig specific stuff in this package (e.g typemaps for numpy types and Swig macros for operators and function return types, as well as some workarounds for Swig problems). Some of it looks like we might need a pybind11 equivalent for, but we probably don't need everything and we won't quite know what we need until we encounter it. I see three options. 1. Remove all Swig specific stuff and add things back in (either here or elsewhere) as needed when wrapping further packages. 2. Create as much as possible equivalent functionality now, at the risk of not actually needing it (or needing a different version of it) later. 3. Postpone work on this ticket until we know what we need. Either way some of the tests also need to be removed (since it makes no sense to test the Swig specific things). So it will be hard to check if the conversion is complete.
          pschella Pim Schellart [X] (Inactive) made changes -
          Status To Do [ 10001 ] In Progress [ 3 ]
          swinbank John Swinbank made changes -
          Sprint DRP F16-2 [ 231 ] TCAMS 2016_03, DRP F16-2 [ 209, 231 ]
          swinbank John Swinbank made changes -
          Rank Ranked higher
          swinbank John Swinbank made changes -
          Sprint TCAMS 2016_03, DRP F16-2 [ 209, 231 ] DRP F16-2 [ 231 ]
          swinbank John Swinbank made changes -
          Sprint DRP F16-2 [ 231 ] DRP F16-2, DRP F16-3 [ 231, 237 ]
          swinbank John Swinbank made changes -
          Rank Ranked lower
          Hide
          pschella Pim Schellart [X] (Inactive) added a comment -

          Note that this will be merged to the epic branch instead of master.

          Show
          pschella Pim Schellart [X] (Inactive) added a comment - Note that this will be merged to the epic branch instead of master.
          pschella Pim Schellart [X] (Inactive) made changes -
          Reviewers Tim Jenness [ tjenness ]
          Status In Progress [ 3 ] In Review [ 10004 ]
          Hide
          tjenness Tim Jenness added a comment -

          Looks great. I love the simplicity of the pybind11 wrapper files. Minor comments on the PR, probably down to me being late to this topic and being confused.

          Show
          tjenness Tim Jenness added a comment - Looks great. I love the simplicity of the pybind11 wrapper files. Minor comments on the PR, probably down to me being late to this topic and being confused.
          tjenness Tim Jenness made changes -
          Status In Review [ 10004 ] Reviewed [ 10101 ]
          Hide
          pschella Pim Schellart [X] (Inactive) added a comment -

          Merged into epic branch after incorporating requested changes.

          Show
          pschella Pim Schellart [X] (Inactive) added a comment - Merged into epic branch after incorporating requested changes.
          pschella Pim Schellart [X] (Inactive) made changes -
          Resolution Done [ 10000 ]
          Status Reviewed [ 10101 ] Done [ 10002 ]
          tjenness Tim Jenness made changes -
          Component/s afw [ 10714 ]
          tjenness Tim Jenness made changes -
          Component/s utils [ 10723 ]
          Component/s afw [ 10714 ]

            People

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

              Dates

              Created:
              Updated:
              Resolved:

                Jenkins

                No builds found.