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

Wrap afw::cameraGeom with pybind11

    XMLWordPrintable

    Details

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

      Description

      The generated wrappers will live parallel to the Swig wrappers. This ticket only covers the C++ wrappers themselves, not the Python layer on top (which will continue to use the old wrappers) all work will stay on a separate branch and will not be merged to master until DM-6168 is complete.

      The tests included in this ticket are:

      1. testDetector.py
      2. testMakePixelToTanPixel.py
      3. testExposure.py
      4. testCamGeomFitsUtils.py
      5. testCameraGeom.py
      6. testDistortedTanWcs.py
      7. testCameraTransformMap.py
      8. testColor.py
      9. testOrientation.py
      10. testCameraSys.py
      11. testWarper.py (moved to DM-8619)

        Attachments

          Issue Links

            Activity

            No builds found.
            fred3m Fred Moolekamp created issue -
            fred3m Fred Moolekamp made changes -
            Field Original Value New Value
            Link This issue blocks DM-7057 [ DM-7057 ]
            pschella Pim Schellart [X] (Inactive) made changes -
            Epic Link DM-6168 [ 24680 ]
            swinbank John Swinbank made changes -
            Epic Link DM-6168 [ 24680 ] DM-7717 [ 26925 ]
            pschella Pim Schellart [X] (Inactive) made changes -
            Description The generated wrappers will live parallel to the Swig wrappers. This ticket only covers the C++ wrappers themselves, not the Python layer on top (which will continue to use the old wrappers) all work will stay on a separate branch and will not be merged to master until DM-6168 is complete.

            The tests included in this ticket are:
            # testDetector.py
            # testMakePixelToTanPixel.py
            # exposure.py
            # testCamGeomFitsUtils.py
            # testCameraGeom.py
            # testDistortedTanWcs.py
            # testCameraTransformMap.py
            # color.py
            # testOrientation.py
            # testCameraSys.py
            The generated wrappers will live parallel to the Swig wrappers. This ticket only covers the C++ wrappers themselves, not the Python layer on top (which will continue to use the old wrappers) all work will stay on a separate branch and will not be merged to master until DM-6168 is complete.

            The tests included in this ticket are:
            # testDetector.py
            # testMakePixelToTanPixel.py
            # exposure.py
            # testCamGeomFitsUtils.py
            # testCameraGeom.py
            # testDistortedTanWcs.py
            # testCameraTransformMap.py
            # color.py
            # testOrientation.py
            # testCameraSys.py
            # testWarper.py
            rowen Russell Owen made changes -
            Assignee Russell Owen [ rowen ]
            rowen Russell Owen made changes -
            Link This issue relates to DM-6168 [ DM-6168 ]
            rowen Russell Owen made changes -
            Component/s afw [ 10714 ]
            Team Data Release Production [ 10301 ] Alert Production [ 10300 ]
            Labels SciencePipelines
            rowen Russell Owen made changes -
            Link This issue is blocked by DM-7797 [ DM-7797 ]
            krzys Krzysztof Findeisen made changes -
            Link This issue is blocked by DM-8447 [ DM-8447 ]
            rowen Russell Owen made changes -
            Epic Link DM-7717 [ 26925 ] DM-8450 [ 28066 ]
            krzys Krzysztof Findeisen made changes -
            Link This issue blocks DM-8423 [ DM-8423 ]
            Hide
            krzys Krzysztof Findeisen added a comment - - edited

            It appears that DM-8420, DM-8422, and DM-8423 all require FOCAL_PLANE to be defined.

            Show
            krzys Krzysztof Findeisen added a comment - - edited It appears that DM-8420 , DM-8422 , and DM-8423 all require FOCAL_PLANE to be defined.
            krzys Krzysztof Findeisen made changes -
            Link This issue blocks DM-8422 [ DM-8422 ]
            krzys Krzysztof Findeisen made changes -
            Link This issue blocks DM-8420 [ DM-8420 ]
            rowen Russell Owen made changes -
            Sprint Alert Production S17 - 12 [ 305 ]
            krzys Krzysztof Findeisen made changes -
            Link This issue blocks DM-8420 [ DM-8420 ]
            krzys Krzysztof Findeisen made changes -
            Link This issue blocks DM-8422 [ DM-8422 ]
            rowen Russell Owen made changes -
            Status To Do [ 10001 ] In Progress [ 3 ]
            rowen Russell Owen made changes -
            Description The generated wrappers will live parallel to the Swig wrappers. This ticket only covers the C++ wrappers themselves, not the Python layer on top (which will continue to use the old wrappers) all work will stay on a separate branch and will not be merged to master until DM-6168 is complete.

            The tests included in this ticket are:
            # testDetector.py
            # testMakePixelToTanPixel.py
            # exposure.py
            # testCamGeomFitsUtils.py
            # testCameraGeom.py
            # testDistortedTanWcs.py
            # testCameraTransformMap.py
            # color.py
            # testOrientation.py
            # testCameraSys.py
            # testWarper.py
            The generated wrappers will live parallel to the Swig wrappers. This ticket only covers the C++ wrappers themselves, not the Python layer on top (which will continue to use the old wrappers) all work will stay on a separate branch and will not be merged to master until DM-6168 is complete.

            The tests included in this ticket are:
            # testDetector.py
            # testMakePixelToTanPixel.py
            # testExposure.py
            # testCamGeomFitsUtils.py
            # testCameraGeom.py
            # testDistortedTanWcs.py
            # testCameraTransformMap.py
            # testColor.py
            # testOrientation.py
            # testCameraSys.py
            # testWarper.py
            rowen Russell Owen made changes -
            Link This issue relates to DM-8609 [ DM-8609 ]
            rowen Russell Owen made changes -
            Link This issue relates to DM-8619 [ DM-8619 ]
            rowen Russell Owen made changes -
            Story Points 5 8
            Hide
            rowen Russell Owen added a comment -

            I moved testWarper.py to a new ticket DM-8619 because I felt modifying afw.math was out of scope for cameraGeom work.

            Show
            rowen Russell Owen added a comment - I moved testWarper.py to a new ticket DM-8619 because I felt modifying afw.math was out of scope for cameraGeom work.
            rowen Russell Owen made changes -
            Reviewers Krzysztof Findeisen [ krzys ]
            Status In Progress [ 3 ] In Review [ 10004 ]
            rowen Russell Owen made changes -
            Description The generated wrappers will live parallel to the Swig wrappers. This ticket only covers the C++ wrappers themselves, not the Python layer on top (which will continue to use the old wrappers) all work will stay on a separate branch and will not be merged to master until DM-6168 is complete.

            The tests included in this ticket are:
            # testDetector.py
            # testMakePixelToTanPixel.py
            # testExposure.py
            # testCamGeomFitsUtils.py
            # testCameraGeom.py
            # testDistortedTanWcs.py
            # testCameraTransformMap.py
            # testColor.py
            # testOrientation.py
            # testCameraSys.py
            # testWarper.py
            The generated wrappers will live parallel to the Swig wrappers. This ticket only covers the C++ wrappers themselves, not the Python layer on top (which will continue to use the old wrappers) all work will stay on a separate branch and will not be merged to master until DM-6168 is complete.

            The tests included in this ticket are:
            # testDetector.py
            # testMakePixelToTanPixel.py
            # testExposure.py
            # testCamGeomFitsUtils.py
            # testCameraGeom.py
            # testDistortedTanWcs.py
            # testCameraTransformMap.py
            # testColor.py
            # testOrientation.py
            # testCameraSys.py
            # testWarper.py (moved to DM-8619)
            rowen Russell Owen made changes -
            Link This issue relates to DM-8626 [ DM-8626 ]
            Hide
            rowen Russell Owen added a comment -

            `testCameraGeom.py` fails in Python 2 because `CameraSys.getName` returns unicode, and that test uses such a value as a key in a `pex.config.ConfigDictField` and pex_config rejects that. See DM-8626.

            I may have a fix for that (which will require no changes to afw). Depending on how quickly I can get that fixed I will either disable `testCameraGeom.py` or get the fix merged to pex_config DM-8467 before merging this ticket and verify that the all tests pass.

            Show
            rowen Russell Owen added a comment - `testCameraGeom.py` fails in Python 2 because `CameraSys.getName` returns unicode, and that test uses such a value as a key in a `pex.config.ConfigDictField` and pex_config rejects that. See DM-8626 . I may have a fix for that (which will require no changes to afw). Depending on how quickly I can get that fixed I will either disable `testCameraGeom.py` or get the fix merged to pex_config DM-8467 before merging this ticket and verify that the all tests pass.
            Hide
            krzys Krzysztof Findeisen added a comment -

            I have a bunch of minor comments, but nothing show-stopping. Feel free to merge after resolving the comments.

            Show
            krzys Krzysztof Findeisen added a comment - I have a bunch of minor comments, but nothing show-stopping. Feel free to merge after resolving the comments.
            krzys Krzysztof Findeisen made changes -
            Status In Review [ 10004 ] Reviewed [ 10101 ]
            rowen Russell Owen made changes -
            Link This issue relates to DM-8647 [ DM-8647 ]
            rowen Russell Owen made changes -
            Link This issue relates to DM-8651 [ DM-8651 ]
            Hide
            rowen Russell Owen added a comment -

            Thank you for the very helpful review. Regarding symbols in lsst::afw::image::detail I am emulating SWIG for now but filed DM-8651 to improve it later. Regarding missing @brief I fixed the doxygen config (on DM-8647) so it's no longer needed. I believe I adopted all your other suggested changes (except a few that were questions "is this really needed?" to which the answer was "yes").

            Show
            rowen Russell Owen added a comment - Thank you for the very helpful review. Regarding symbols in lsst::afw::image::detail I am emulating SWIG for now but filed DM-8651 to improve it later. Regarding missing @brief I fixed the doxygen config (on DM-8647 ) so it's no longer needed. I believe I adopted all your other suggested changes (except a few that were questions "is this really needed?" to which the answer was "yes").
            rowen Russell Owen made changes -
            Status Reviewed [ 10101 ] Reviewed [ 10101 ]
            rowen Russell Owen made changes -
            Resolution Done [ 10000 ]
            Status Reviewed [ 10101 ] Done [ 10002 ]

              People

              Assignee:
              rowen Russell Owen
              Reporter:
              fred3m Fred Moolekamp
              Reviewers:
              Krzysztof Findeisen
              Watchers:
              Fred Moolekamp, Krzysztof Findeisen, Russell Owen
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  CI Builds

                  No builds found.