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

Fix Cubeviz coordinates panel with inaccurate sky coordinates sometimes #2002

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ New Features
Cubeviz
^^^^^^^

- Moment map output now has celestial WCS, when applicable. [#2002]

Imviz
^^^^^

Expand Down Expand Up @@ -43,6 +45,11 @@ Bug Fixes
Cubeviz
^^^^^^^

- Fixed a bug where sky coordinates reported to coordinates info panel
might be wrong for "uncert" and "mask" data. This bug only happens when
certain parsing conditions were met. When in doubt, always verify with
info from "flux" data. [#2002]

Imviz
^^^^^

Expand Down
10 changes: 9 additions & 1 deletion jdaviz/configs/cubeviz/plugins/moment_maps/moment_maps.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

from astropy import units as u
from astropy.nddata import CCDData
from astropy.wcs import WCS

from traitlets import Unicode, Bool, observe
from specutils import Spectrum1D, manipulation, analysis
Expand Down Expand Up @@ -103,7 +104,14 @@ def calculate_moment(self, add_data=True):
# Need transpose to align JWST mirror shape. Not sure why.
# TODO: WCS can be grabbed from cube.wcs[:, :, 0] but CCDData will not take it.
# But if we use NDData, glue-astronomy translator fails.
self.moment = CCDData(analysis.moment(slab, order=n_moment).T)
data = self.dataset.selected_dc_item
if isinstance(data.coords, WCS):
kwargs = {'wcs': data.coords.celestial}
elif isinstance(data.meta.get('_orig_wcs', None), WCS):
kwargs = {'wcs': data.meta['_orig_wcs'].celestial}
else:
kwargs = {}
self.moment = CCDData(analysis.moment(slab, order=n_moment).T, **kwargs)

fname_label = self.dataset_selected.replace("[", "_").replace("]", "")
self.filename = f"moment{n_moment}_{fname_label}.fits"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import pytest
from astropy.io import fits
from astropy.nddata import CCDData
from astropy.wcs import WCS
from numpy.testing import assert_allclose

from jdaviz.configs.cubeviz.plugins.moment_maps.moment_maps import MomentMap
Expand Down Expand Up @@ -36,7 +37,7 @@ def test_moment_calculation(cubeviz_helper, spectrum1d_cube, tmpdir):
assert len(mv_data) == 1
assert mv_data[0].label == 'moment 0'

assert len(dc.links) == 8
assert len(dc.links) == 14

# label should remain unchanged but raise overwrite warnings
assert mm.results_label == 'moment 0'
Expand All @@ -47,8 +48,8 @@ def test_moment_calculation(cubeviz_helper, spectrum1d_cube, tmpdir):
assert flux_viewer.state.slices == (0, 0, 1)
assert flux_viewer.label_mouseover.pixel == 'x=00.0 y=00.0'
assert flux_viewer.label_mouseover.value == '+8.00000e+00 Jy' # Slice 0 has 8 pixels, this is Slice 1 # noqa
assert flux_viewer.label_mouseover.world_ra_deg == '204.9997755346'
assert flux_viewer.label_mouseover.world_dec_deg == '27.0000999998'
assert flux_viewer.label_mouseover.world_ra_deg == '204.9998877673'
assert flux_viewer.label_mouseover.world_dec_deg == '27.0001000000'

# Make sure adding it to viewer does not crash.
cubeviz_helper.app.add_data_to_viewer(
Expand All @@ -57,16 +58,14 @@ def test_moment_calculation(cubeviz_helper, spectrum1d_cube, tmpdir):

result = dc[1].get_object(cls=CCDData)
assert result.shape == (4, 2) # Cube shape is (2, 2, 4)

# FIXME: Need spatial WCS, see https://github.com/spacetelescope/jdaviz/issues/1025
assert dc[1].coords is None
assert isinstance(dc[1].coords, WCS)

# Make sure coordinate display now show moment map info (no WCS)
flux_viewer.on_mouse_or_key_event({'event': 'mousemove', 'domain': {'x': 0, 'y': 0}})
assert flux_viewer.label_mouseover.pixel == 'x=00.0 y=00.0'
assert flux_viewer.label_mouseover.value == '+8.00000e+00 Jy' # Slice 0 has 8 pixels, this is Slice 1 # noqa
assert flux_viewer.label_mouseover.world_ra_deg == '204.9997755346'
assert flux_viewer.label_mouseover.world_dec_deg == '27.0000999998'
assert flux_viewer.label_mouseover.world_ra_deg == '204.9998877673'
assert flux_viewer.label_mouseover.world_dec_deg == '27.0001000000'

assert mm.filename == 'moment0_test_FLUX.fits' # Auto-populated on calculate.
mm.filename = str(tmpdir.join(mm.filename)) # But we want it in tmpdir for testing.
Expand All @@ -80,26 +79,15 @@ def test_moment_calculation(cubeviz_helper, spectrum1d_cube, tmpdir):

assert dc[2].label == 'moment 1'

assert len(dc.links) == 10
assert len(dc.external_links) == 4
assert len(dc.links) == 22
assert len(dc.external_links) == 2
# Link 3D z to 2D x and 3D y to 2D y

# Link 3:
# Pixel Axis 0 [z] from cube.pixel_component_ids[0]
# Pixel Axis 1 [x] from plugin.pixel_component_ids[1]
assert dc.external_links[2].cids1[0] == dc[0].pixel_component_ids[0]
assert dc.external_links[2].cids2[0] == dc[-1].pixel_component_ids[1]
# Link 4:
# Pixel Axis 1 [y] from cube.pixel_component_ids[1]
# Pixel Axis 0 [y] from plugin.pixel_component_ids[0]
assert dc.external_links[3].cids1[0] == dc[0].pixel_component_ids[1]
assert dc.external_links[3].cids2[0] == dc[-1].pixel_component_ids[0]

# Coordinate display should be unaffected.
assert flux_viewer.label_mouseover.pixel == 'x=00.0 y=00.0'
assert flux_viewer.label_mouseover.value == '+8.00000e+00 Jy' # Slice 0 has 8 pixels, this is Slice 1 # noqa
assert flux_viewer.label_mouseover.world_ra_deg == '204.9997755346'
assert flux_viewer.label_mouseover.world_dec_deg == '27.0000999998'
assert flux_viewer.label_mouseover.world_ra_deg == '204.9998877673'
assert flux_viewer.label_mouseover.world_dec_deg == '27.0001000000'


@pytest.mark.filterwarnings('ignore:No observer defined on WCS')
Expand All @@ -125,11 +113,16 @@ def test_write_momentmap(cubeviz_helper, spectrum1d_cube, tmp_path):

# We should get an overwrite warning
assert plugin._obj.overwrite_warn is True
# and shouldn't have written anything (the file should be in tact)
# and shouldn't have written anything (the file should be intact)
assert test_file.read_text() == existing_sentinel_text

# Force overwrite the existing file
plugin._obj.vue_overwrite_fits()

# Read back in the file and check that it is equal to the one we calculated
assert_allclose(moment.data, fits.getdata(test_file_str))
with fits.open(test_file_str) as pf:
assert_allclose(pf[0].data, moment.data)
w = WCS(pf[0].header)
sky = w.pixel_to_world(0, 0)
assert_allclose(sky.ra.deg, 204.99988777)
assert_allclose(sky.dec.deg, 27.0001)
14 changes: 8 additions & 6 deletions jdaviz/configs/cubeviz/plugins/tests/test_parsers.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,13 @@ def test_fits_image_hdu_with_microns(image_cube_hdu_obj_microns, cubeviz_helper)
flux_viewer.on_mouse_or_key_event({'event': 'mousemove', 'domain': {'x': 0, 'y': 0}})
assert flux_viewer.label_mouseover.pixel == 'x=00.0 y=00.0'
assert flux_viewer.label_mouseover.value == '+1.00000e+00 1e-17 erg / (Angstrom cm2 s)'
# Sky: 205.4398996, 27.00341788

unc_viewer = cubeviz_helper.app.get_viewer('uncert-viewer')
unc_viewer.on_mouse_or_key_event({'event': 'mousemove', 'domain': {'x': -1, 'y': 0}})
assert unc_viewer.label_mouseover.pixel == 'x=-1.0 y=00.0'
assert unc_viewer.label_mouseover.value == '' # Out of bounds
# Sky: 205.43994013, 27.00341788


def test_spectrum1d_with_fake_fixed_units(spectrum1d, cubeviz_helper):
Expand Down Expand Up @@ -103,15 +105,15 @@ def test_fits_image_hdu_parse_from_file(tmpdir, image_cube_hdu_obj, cubeviz_help
flux_viewer.on_mouse_or_key_event({'event': 'mousemove', 'domain': {'x': 0, 'y': 0}})
assert flux_viewer.label_mouseover.pixel == 'x=00.0 y=00.0'
assert flux_viewer.label_mouseover.value == '+1.00000e+00 1e-17 erg / (Angstrom cm2 s)'
assert flux_viewer.label_mouseover.world_ra_deg == '205.4433848390'
assert flux_viewer.label_mouseover.world_dec_deg == '26.9996149270'
assert flux_viewer.label_mouseover.world_ra_deg == '205.4441642302'
assert flux_viewer.label_mouseover.world_dec_deg == '26.9996148973'

unc_viewer = cubeviz_helper.app.get_viewer(cubeviz_helper._default_uncert_viewer_reference_name)
unc_viewer.on_mouse_or_key_event({'event': 'mousemove', 'domain': {'x': -1, 'y': 0}})
assert unc_viewer.label_mouseover.pixel == 'x=-1.0 y=00.0'
assert unc_viewer.label_mouseover.value == '' # Out of bounds
assert unc_viewer.label_mouseover.world_ra_deg == '205.4441642302'
assert unc_viewer.label_mouseover.world_dec_deg == '26.9996148973'
assert unc_viewer.label_mouseover.world_ra_deg == '205.4443201084'
assert unc_viewer.label_mouseover.world_dec_deg == '26.9996148908'


@pytest.mark.filterwarnings('ignore')
Expand All @@ -131,8 +133,8 @@ def test_spectrum3d_parse(image_cube_hdu_obj, cubeviz_helper):
flux_viewer.on_mouse_or_key_event({'event': 'mousemove', 'domain': {'x': 0, 'y': 0}})
assert flux_viewer.label_mouseover.pixel == 'x=00.0 y=00.0'
assert flux_viewer.label_mouseover.value == '+1.00000e+00 1e-17 erg / (Angstrom cm2 s)'
assert flux_viewer.label_mouseover.world_ra_deg == '205.4433848390'
assert flux_viewer.label_mouseover.world_dec_deg == '26.9996149270'
assert flux_viewer.label_mouseover.world_ra_deg == '205.4441642302'
assert flux_viewer.label_mouseover.world_dec_deg == '26.9996148973'

# These viewers have no data.

Expand Down
22 changes: 19 additions & 3 deletions jdaviz/configs/cubeviz/plugins/viewers.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import numpy as np
from astropy.coordinates import SkyCoord
from astropy.wcs import WCS
from glue.core import BaseData
from glue.core.subset import RoiSubsetState, RangeSubsetState
from glue_jupyter.bqplot.image import BqplotImageView
Expand Down Expand Up @@ -49,6 +51,20 @@ def __init__(self, *args, **kwargs):
self.add_event_callback(self.on_mouse_or_key_event, events=['mousemove', 'mouseenter',
'mouseleave'])

def _get_cube_skycoord(self, w, x, y):
if isinstance(w, WCS):
# TODO: Is x always RA and y always DEC?
pllim marked this conversation as resolved.
Show resolved Hide resolved
if w.celestial.axis_type_names == ['RA', 'DEC']:
sky = w.celestial.pixel_to_world(x, y)
else: # ['DEC', 'RA']
sky = w.celestial.pixel_to_world(y, x)
else: # The old and possibly incorrect way
sky_list = w.pixel_to_world(x, y, self.state.slices[-1])
for sky in sky_list:
if isinstance(sky, SkyCoord):
break
return sky.icrs

def on_mouse_or_key_event(self, data):

# Find visible layers
Expand Down Expand Up @@ -93,18 +109,18 @@ def on_mouse_or_key_event(self, data):
# data for this application. This section will need to be updated
# when that is no longer true.
# Hack to insert WCS for generated 2D and 3D images using FLUX cube WCS.
if 'Plugin' in image.meta:
if 'Plugin' in image.meta and not image.coords:
coo_data = self.jdaviz_app.data_collection[0]
else:
coo_data = image

# Hack around various WCS propagation issues in Cubeviz.
if '_orig_wcs' in coo_data.meta:
coo = coo_data.meta['_orig_wcs'].pixel_to_world(x, y, self.state.slices[-1])[0].icrs
coo = self._get_cube_skycoord(coo_data.meta['_orig_wcs'], x, y)
self.label_mouseover.set_coords(coo)
elif data_has_valid_wcs(coo_data):
try:
coo = coo_data.coords.pixel_to_world(x, y, self.state.slices[-1])[-1].icrs
coo = self._get_cube_skycoord(coo_data.coords, x, y)
except Exception:
self.label_mouseover.reset_coords_display()
else:
Expand Down