-
Notifications
You must be signed in to change notification settings - Fork 74
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
Cubeviz image viewer coordinates display #1315
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,12 +8,10 @@ | |
|
||
@pytest.mark.filterwarnings('ignore:No observer defined on WCS') | ||
def test_moment_calculation(cubeviz_helper, spectrum1d_cube, tmpdir): | ||
app = cubeviz_helper.app | ||
dc = app.data_collection | ||
app.add_data(spectrum1d_cube, 'test[FLUX]') | ||
app.add_data_to_viewer('flux-viewer', 'test[FLUX]') | ||
dc = cubeviz_helper.app.data_collection | ||
cubeviz_helper.load_data(spectrum1d_cube, data_label='test') | ||
|
||
mm = MomentMap(app=app) | ||
mm = MomentMap(app=cubeviz_helper.app) | ||
mm.dataset_selected = 'test[FLUX]' | ||
|
||
mm.n_moment = 0 # Collapsed sum, will get back 2D spatial image | ||
|
@@ -30,11 +28,20 @@ def test_moment_calculation(cubeviz_helper, spectrum1d_cube, tmpdir): | |
assert mm.results_label_overwrite is True | ||
|
||
result = dc[1].get_object(cls=CCDData) | ||
assert result.shape == (2, 4) # Cube shape is (2, 2, 4) | ||
assert result.shape == (4, 2) # Cube shape is (2, 2, 4) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But in changing from using |
||
|
||
# FIXME: Need spatial WCS, see https://github.com/spacetelescope/jdaviz/issues/1025 | ||
assert dc[1].coords is None | ||
|
||
# Make sure coordinate display still works | ||
flux_viewer = cubeviz_helper.app.get_viewer('flux-viewer') | ||
flux_viewer.on_mouse_or_key_event({'event': 'mousemove', 'domain': {'x': 0, 'y': 0}}) | ||
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 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. | ||
mm.vue_save_as_fits() | ||
|
@@ -52,3 +59,9 @@ def test_moment_calculation(cubeviz_helper, spectrum1d_cube, tmpdir): | |
assert dc[2].label == 'moment 1' | ||
|
||
assert len(dc.links) == 10 | ||
|
||
# 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' |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -75,9 +75,9 @@ def parse_data(app, file_obj, data_type=None, data_label=None): | |
# into something glue can understand. | ||
elif isinstance(file_obj, Spectrum1D): | ||
if file_obj.flux.ndim == 3: | ||
_parse_spectrum1d_3d(app, file_obj) | ||
_parse_spectrum1d_3d(app, file_obj, data_label=data_label) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is highly annoying to me that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you for doing this! It's actually bothered me before but I guess not enough to add it 😅 |
||
else: | ||
_parse_spectrum1d(app, file_obj) | ||
_parse_spectrum1d(app, file_obj, data_label=data_label) | ||
else: | ||
raise NotImplementedError(f'Unsupported data format: {file_obj}') | ||
|
||
|
@@ -202,8 +202,11 @@ def _parse_esa_s3d(app, hdulist, data_label, ext='DATA', viewer_name='flux-viewe | |
app.add_data_to_viewer('spectrum-viewer', data_label) | ||
|
||
|
||
def _parse_spectrum1d_3d(app, file_obj): | ||
# Load spectrum1d as a cube | ||
def _parse_spectrum1d_3d(app, file_obj, data_label=None): | ||
"""Load spectrum1d as a cube.""" | ||
|
||
if data_label is None: | ||
data_label = "Unknown spectrum object" | ||
|
||
for attr in ["flux", "mask", "uncertainty"]: | ||
val = getattr(file_obj, attr) | ||
|
@@ -224,20 +227,21 @@ def _parse_spectrum1d_3d(app, file_obj): | |
|
||
s1d = Spectrum1D(flux=flux, wcs=file_obj.wcs) | ||
|
||
data_label = f"Unknown spectrum object[{attr.upper()}]" | ||
app.add_data(s1d, data_label) | ||
cur_data_label = f"{data_label}[{attr.upper()}]" | ||
app.add_data(s1d, cur_data_label) | ||
|
||
if attr == 'flux': | ||
app.add_data_to_viewer('flux-viewer', data_label) | ||
app.add_data_to_viewer('spectrum-viewer', data_label) | ||
app.add_data_to_viewer('flux-viewer', cur_data_label) | ||
app.add_data_to_viewer('spectrum-viewer', cur_data_label) | ||
elif attr == 'mask': | ||
app.add_data_to_viewer('mask-viewer', data_label) | ||
app.add_data_to_viewer('mask-viewer', cur_data_label) | ||
else: # 'uncertainty' | ||
app.add_data_to_viewer('uncert-viewer', data_label) | ||
app.add_data_to_viewer('uncert-viewer', cur_data_label) | ||
|
||
|
||
def _parse_spectrum1d(app, file_obj): | ||
data_label = "Unknown spectrum object" | ||
def _parse_spectrum1d(app, file_obj, data_label=None): | ||
if data_label is None: | ||
data_label = "Unknown spectrum object" | ||
|
||
# TODO: glue-astronomy translators only look at the flux property of | ||
# specutils Spectrum1D objects. Fix to support uncertainties and masks. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,11 @@ | ||
import numpy as np | ||
from glue.core import BaseData | ||
from glue_jupyter.bqplot.image import BqplotImageView | ||
|
||
from jdaviz.core.registries import viewer_registry | ||
from jdaviz.core.marks import SliceIndicator | ||
from jdaviz.configs.default.plugins.viewers import JdavizViewerMixin | ||
from jdaviz.configs.imviz.helper import data_has_valid_wcs | ||
from jdaviz.configs.specviz.plugins.viewers import SpecvizProfileView | ||
|
||
__all__ = ['CubevizImageView', 'CubevizProfileView'] | ||
|
@@ -37,6 +39,76 @@ def __init__(self, *args, **kwargs): | |
self._initialize_toolbar_nested() | ||
self.state.add_callback('reference_data', self._initial_x_axis) | ||
|
||
self.label_mouseover = None | ||
self.add_event_callback(self.on_mouse_or_key_event, events=['mousemove', 'mouseenter', | ||
'mouseleave']) | ||
|
||
def on_mouse_or_key_event(self, data): | ||
|
||
# Find visible layers | ||
visible_layers = [layer for layer in self.state.layers if layer.visible] | ||
|
||
if len(visible_layers) == 0: | ||
return | ||
|
||
if self.label_mouseover is None: | ||
if 'g-coords-info' in self.session.application._tools: | ||
self.label_mouseover = self.session.application._tools['g-coords-info'] | ||
else: | ||
return | ||
|
||
if data['event'] == 'mousemove': | ||
# Display the current cursor coordinates (both pixel and world) as | ||
# well as data values. For now we use the first dataset in the | ||
# viewer for the data values. | ||
|
||
# Extract first dataset from visible layers and use this for coordinates - the choice | ||
# of dataset shouldn't matter if the datasets are linked correctly | ||
image = visible_layers[0].layer | ||
Comment on lines
+65
to
+67
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. how is This behavior/situation might be something that is worth adding test coverage, if possible. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The inner workings of visible layers, I'd have to defer to @astrofrog or @maartenbreddels. I wish there is an easier way for me to reliably grab this info too but then Glue lets you overlay this and that, which complicates things. This particular info panel would ignore the semi-transparent overlay use cases. Re: tests -- the info panel is kinda tested non-interactively in https://github.com/spacetelescope/jdaviz/blob/main/jdaviz/configs/imviz/tests/test_linking.py (the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. from trial and error, it seems that this does always show the "top" layer, which is the most sensible (simple) behavior. If there is any room, I do think a label would be useful, but will approve this and that can always be added later if you don't have time to include it in scope here (as I think the same thing could apply to the existing display in imviz). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Re: label -- In Imviz, I incorporated that into Compass plugin (because the label just seems to go with Compass naturally). Originally when I was new and naive, I attempted glue-viz/glue-jupyter#244 but failed. I don't think there is any more space for label in the coordinates display, as it is already pretty crowded. This does bring up a good point... What if someone deselects the cube and have a 2D moment map displayed in the cube viewer instead? Is that a use case we even support? |
||
|
||
# Extract data coordinates - these are pixels in the reference image | ||
x = data['domain']['x'] | ||
y = data['domain']['y'] | ||
|
||
if x is None or y is None: # Out of bounds | ||
self.label_mouseover.pixel = "" | ||
self.label_mouseover.reset_coords_display() | ||
self.label_mouseover.value = "" | ||
return | ||
|
||
maxsize = int(np.ceil(np.log10(np.max(image.shape[:2])))) + 3 | ||
fmt = 'x={0:0' + str(maxsize) + '.1f} y={1:0' + str(maxsize) + '.1f}' | ||
self.label_mouseover.pixel = (fmt.format(x, y)) | ||
|
||
if data_has_valid_wcs(image): | ||
try: | ||
coo = image.coords.pixel_to_world(x, y, self.state.slices[-1])[-1].icrs | ||
except Exception: | ||
self.label_mouseover.reset_coords_display() | ||
else: | ||
self.label_mouseover.set_coords(coo) | ||
else: | ||
self.label_mouseover.reset_coords_display() | ||
|
||
# Extract data values at this position. | ||
# Assume shape is [x, y, z] and not [y, x] like Imviz. | ||
if (x > -0.5 and y > -0.5 | ||
and x < image.shape[0] - 0.5 and y < image.shape[1] - 0.5 | ||
and hasattr(visible_layers[0], 'attribute')): | ||
attribute = visible_layers[0].attribute | ||
value = image.get_data(attribute)[int(round(x)), int(round(y)), | ||
self.state.slices[-1]] | ||
unit = image.get_component(attribute).units | ||
self.label_mouseover.value = f'{value:+10.5e} {unit}' | ||
else: | ||
self.label_mouseover.value = '' | ||
|
||
elif data['event'] == 'mouseleave' or data['event'] == 'mouseenter': | ||
|
||
self.label_mouseover.pixel = "" | ||
self.label_mouseover.reset_coords_display() | ||
self.label_mouseover.value = "" | ||
|
||
def _initial_x_axis(self, *args): | ||
# Make sure that the x_att is correct on data load | ||
ref_data = self.state.reference_data | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since I am updating this test anyway, I took the liberty to make it more realistic in loading like a user would.