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

Rewrite ndarray pybind11 type_casters to avoid numpy C API

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None

      Description

      Using the NumPy C API requires some ugly boilerplate in every wrapper file that uses the ndarray type_casters. pybind11 provides its own low-level numpy interface, and it'd be better to use that instead. I imagine this will uncover some functionality we'd want to add to upstream pybind11, but I imagine we'd at least have the option of doing that locally if that isn't a possibility.

        Attachments

          Issue Links

            Activity

            Hide
            jbosch Jim Bosch added a comment -

            Review of the main code is ongoing on the upstream repo:

            https://github.com/ndarray/ndarray/pull/67

            After that we'll still need to roll out a new ndarray release and adopt it LSST. These changes also bring some changes to how cmake finds pybind11, so it's possible we'll have to update the eupspkg scripts as well.

            These changes will not require us to remove the NumPy includes and _import_array calls in the stack immediately, but after adopting this we should be able to do that package-by-package.

            Show
            jbosch Jim Bosch added a comment - Review of the main code is ongoing on the upstream repo: https://github.com/ndarray/ndarray/pull/67 After that we'll still need to roll out a new ndarray release and adopt it LSST. These changes also bring some changes to how cmake finds pybind11, so it's possible we'll have to update the eupspkg scripts as well. These changes will not require us to remove the NumPy includes and _import_array calls in the stack immediately, but after adopting this we should be able to do that package-by-package.
            Hide
            pschella Pim Schellart [X] (Inactive) added a comment -

            Some minor things on PR, but overall looks good. Just a question, did you test it with our current version of pybind11? And also please note that we currently don't actually install the pybind11 package, so the cmake module probably doesn't exist. Something that should be changed on a separate ticket perhaps?

            Show
            pschella Pim Schellart [X] (Inactive) added a comment - Some minor things on PR, but overall looks good. Just a question, did you test it with our current version of pybind11? And also please note that we currently don't actually install the pybind11 package, so the cmake module probably doesn't exist. Something that should be changed on a separate ticket perhaps?
            Hide
            jbosch Jim Bosch added a comment -

            Upstream work is done; using the new upstream release is DM-13534.

            Show
            jbosch Jim Bosch added a comment - Upstream work is done; using the new upstream release is DM-13534 .

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel