# Tweaks to OO display interface

XMLWordPrintable

#### Details

• Type: Story
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
None
• Story Points:
2
• Sprint:
Science Pipelines DM-S15-4
• Team:
Data Release Production

#### Description

When I wrote the initial version of display_firefly I found a few minor issues in the way I'd designed the Display class; at the same time, Lauren MacArthur found some missing functions in the backward-compatibility support for ds9.

Please fix these; note that this implies changes to afw, display_ds9, and display_firefly.

#### Activity

Hide
Robert Lupton added a comment -

afw and display_ds9 are pushed to tickets/DM-2849; display_firefly is pushed to master as it's never been released (and has no tests to fail) – but that was a mistake.

 $git diff --stat master  python/lsst/display/ds9/ds9.py | 92 ++++++++++++++++----------------------------------------------------------------------------  1 file changed, 16 insertions(+), 76 deletions(-)   $ git diff --stat master  python/lsst/afw/display/ds9.py | 23 +++++++++++++----  python/lsst/afw/display/ds9Regions.py | 123 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++  python/lsst/afw/display/interface.py | 49 +++++++++++++++--------------------  python/lsst/afw/display/virtualDevice.py | 31 +++++++++++-----------  tests/display.py | 10 +++++---  5 files changed, 184 insertions(+), 52 deletions(-) 

Show
Robert Lupton added a comment - afw and display_ds9 are pushed to tickets/ DM-2849 ; display_firefly is pushed to master as it's never been released (and has no tests to fail) – but that was a mistake. $git diff --stat master python/lsst/display/ds9/ds9.py | 92 ++++++++++++++++---------------------------------------------------------------------------- 1 file changed, 16 insertions(+), 76 deletions(-)$ git diff --stat master python/lsst/afw/display/ds9.py | 23 +++++++++++++---- python/lsst/afw/display/ds9Regions.py | 123 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ python/lsst/afw/display/interface.py | 49 +++++++++++++++-------------------- python/lsst/afw/display/virtualDevice.py | 31 +++++++++++----------- tests/display.py | 10 +++++--- 5 files changed, 184 insertions(+), 52 deletions(-)
Hide
Lauren MacArthur added a comment -

I've started to have a look at this. I've not yet gone through in detail (got side-tracked by a request for Jim), but I thought there was some logic to fix in the _getDisplayFromDisplayOrFrame() function in utils.py (i.e. could end up with display of None if neither display nor frame is passed into other functions such as drawBBox(), which does happen in existing code).? Is this no longer the case??

Show
Lauren MacArthur added a comment - I've started to have a look at this. I've not yet gone through in detail (got side-tracked by a request for Jim), but I thought there was some logic to fix in the _getDisplayFromDisplayOrFrame() function in utils.py (i.e. could end up with display of None if neither display nor frame is passed into other functions such as drawBBox() , which does happen in existing code).? Is this no longer the case??
Hide
Robert Lupton added a comment -

You're right; sorry. I've pushed a patch

Show
Robert Lupton added a comment - You're right; sorry. I've pushed a patch
Hide
Lauren MacArthur added a comment -

This all looks pretty good. I've put a few comments in the commits in afw (mostly requests for commit squashing).

I get one test failure as I think you now need to remove the references to scaleLimits and scaleType in testDs9.py (and in your notebook imageDisplay.ipynb).

Running with --debug on a {processCcd.py}} run was successful except for a call to ds9.ds9Cmd in pcaPsfDeterminer.py in meas_algorithms. I this a backwards compatibility issue? If so, and it can be fixed, great. If not, this also appears in analysis: deblender.py and utils.py and obs_subaru: rings.py, so would need fixing there.

Otherwise, if you get a Buildbot to pass, I'm happy for you to merge.

Show
Lauren MacArthur added a comment - This all looks pretty good. I've put a few comments in the commits in afw (mostly requests for commit squashing). I get one test failure as I think you now need to remove the references to scaleLimits and scaleType in testDs9.py (and in your notebook imageDisplay.ipynb ). Running with --debug on a {processCcd.py}} run was successful except for a call to ds9.ds9Cmd in pcaPsfDeterminer.py in meas_algorithms . I this a backwards compatibility issue? If so, and it can be fixed, great. If not, this also appears in analysis: deblender.py and utils.py and obs_subaru: rings.py , so would need fixing there. Otherwise, if you get a Buildbot to pass, I'm happy for you to merge.
Hide
Robert Lupton added a comment -

I ran buildbot and merged to master.

Packages affected ended up being afw, meas_algorithms, and display_ds9 (where I addressed TimJ's comments).

Show
Robert Lupton added a comment - I ran buildbot and merged to master. Packages affected ended up being afw, meas_algorithms, and display_ds9 (where I addressed TimJ's comments).

#### People

Assignee:
Robert Lupton
Reporter:
Robert Lupton
Reviewers:
Lauren MacArthur
Watchers:
Lauren MacArthur, Robert Lupton