Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
mwcraig opened this issue Sep 13, 2021 · 12 comments
Open

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

mwcraig opened this issue Sep 13, 2021 · 12 comments
Labels
API Issues that relate to the API itself rather than implementations bug Something isn't working

Comments

@mwcraig
Copy link
Member

mwcraig commented Sep 13, 2021

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.

@mwcraig mwcraig added the bug Something isn't working label Sep 13, 2021
@pllim
Copy link
Member

pllim commented Sep 13, 2021

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
Copy link
Member

pllim commented Sep 13, 2021

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
Copy link
Member Author

mwcraig commented Sep 13, 2021

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
Copy link
Member Author

mwcraig commented Sep 13, 2021

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

@pllim
Copy link
Member

pllim commented Sep 13, 2021

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
Copy link
Member

pllim commented Sep 13, 2021

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
Copy link
Member Author

mwcraig commented Sep 13, 2021

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
Copy link
Member

pllim commented Sep 13, 2021

I guess I don't understand your comment at #149 (comment) -- Are you saying there is a bug in the notebook, Ginga backend implementation here, or Ginga WCS handling?

@mwcraig
Copy link
Member Author

mwcraig commented Sep 13, 2021

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
Copy link
Member

pllim commented Sep 14, 2021

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
Copy link
Member

pllim commented Sep 14, 2021

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

@ejeschke
Copy link
Contributor

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

@pllim pllim added backend: ginga API Issues that relate to the API itself rather than implementations and removed backend: ginga labels Jun 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Issues that relate to the API itself rather than implementations bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants