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

Wrap afw::table with pybind11

    XMLWordPrintable

Details

    • Story
    • Status: Done
    • Resolution: Done
    • None
    • afw
    • 32
    • DRP F16-6
    • Data Release Production

    Description

      Following the same pattern as DM-6926, DM-6297, etc.

      The tests included in this ticket are:

      1. testSchema.py
      2. testSimpleTable.py
      3. testFunctorKeys.py

      Attachments

        Issue Links

          Activity

            tjenness Tim Jenness added a comment -

            I think pschella hits the nail on the head here in that cython and Swig merge user code with automated code so linting becomes irrelevant in the output. In our case we are importing the whole C++ interface into python, augmenting it with a bunch of new python code and then forwarding the whole thing on to the _init_.py. I'm not entirely sure how we can square that circle and have proper code quality checks in the code whilst also not having to specify every C++ interface explicitly in the code. Sorry that I'm not really helping.

            tjenness Tim Jenness added a comment - I think pschella hits the nail on the head here in that cython and Swig merge user code with automated code so linting becomes irrelevant in the output. In our case we are importing the whole C++ interface into python, augmenting it with a bunch of new python code and then forwarding the whole thing on to the _ init _.py . I'm not entirely sure how we can square that circle and have proper code quality checks in the code whilst also not having to specify every C++ interface explicitly in the code. Sorry that I'm not really helping.

            There were a few upstream bugs that had to be handled but now the Jenkins builds pass and once approved this branch is ready to merge.

            fred3m Fred Moolekamp added a comment - There were a few upstream bugs that had to be handled but now the Jenkins builds pass and once approved this branch is ready to merge.

            This looks very good. I just had a few minor requests, such as adding code comments to explain why some code is commented out, so users can better gauge when and why it can be deleted or uncommented.

            rowen Russell Owen added a comment - This looks very good. I just had a few minor requests, such as adding code comments to explain why some code is commented out, so users can better gauge when and why it can be deleted or uncommented.

            Made the comments and fixes suggested by Russel. Py 2&3 tests passed on Jenkins (https://ci.lsst.codes/job/stack-os-matrix/18320/, https://ci.lsst.codes/job/stack-os-matrix/18319/). Merging to DM-6168

            fred3m Fred Moolekamp added a comment - Made the comments and fixes suggested by Russel. Py 2&3 tests passed on Jenkins ( https://ci.lsst.codes/job/stack-os-matrix/18320/ , https://ci.lsst.codes/job/stack-os-matrix/18319/ ). Merging to DM-6168

            Just to confirm we will now follow the approach used on this ticket to forward the API. Using from package import * only in `packageLib.py` files. We will reevaluate this approach after afw is fully wrapped and RFC it along with the rest of the DMTN-024 pybind11 coding guidelines.

            pschella Pim Schellart [X] (Inactive) added a comment - - edited Just to confirm we will now follow the approach used on this ticket to forward the API. Using from package import * only in `packageLib.py` files. We will reevaluate this approach after afw is fully wrapped and RFC it along with the rest of the DMTN-024 pybind11 coding guidelines.

            People

              fred3m Fred Moolekamp
              swinbank John Swinbank
              Russell Owen
              Fred Moolekamp, Jim Bosch, John Swinbank, Kian-Tat Lim, Krzysztof Findeisen, Pim Schellart [X] (Inactive), Russell Owen, Tim Jenness
              Votes:
              0 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Jenkins

                  No builds found.