# Wrap afw::cameraGeom with pybind11

XMLWordPrintable

#### Details

• Type: Story
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
• Story Points:
8
• Sprint:
• Team:

#### 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)

#### Activity

No builds found.
Fred Moolekamp created issue -
Field Original Value New Value
Link This issue blocks DM-7057 [ DM-7057 ]
Pim Schellart [X] (Inactive) made changes -
 Epic Link DM-6168 [ 24680 ]
 Epic Link DM-6168 [ 24680 ] DM-7717 [ 26925 ]
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
 Assignee Russell Owen [ rowen ]
 Link This issue relates to DM-6168 [ DM-6168 ]
 Component/s afw [ 10714 ] Team Data Release Production [ 10301 ] Alert Production [ 10300 ] Labels SciencePipelines
 Link This issue is blocked by DM-7797 [ DM-7797 ]
 Link This issue is blocked by DM-8447 [ DM-8447 ]
 Epic Link DM-7717 [ 26925 ] DM-8450 [ 28066 ]
 Link This issue blocks DM-8423 [ DM-8423 ]
Hide
Krzysztof Findeisen added a comment - - edited

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

Show
Krzysztof Findeisen added a comment - - edited It appears that DM-8420 , DM-8422 , and DM-8423 all require FOCAL_PLANE to be defined.
 Link This issue blocks DM-8422 [ DM-8422 ]
 Link This issue blocks DM-8420 [ DM-8420 ]
 Sprint Alert Production S17 - 12 [ 305 ]
 Link This issue blocks DM-8420 [ DM-8420 ]
 Link This issue blocks DM-8422 [ DM-8422 ]
 Status To Do [ 10001 ] In Progress [ 3 ]
 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
 Link This issue relates to DM-8609 [ DM-8609 ]
 Link This issue relates to DM-8619 [ DM-8619 ]
 Story Points 5 8
Hide
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
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.
 Reviewers Krzysztof Findeisen [ krzys ] Status In Progress [ 3 ] In Review [ 10004 ]
 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)
 Link This issue relates to DM-8626 [ DM-8626 ]
Hide
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
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
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
Krzysztof Findeisen added a comment - I have a bunch of minor comments, but nothing show-stopping. Feel free to merge after resolving the comments.
 Status In Review [ 10004 ] Reviewed [ 10101 ]
 Link This issue relates to DM-8647 [ DM-8647 ]
 Link This issue relates to DM-8651 [ DM-8651 ]
Hide
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
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").
 Status Reviewed [ 10101 ] Reviewed [ 10101 ]
 Resolution Done [ 10000 ] Status Reviewed [ 10101 ] Done [ 10002 ]

#### People

Assignee:
Russell Owen
Reporter:
Fred Moolekamp
Reviewers:
Krzysztof Findeisen
Watchers:
Fred Moolekamp, Krzysztof Findeisen, Russell Owen