# Wrap display_ds9 with pybind11

XMLWordPrintable

#### Details

• Type: Story
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
• Story Points:
3
• Epic Link:
• Sprint:
Alert Production S17 - 2
• Team:
Alert Production

#### Description

May not have any work associated with it, but is an lsst_distrib dependency. Investigate and update SP.

#### Activity

Hide
Russell Owen added a comment -

It has SWIGged code in it, so definitely needs work. Unfortunately it also has no tests.

Show
Russell Owen added a comment - It has SWIGged code in it, so definitely needs work. Unfortunately it also has no tests.
Hide
Russell Owen added a comment - - edited

The changes are:

display_ds9:

• wrap a custom C++ interface to xpa. I defined this in the same file as the wrapper because that is how the SWIG did it, there was no header file and it uses globals. I felt safer duplicating what SWIG did. Even so I had to tweak the code a bit because SWIG has the ability to raise a useful error if a function returns NULL and pybind11 does not, as far as I know). So instead of returning NULL I raise an exception.
• ds9.py function _i_mtv has special handling for uint16 data and the check for that required a small update to work with pybind11.

afw:

• I wrapped the writeFitsImage function. I believe this completes all wrapping of afw/display.

I tested it by running the following snippet:

 import os.path   from lsst.utils import getPackageDir import lsst.afw.display as afwDisplay import lsst.afw.image as afwImage import lsst.afw.geom as afwGeom import lsst.display.ds9   afwDataDir = getPackageDir("afwdata") assert afwDataDir   path = os.path.join(afwDataDir, "ImSim/calexp/v85408556-fr/R23/S11.fits") subregion = afwGeom.Box2I(afwGeom.Point2I(0, 0), afwGeom.Extent2I(500, 500)) exp = afwImage.ExposureF(path, subregion) mi = exp.getMaskedImage() image = mi.getImage() mask = mi.getMask()   disp = afwDisplay.getDisplay(1) disp.mtv(exp, title="foo") 

and also modifying the snippet to only show an image or a mask.

Show
Russell Owen added a comment - - edited The changes are: display_ds9: wrap a custom C++ interface to xpa . I defined this in the same file as the wrapper because that is how the SWIG did it, there was no header file and it uses globals. I felt safer duplicating what SWIG did. Even so I had to tweak the code a bit because SWIG has the ability to raise a useful error if a function returns NULL and pybind11 does not, as far as I know). So instead of returning NULL I raise an exception. ds9.py function _i_mtv has special handling for uint16 data and the check for that required a small update to work with pybind11. afw: I wrapped the writeFitsImage function. I believe this completes all wrapping of afw/display . I tested it by running the following snippet: import os.path   from lsst.utils import getPackageDir import lsst.afw.display as afwDisplay import lsst.afw.image as afwImage import lsst.afw.geom as afwGeom import lsst.display.ds9   afwDataDir = getPackageDir("afwdata") assert afwDataDir   path = os.path.join(afwDataDir, "ImSim/calexp/v85408556-fr/R23/S11.fits") subregion = afwGeom.Box2I(afwGeom.Point2I(0, 0), afwGeom.Extent2I(500, 500)) exp = afwImage.ExposureF(path, subregion) mi = exp.getMaskedImage() image = mi.getImage() mask = mi.getMask()   disp = afwDisplay.getDisplay(1) disp.mtv(exp, title="foo") and also modifying the snippet to only show an image or a mask.
Hide
Krzysztof Findeisen added a comment -

Sigh I can see why you were afraid to touch the Swig code. Do you know if there is a technical debt ticket for cleaning up this package?

Aside from the contents of _xpa.cc/xpa.i, my only requests are to make the module structure a bit more pybind11-standard and to clean up the commit history.

Show
Krzysztof Findeisen added a comment - Sigh I can see why you were afraid to touch the Swig code. Do you know if there is a technical debt ticket for cleaning up this package? Aside from the contents of _xpa.cc / xpa.i , my only requests are to make the module structure a bit more pybind11-standard and to clean up the commit history.
Hide
Russell Owen added a comment - - edited

Thank you for the helpful (and quick!) review. I fixed the issues you caught, including rebasing to correct the contents of one commit and fixing .gitignore.

I filed DM-9273 for the technical debt in display_ds9's xpa wrapper. I also added xpa.py which imports lsst.pex.exceptions and the symbols defined in _xpa.cc. I chose not to add a displayLib.py because in this case only the xpa interface is wrapped and failure to import that suggests that 3rd party package xpa is missing or broken. So in this case I feel it is preferable for that one module to be self-contained.

Show
Russell Owen added a comment - - edited Thank you for the helpful (and quick!) review. I fixed the issues you caught, including rebasing to correct the contents of one commit and fixing .gitignore . I filed DM-9273 for the technical debt in display_ds9 's xpa wrapper. I also added xpa.py which imports lsst.pex.exceptions and the symbols defined in _xpa.cc . I chose not to add a displayLib.py because in this case only the xpa interface is wrapped and failure to import that suggests that 3rd party package xpa is missing or broken. So in this case I feel it is preferable for that one module to be self-contained.

#### People

Assignee:
Russell Owen
Reporter:
Pim Schellart [X] (Inactive)
Reviewers:
Krzysztof Findeisen
Watchers:
Krzysztof Findeisen, Pim Schellart [X] (Inactive), Russell Owen
Votes:
0 Vote for this issue
Watchers:
3 Start watching this issue

#### Dates

Created:
Updated:
Resolved:

#### CI Builds

No builds found.