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

Document that stl_bind.h pybind11 wrapping is not recommended

    XMLWordPrintable

    Details

    • Type: Improvement
    • Status: To Do
    • Resolution: Unresolved
    • Fix Version/s: None
    • Component/s: None
    • Story Points:
      1

      Description

      While wrapping some some jointcal C++ to write python tests for it, I discovered pybind11's stl_bind.h opaque wrappers. This did the job for me, but it was not recommended by our pybind11 gurus, for various reasons. We don't mention it in the stl section of the dev guide, although we do have two related mentions of wrapping stl types in the pybind11 style guide:

      https://developer.lsst.io/coding/pybind11_style_guide.html#default-automatic-conversions-shall-be-used-for-all-stl-containers
      https://developer.lsst.io/coding/pybind11_style_guide.html#where-copying-of-stl-containers-is-undesirable-an-ndarray-type-should-be-used-instead

      Please add a note to the dev guide stl section about why not to use the stl_bind-style opaque wrappers, since that's the obvious place to look when wondering about wrapping stl objects. It might be worth adding an example that returns the object via a C++ lambda (if we don't have such an example already), e.g.:

          cls.def("getMappingIndices", [](PhotometryMappingBase const &self) {
              std::vector<unsigned> indices(0);
              self.getMappingIndices(indices);
              return indices;
          });
      

        Attachments

          Issue Links

            Activity

            Hide
            rowen Russell Owen added a comment -

            I don't know enough about this issue to feel comfortable documenting it.

            Show
            rowen Russell Owen added a comment - I don't know enough about this issue to feel comfortable documenting it.
            Hide
            jbosch Jim Bosch added a comment -

            The upstream docs on this may have improved since we wrote our dev guide section; we may want to consider delegating to them more and removing some of our own content:

            https://pybind11.readthedocs.io/en/stable/advanced/cast/stl.html#binding-stl-containers

            On the other hand, I'm not sure all of the functionality described there is available yet in the version of pybind11 we are currently using.

            Show
            jbosch Jim Bosch added a comment - The upstream docs on this may have improved since we wrote our dev guide section; we may want to consider delegating to them more and removing some of our own content: https://pybind11.readthedocs.io/en/stable/advanced/cast/stl.html#binding-stl-containers On the other hand, I'm not sure all of the functionality described there is available yet in the version of pybind11 we are currently using.
            Hide
            jbosch Jim Bosch added a comment -

            Still seems valid.  Last comment applies; upstream docs do cover this, though they don't draw attention to it, and perhaps we should.

            Show
            jbosch Jim Bosch added a comment - Still seems valid.  Last comment applies; upstream docs do cover this, though they don't draw attention to it, and perhaps we should.

              People

              Assignee:
              Unassigned Unassigned
              Reporter:
              Parejkoj John Parejko
              Watchers:
              Jim Bosch, John Parejko, John Swinbank, Kian-Tat Lim
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

                Dates

                Created:
                Updated:

                  Jenkins

                  No builds found.