astropy / astrowidgets

*PRE-ALPHA*/heavy dev. Jupyter widgets leveraging the Astropy ecosystem
https://astrowidgets.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
31 stars 14 forks source link

Coordinates in cursor display may not be RA/Dec #149

Open mwcraig opened 3 years ago

mwcraig commented 3 years ago

I noticed while working on #142 that although the cursor is always labeled as RA/Dec it is not necessary actually RA/Dec. In the ginga implementation the sample image has a WCS in galactic coordinates and that is what the cursor displays. I don't know that that is wrong, necessarily, but it is incorrect to label it as RA/Dec.

Seems like there are a couple of options:

  1. always display RA/DEC (ICRS? other?), converting as necessary.
  2. Display whatever coordinates the WCS is in, with appropriate names for the lat/lon.

@pllim -- you may want to check what the imviz viewer currently does. This question above is probably an API question rather than an implementation decision.

pllim commented 3 years ago

I think in Ginga, we always convert to ICRS anyway? Can't remember exactly now.

For Imviz, that also appears to be the case, as @astrofrog coded it: https://github.com/spacetelescope/jdaviz/blob/c2cf8cb4160448628ec480ba98e05abb7f852490/jdaviz/configs/imviz/plugins/viewers.py#L86

pllim commented 3 years ago

For Ginga, it does default to ICRS too but there is an option to customize for standalone app: https://github.com/ejeschke/ginga/blob/61d9791fc2803869174243510a2fabe15b22af2e/ginga/examples/configs/channel_Image.cfg#L58

mwcraig commented 3 years ago

In the current ginga demo notebook, though, centering on a SkyCoord is done with

from astropy.coordinates import SkyCoord

# Change the values here if you are not using given
# example image.
ra_str = '01h13m23.193s'
dec_str = '+00d12m32.19s'
frame = 'icrs'

# Programmatically center to SkyCoord on viewer
w.center_on(SkyCoord(ra_str, dec_str, frame=frame))

This centers on one of the bright stars even though the RA/Dec is very different according to the FITS header:

RA      =         275.92636108 / [Deg] Right ascension at mosaic center         
DEC     =         -13.25728989 / [Deg] Declination at mosaic center  
mwcraig commented 3 years ago

So it sounds like the issue may really just be that ginga seems to assume that the initial WCS is galactic not ICRS....

pllim commented 3 years ago

I am not familiar with the content of that file. Just something I (or someone) grabbed from Astropy data server: http://data.astropy.org/photometry/spitzer_example_image.fits

Have to ask Spitzer what those keywords really mean, I guess?

pllim commented 3 years ago

But the initial WCS is Galactic... isn't it?

CRPIX1  = 1.161500000000000E+03 / Reference pixel for x-position
CRPIX2  = -3.885000000000000E+02 / Reference pixel for y-position
CTYPE1  = 'GLON-CAR'           / Projection Type
CTYPE2  = 'GLAT-CAR'           / Projection Type
CRVAL1  =          18.00000000 / [Deg] Galactic Longtitude at reference pixel
CRVAL2  =           0.00000000 / [Deg] Galactic Latitude at reference pixel
EQUINOX =               2000.0 / Equinox for celestial coordinate system
DELTA-X =           3.10666656 / [Deg] size of image in axis 1
DELTA-Y =           2.40666676 / [Deg] size of image in axis 2
BORDER  =           0.00333333 / [Deg] mosaic grid border
CD1_1   =      -3.33333330E-04
CD1_2   =       0.00000000E+00
CD2_1   =       0.00000000E+00
CD2_2   =       3.33333330E-04
PIXSCAL1=                1.200 / [arcsec/pixel] pixel scale for axis 1
PIXSCAL2=                1.200 / [arcsec/pixel] pixel scale for axis 2
OLDPIXSC=                1.213 / [arcsec/pixel] pixel scale of single IRAC frame
RA      =         275.92636108 / [Deg] Right ascension at mosaic center
DEC     =         -13.25728989 / [Deg] Declination at mosaic center
mwcraig commented 3 years ago

But the initial WCS is Galactic... isn't it?

Yep -- I noticed it because in the bqplot viewer draft I was explicitly setting the display to ICRS and was seeing different coordinates than in the ginga viewer, so I checked the WCS and saw that it was galactic.

pllim commented 3 years ago

I guess I don't understand your comment at https://github.com/astropy/astrowidgets/issues/149#issuecomment-918306427 -- Are you saying there is a bug in the notebook, Ginga backend implementation here, or Ginga WCS handling?

mwcraig commented 3 years ago

Are you saying there is a bug in the notebook, Ginga backend implementation here, or Ginga WCS handling?

I think I'm saying it is a combination of notebook bug, Ginga backend implementation, and maybe the use of Ginga's world/pixel conversions instead of astropy's.

When the Ginga WCS is created from this image it correctly recognizes that the WCS is in galactic coordinates. So far, so good.

  1. In the current implementation of the ginga backend I think there are two issues: a. Ginga returns two floats rather than a SkyCoord from its pixtoradec -- the issue is that the two numbers in this case are galactic longitude, ℓ, and latitude b, rather than RA/DEC b. The cursor display assumes these are RA and Dec and displays them as such.
  2. The other backend issue is in center_on, which assumes the SkyCoord is ICRS or something else with an RA/Dec. The API doesn't rule out providing coordinates in, e.g. galactic, though it doesn't explicitly support it either. Not sure how to handle that.
  3. Finally, in the notebook cell showing center_on the coordinate is really in frame galactic not frame ICRS. The cell works because center_on passes the RA/Dec to set_pan, which presumably assumes they are galactic ℓ and b because that is what the image WCS is.

I don't think there is anything to fix immediately -- I'm keeping track of the additional API questions I have as I work out the remaining bugs in the bqplot viewer.

In any event, it is good this was the example image -- I wouldn't have thought to try an image in a non-ICRS frame!

pllim commented 3 years ago

My head is spinning, LoL! Perhaps in the widget implementation of Ginga backend, there was an oversight. If you look at standalone app, there seems to be some system-specific handling at https://github.com/ejeschke/ginga/blob/61d9791fc2803869174243510a2fabe15b22af2e/ginga/AstroImage.py#L490 .

pllim commented 3 years ago

Just gonna try my luck to ping @ejeschke here in case he has insight on Ginga behavior.

ejeschke commented 3 years ago

@mwcraig, @pllim, this is simply the cursor display making the assumption about what the coordinates are (i.e. the format string here). The Ginga reference viewer has a more sophisticated processing of the WCS output.

Wouldn't it make sense to process the cursor display using common astropy routines if you have different backends? Basically you pass the X/Y coords to common routines that can return coordinates and labels as strings. Then these strings can be displayed according to the specific backend's method.