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

Tweaks to OO display interface

    XMLWordPrintable

Details

    • Story
    • Status: Done
    • Resolution: Done
    • None
    • afw
    • None

    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 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.

      Attachments

        Issue Links

          Activity

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

            rhl 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(-)

            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??

            lauren 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??

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

            rhl Robert Lupton added a comment - You're right; sorry. I've pushed a patch

            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.

            lauren 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.

            I ran buildbot and merged to master.

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

            rhl 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

              rhl Robert Lupton
              rhl Robert Lupton
              Lauren MacArthur
              Lauren MacArthur, Robert Lupton
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Jenkins

                  No builds found.