# Refactor the image display code to remove explicit references to ds9

XMLWordPrintable

#### Details

• Type: RFC
• Status: Implemented
• Resolution: Done
• Component/s:
• Labels:
None
• Location:
PR

#### Description

The current stack image display code assumes that you're talking to ds9:

 import lsst.afw.display.ds9 as ds9   ds9.erase()

As the IPAC team thinks about supporting firefly, and ginga gains traction in the scipy world, we need replace this interface with something more general.

In the longer run we need to decide what APIs we need to support, but this is hard to do until we have more than one backend. This RFC proposes that we change the interface to something like:

 import lsst.afw.display.imdisplay as imdisplay imdisplay.setBackend('ds9')   imdisplay.erase()

It would be possible to hoist these methods to lsst.afw.display itself:

 import lsst.afw.display as afwDisplay afwDisplay.setBackend('ds9')   afwDisplay.erase()

A discussion of the pros and cons is in scope for the RFC.

The choice of backend (in this case ds9) should be configurable either explicitly, as illustrated here, or via some deus ex machina technique (a dot file, an environment variable, ...). I put the backend choice explicitly in the examples, but I would not expect this to appear in utility code.

#### Activity

No builds found.
Robert Lupton created issue -
Hide
Russell Owen added a comment -

I agree this should be changed, but I personally would be inclined to wait until we have another backend, so we have a better idea what a system that supports multiple backends needs to support.

Show
Russell Owen added a comment - I agree this should be changed, but I personally would be inclined to wait until we have another backend, so we have a better idea what a system that supports multiple backends needs to support.
Hide
Robert Lupton added a comment -

(I'd prefer .setBackend() to .configure() now I think about it, and shall modify the RFC)

I think we need something like this to be able to play with new backends like firefly. So I'd like:
1. To accept this RFC (I don't think it's controversial)
2. Prototype firefly with the current API
3. File another RFC on a better interface based on the experience with two (or more) backends.

There's backwards compatibility with e.g.

 import lsst.afw.display as afwDisplay afwDisplay.setBackend("ds9") ds9 = afwDisplay

Show
Robert Lupton added a comment - (I'd prefer .setBackend() to .configure() now I think about it, and shall modify the RFC) I think we need something like this to be able to play with new backends like firefly. So I'd like: 1. To accept this RFC (I don't think it's controversial) 2. Prototype firefly with the current API 3. File another RFC on a better interface based on the experience with two (or more) backends. There's backwards compatibility with e.g. import lsst.afw.display as afwDisplay afwDisplay.setBackend("ds9") ds9 = afwDisplay
Field Original Value New Value
Description The current stack image display code assumes that you're talking to ds9:
{code}
import lsst.afw.display.ds9 as ds9

ds9.erase()
{code}
As the IPAC team thinks about supporting firefly, and ginga gains traction in the scipy world, we need replace this interface with something more general.

In the longer run we need to decide what APIs we need to support, but this is hard to do until we have more than one backend. This RFC proposes that we change the interface to something like:
{code}
import lsst.afw.display.imdisplay as imdisplay
imdisplay.configure('ds9')

imdisplay.erase()
{code}

It would be possible to hoist these methods to lsst.afw.display itself:
{code}
import lsst.afw.display as display
display.configure('ds9')

display.erase()
{code}
A discussion of the pros and cons is in scope for the RFC
The current stack image display code assumes that you're talking to ds9:
{code}
import lsst.afw.display.ds9 as ds9

ds9.erase()
{code}
As the IPAC team thinks about supporting firefly, and ginga gains traction in the scipy world, we need replace this interface with something more general.

In the longer run we need to decide what APIs we need to support, but this is hard to do until we have more than one backend. This RFC proposes that we change the interface to something like:
{code}
import lsst.afw.display.imdisplay as imdisplay
imdisplay.setBackend('ds9')

imdisplay.erase()
{code}

It would be possible to hoist these methods to lsst.afw.display itself:
{code}
import lsst.afw.display as afwDisplay
afwDisplay.setBackend('ds9')

afwDisplay.erase()
{code}
A discussion of the pros and cons is in scope for the RFC
Hide
Michael Wood-Vasey [X] (Inactive) added a comment -

+1
particularly in the context of your plan in comment 2.

Show
Michael Wood-Vasey [X] (Inactive) added a comment - +1 particularly in the context of your plan in comment 2.
Hide
Tim Jenness added a comment - - edited

This is obviously important. The ORAC-DR display system worked exactly like this with multiple backends (actually, they were configurable by the user so the pipeline code just said "display" and the pipeline worked out what that meant). Whether it's advisable to hard code ds9 into the code at all depends on how well functionality in the different display tools is abstracted by the display engine. Again, ORAC-DR had the notion that each tool knew whether it could handle displays of images, catalogs, spectra etc.

I know how to interface to GAIA/SkyCat if that is of interest as an additional data point.

Show
Tim Jenness added a comment - - edited This is obviously important. The ORAC-DR display system worked exactly like this with multiple backends (actually, they were configurable by the user so the pipeline code just said "display" and the pipeline worked out what that meant). Whether it's advisable to hard code ds9 into the code at all depends on how well functionality in the different display tools is abstracted by the display engine. Again, ORAC-DR had the notion that each tool knew whether it could handle displays of images, catalogs, spectra etc. I know how to interface to GAIA/SkyCat if that is of interest as an additional data point.
Hide
Jim Bosch added a comment -

What about using a class instance instead of a module global variable to represent the connection to a particular display window? I think that's a more general interface, in that it supports connections that have more state than we'd want to stuff in global module variables, and I think it better captures the idea that we're displaying multiple images. With that approach, you'd set the backend by constructing an instance of the right class, which would all just be subclasses of a common base class.

In the DS9 implementation of such an interface, I'm not sure if we'd want to have one instance for all frames while continuing to use a frame kwarg, or one instance for each frame; it might depend on whether other display tools we're considering have a frame or layer concept.

Show
Jim Bosch added a comment - What about using a class instance instead of a module global variable to represent the connection to a particular display window? I think that's a more general interface, in that it supports connections that have more state than we'd want to stuff in global module variables, and I think it better captures the idea that we're displaying multiple images. With that approach, you'd set the backend by constructing an instance of the right class, which would all just be subclasses of a common base class. In the DS9 implementation of such an interface, I'm not sure if we'd want to have one instance for all frames while continuing to use a frame kwarg, or one instance for each frame; it might depend on whether other display tools we're considering have a frame or layer concept.
Hide
Robert Lupton added a comment -

Re Tim Jenness's comment, are you proposing that the system guess what backend you want? I can see providing a default (maybe "guess"), but I'd have thought users would want control (maybe via a . file, of course – cf. matplotlib). For example, within a few months I hope to see ds9 and firefly as viable backends; having GAIA/skycat too would be great. I'm not sure which I'll want on a particular day.

Show
Robert Lupton added a comment - Re Tim Jenness 's comment, are you proposing that the system guess what backend you want? I can see providing a default (maybe "guess"), but I'd have thought users would want control (maybe via a . file, of course – cf. matplotlib). For example, within a few months I hope to see ds9 and firefly as viable backends; having GAIA/skycat too would be great. I'm not sure which I'll want on a particular day.
Hide
John Swinbank added a comment -

In first reading, my concern – and I think it's related to Tim Jenness's point – is that it's not obvious why:

 import lsst.afw.display as afwDisplay afwDisplay.setBackend('ds9')

is any more general than

 import lsst.afw.display.ds9 as ds9

– in both cases, you've baked into the pipeline code that you'll be using ds9 for display. That might be necessary if whatever it is you're displaying absolutely needs ds9's functionality, but it doesn't seem to be much of a win. Better is to let the user configure whichever backend they like (by means of command line argument, dotfile, whatever).

Based on the subsequent comment, I think that was implicit in your proposal, and the hard-coded 'ds9' was just a placeholder. In which case I am happy.

Show
John Swinbank added a comment - In first reading, my concern – and I think it's related to Tim Jenness 's point – is that it's not obvious why: import lsst.afw.display as afwDisplay afwDisplay.setBackend('ds9') is any more general than import lsst.afw.display.ds9 as ds9 – in both cases, you've baked into the pipeline code that you'll be using ds9 for display. That might be necessary if whatever it is you're displaying absolutely needs ds9's functionality, but it doesn't seem to be much of a win. Better is to let the user configure whichever backend they like (by means of command line argument, dotfile, whatever). Based on the subsequent comment, I think that was implicit in your proposal, and the hard-coded 'ds9' was just a placeholder. In which case I am happy.
Hide
Tim Jenness added a comment -

I'm saying that Frossie Economou required a system that completely separated the code requesting the display of spectrum/catalog/image/overlay from the configuration code that actually expressed the desires of the user as to what tool should be used. In fact, sometimes a display command in the pipeline code would not result in a display at all because the user had declared that a particular product was not of interest to them. All the "are you interested in product X" and "how should we display product Y" decisions were under the control of the user (instrument scientist in reality) and no code was tweaked.

Show
Tim Jenness added a comment - I'm saying that Frossie Economou required a system that completely separated the code requesting the display of spectrum/catalog/image/overlay from the configuration code that actually expressed the desires of the user as to what tool should be used. In fact, sometimes a display command in the pipeline code would not result in a display at all because the user had declared that a particular product was not of interest to them. All the "are you interested in product X" and "how should we display product Y" decisions were under the control of the user (instrument scientist in reality) and no code was tweaked.
 Description The current stack image display code assumes that you're talking to ds9: {code} import lsst.afw.display.ds9 as ds9 ds9.erase() {code} As the IPAC team thinks about supporting firefly, and ginga gains traction in the scipy world, we need replace this interface with something more general. In the longer run we need to decide what APIs we need to support, but this is hard to do until we have more than one backend. This RFC proposes that we change the interface to something like: {code} import lsst.afw.display.imdisplay as imdisplay imdisplay.setBackend('ds9') imdisplay.erase() {code} It would be possible to hoist these methods to lsst.afw.display itself: {code} import lsst.afw.display as afwDisplay afwDisplay.setBackend('ds9') afwDisplay.erase() {code} A discussion of the pros and cons is in scope for the RFC The current stack image display code assumes that you're talking to ds9: {code} import lsst.afw.display.ds9 as ds9 ds9.erase() {code} As the IPAC team thinks about supporting firefly, and ginga gains traction in the scipy world, we need replace this interface with something more general. In the longer run we need to decide what APIs we need to support, but this is hard to do until we have more than one backend. This RFC proposes that we change the interface to something like: {code} import lsst.afw.display.imdisplay as imdisplay imdisplay.setBackend('ds9') imdisplay.erase() {code} It would be possible to hoist these methods to lsst.afw.display itself: {code} import lsst.afw.display as afwDisplay afwDisplay.setBackend('ds9') afwDisplay.erase() {code} A discussion of the pros and cons is in scope for the RFC. The choice of backend (in this case ds9) should be configurable either explicitly, as illustrated here, or via some deus ex machina technique (a dot file, an environment variable, ...). I put the backend choice explicitly in the examples, but I would not expect this to appear in utility code.
Hide
Robert Lupton added a comment -

I clarified the RFC. Utility code should not call the "setBackend" method.

Show
Robert Lupton added a comment - I clarified the RFC. Utility code should not call the "setBackend" method.
Hide
Robert Lupton added a comment -

Re Jim Bosch's comment, having separate connection objects sounds like a nice way to generalise the ds9-centric frame argument. I think you'd want to do this via a factory object, which is itself configured in the ways that we have been discussing.

I'll include something along these lines in my prototype implementation of this RFC.

Show
Robert Lupton added a comment - Re Jim Bosch 's comment, having separate connection objects sounds like a nice way to generalise the ds9-centric frame argument. I think you'd want to do this via a factory object, which is itself configured in the ways that we have been discussing. I'll include something along these lines in my prototype implementation of this RFC.
Hide
Tim Jenness added a comment -

I'll note that for the "do I want this displayed" logic to work, there needs to be a way for the display system to understand what stage in the processing this product represents. Do the ExposureX objects, or whatever, know what's been done to them?

Show
Tim Jenness added a comment - I'll note that for the "do I want this displayed" logic to work, there needs to be a way for the display system to understand what stage in the processing this product represents. Do the ExposureX objects, or whatever, know what's been done to them?
Hide
Frossie Economou added a comment -

Yes, I am very keen on the design approach of (a) decoupling the code from a gui implementation and (b) giving control to the user as to how to tap into the display [Gregory Dubois-Felsmann got an earful of my opinions on the matter the other day]. It stands up to a wide range of scenarios and people are happy. You can have a default configuration for the display, but the code itself should be display agnostic.

Show
Frossie Economou added a comment - Yes, I am very keen on the design approach of (a) decoupling the code from a gui implementation and (b) giving control to the user as to how to tap into the display [ Gregory Dubois-Felsmann got an earful of my opinions on the matter the other day]. It stands up to a wide range of scenarios and people are happy. You can have a default configuration for the display, but the code itself should be display agnostic.
Hide
Robert Lupton added a comment -

Re Tim Jenness's comment:

I'll note that for the "do I want this displayed" logic to work, there needs to be a way for the display system to understand what stage in the processing this product represents. Do the ExposureX objects, or whatever, know what's been done to them?

I think this is out of scope for this RFC, which is about display technology.

The answer is no, they don't. We are not currently configuring debug displays this way (they are configured via the lsstDebug mechanism), although it sounds perfectly reasonable. It surely ties into the whole provenance system which doesn't exist yet.

Show
Robert Lupton added a comment - Re Tim Jenness 's comment: I'll note that for the "do I want this displayed" logic to work, there needs to be a way for the display system to understand what stage in the processing this product represents. Do the ExposureX objects, or whatever, know what's been done to them? I think this is out of scope for this RFC, which is about display technology. The answer is no, they don't. We are not currently configuring debug displays this way (they are configured via the lsstDebug mechanism), although it sounds perfectly reasonable. It surely ties into the whole provenance system which doesn't exist yet.
Hide
Robert Lupton added a comment -

I've pushed a prototype to tickets/pr-42, with a notebook to illustrate the API in examples/imageDisplay.ipynb (I can't get nbviewer to display that notebook, I think because it's on a branch; for now see http://www.astro.princeton.edu/~rhl/imageDisplay.html).

I adopted Jim's suggestion to make the interface use objects, and support "virtualDisplay" and "ds9"

Show
Robert Lupton added a comment - I've pushed a prototype to tickets/pr-42, with a notebook to illustrate the API in examples/imageDisplay.ipynb (I can't get nbviewer to display that notebook, I think because it's on a branch; for now see http://www.astro.princeton.edu/~rhl/imageDisplay.html ). I adopted Jim's suggestion to make the interface use objects, and support "virtualDisplay" and "ds9"
 Resolution Done [ 10000 ] Status Proposed [ 10805 ] Adopted [ 10806 ]
Hide
Xiuqin Wu [X] (Inactive) added a comment -

SUI team at IPAC got together and discussed this RFC and Robert's new implementation. In general, Firefly will be able to support this with Python APIs. More details and questions later.

Show
Xiuqin Wu [X] (Inactive) added a comment - SUI team at IPAC got together and discussed this RFC and Robert's new implementation. In general, Firefly will be able to support this with Python APIs. More details and questions later.
 Link This issue is triggering DM-2709 [ DM-2709 ]
 Status Adopted [ 10806 ] Implemented [ 11105 ]

#### People

Assignee:
Robert Lupton
Reporter:
Robert Lupton
Watchers:
Frossie Economou, Gregory Dubois-Felsmann, Jim Bosch, John Swinbank, Michael Wood-Vasey [X] (Inactive), Robert Lupton, Russell Owen, Tim Jenness, Xiuqin Wu [X] (Inactive)
0 Vote for this issue
Watchers:
9 Start watching this issue

#### Dates

Created:
Updated:
Resolved:
Planned End:

#### Jenkins Builds

No builds found.