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

Tweaks to OO display interface

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: afw
    • Labels:
      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 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.

        Attachments

          Issue Links

            Activity

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

            Show
            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(-)
            Hide
            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??

            Show
            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??
            Hide
            rhl Robert Lupton added a comment -

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

            Show
            rhl Robert Lupton added a comment - You're right; sorry. I've pushed a patch
            Hide
            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.

            Show
            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.
            Hide
            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).

            Show
            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

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

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel