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

View spectrum from single spaxel on hover #2647

Merged
merged 14 commits into from
Jan 22, 2024
2 changes: 2 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ Cubeviz

- Moment map plugin now supports linear per-spaxel continuum subtraction. [#2587]

- Single-pixel subset tool now shows spectrum-at-spaxel on hover. [#2647]

Imviz
^^^^^

Expand Down
10 changes: 8 additions & 2 deletions docs/cubeviz/displaycubes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,10 @@ the bottom of the UI.
Spectrum At Spaxel
==================

This tool allows the user to create a one spaxel subset in an image viewer. This subset will then be
This tool allows the user to create a single-spaxel subset in an image viewer. This subset will then be
visualized in the spectrum viewer by showing the spectrum at that spaxel.
Activate this tool and then left-click to create the new region.
While this tool is active, hovering over a pixel in the image viewer will show a preview of the spectrum
at that spaxel in the spectrum viewer, and left-clicking will create a new subset at that spaxel.
Click again to move the region to a new location under the cursor. Holding down the
alt key (Alt key on Windows, Option key on Mac) while clicking on a spaxel creates a new subset at
that point instead of moving the previously created region.
Expand All @@ -110,6 +111,11 @@ You can also use the subset modes that are explained in the
:ref:`Spatial Regions <imviz_defining_spatial_regions>`
section above in the same way you would with the other subset selection tools.

Note that moving the cursor outside of the image viewer or deactivating the spectrum-at-spaxel tool
will revert the spectrum viewer zoom limits from the zoomed-in preview view to the limits set prior
to using the tool. Thus it may be necessary to reset the zoom to see any single-spaxel subset spectra
created using the tool.

.. _cubeviz-display-settings:

Display Settings
Expand Down
11 changes: 11 additions & 0 deletions jdaviz/configs/cubeviz/plugins/tests/test_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ def test_spectrum_at_spaxel(cubeviz_helper, spectrum1d_cube):
assert len(flux_viewer.native_marks) == 2
assert len(spectrum_viewer.data()) == 1

# Move to spaxel location
flux_viewer.toolbar.active_tool.on_mouse_move(
{'event': 'mousemove', 'domain': {'x': x, 'y': y}, 'altKey': False})
assert flux_viewer.toolbar.active_tool._mark in spectrum_viewer.figure.marks
assert flux_viewer.toolbar.active_tool._mark.visible is True

# Click on spaxel location
flux_viewer.toolbar.active_tool.on_mouse_event(
{'event': 'click', 'domain': {'x': x, 'y': y}, 'altKey': False})
Expand All @@ -30,6 +36,11 @@ def test_spectrum_at_spaxel(cubeviz_helper, spectrum1d_cube):
assert len(subsets) == 1
assert isinstance(reg, RectanglePixelRegion)

# Mouse leave event
flux_viewer.toolbar.active_tool.on_mouse_move(
{'event': 'mouseleave', 'domain': {'x': x, 'y': y}, 'altKey': False})
assert flux_viewer.toolbar.active_tool._mark.visible is False

# Deselect tool
flux_viewer.toolbar.active_tool = None
assert len(flux_viewer.native_marks) == 3
Expand Down
58 changes: 58 additions & 0 deletions jdaviz/configs/cubeviz/plugins/tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@
from glue.config import viewer_tool
from glue_jupyter.bqplot.image import BqplotImageView
from glue.viewers.common.tool import CheckableTool
import numpy as np

from jdaviz.configs.imviz.plugins.tools import _MatchedZoomMixin
from jdaviz.core.events import SliceToolStateMessage
from jdaviz.core.tools import PanZoom, BoxZoom, SinglePixelRegion
from jdaviz.core.marks import PluginLine

__all__ = []

Expand Down Expand Up @@ -83,3 +85,59 @@
tool_id = 'jdaviz:spectrumperspaxel'
action_text = 'See spectrum at a single spaxel'
tool_tip = 'Click on the viewer and see the spectrum at that spaxel in the spectrum viewer'

def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
self._spectrum_viewer = None
self._previous_bounds = None
self._mark = None
self._data = None

def _reset_spectrum_viewer_bounds(self):
sv_state = self._spectrum_viewer.state
sv_state.x_min = self._previous_bounds[0]
sv_state.x_max = self._previous_bounds[1]
sv_state.y_min = self._previous_bounds[2]
sv_state.y_max = self._previous_bounds[3]

def activate(self):
self.viewer.add_event_callback(self.on_mouse_move, events=['mousemove', 'mouseleave'])
if self._spectrum_viewer is None:
self._spectrum_viewer = self.viewer.jdaviz_helper.app.get_viewer('spectrum-viewer')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we avoid hardcoding 'spectrum-viewer' here by using the helper attributes?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still think this might be worth doing to avoid future headache

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, grabs the first profile viewer now.

if self._mark is None:
self._mark = PluginLine(self._spectrum_viewer, visible=False)
self._spectrum_viewer.figure.marks = self._spectrum_viewer.figure.marks + [self._mark,]
# Store these so we can revert to previous user-set zoom after preview view
sv_state = self._spectrum_viewer.state
self._previous_bounds = [sv_state.x_min, sv_state.x_max, sv_state.y_min, sv_state.y_max]
super().activate()

def deactivate(self):
self.viewer.remove_event_callback(self.on_mouse_move)
self._reset_spectrum_viewer_bounds()
super().deactivate()

def on_mouse_move(self, data):
if data['event'] == 'mouseleave':
self._mark.visible = False
self._reset_spectrum_viewer_bounds()
return

x = int(np.round(data['domain']['x']))
y = int(np.round(data['domain']['y']))

# Use first visible layer for now
cube_data = [layer.layer for layer in self.viewer.layers if layer.state.visible][0]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this use similar logic as #2646 to ensure its pulling from the flux cube? What happens when there are no visible layers (I'm guessing a traceback... we might want to just skip that case gracefully)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed this to return on an empty result, good catch.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic in 2646 might be useful, but I don't think it's worth holding this up for that to be merged. We could potentially do that if/when we change the axis handling.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, but the first visible layer now could still be a flat image, or the uncertainty or mask cubes, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a check to at least only use 3D data, avoiding a traceback if something like a collapsed image is displayed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thinking about this more... I think this should listen to the layer selection in mouseover itself, which also might fix the bug with the uncertainty cube failing to display. That or we just ALWAYS pull the flux cube, regardless of the viewer and whether it is an active layer in that viewer or not 🤷

spectrum = cube_data.get_object(statistic=None)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it worth caching this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about getting this on activation but was worried about data changing while the tool is active, and since it seems performant I didn't think it was worth the overhead of adding a hub listener or something along those lines.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this might not be performant on large cubes (or anything where get_object is expensive), but haven't tested. I guess we can leave that for future improvement, since depending on the decision of how to handle layer selection, you may need to have a listener to invalidate the cache


if x >= spectrum.flux.shape[0] or x < 0 or y >= spectrum.flux.shape[1] or y < 0:
self._mark.visible = False

Check warning on line 134 in jdaviz/configs/cubeviz/plugins/tools.py

View check run for this annotation

Codecov / codecov/patch

jdaviz/configs/cubeviz/plugins/tools.py#L134

Added line #L134 was not covered by tests
kecnry marked this conversation as resolved.
Show resolved Hide resolved
else:
self._mark.visible = True
kecnry marked this conversation as resolved.
Show resolved Hide resolved
y_values = spectrum.flux[x, y, :]
if np.all(np.isnan(y_values)):
self._mark.visible = False
return

Check warning on line 140 in jdaviz/configs/cubeviz/plugins/tools.py

View check run for this annotation

Codecov / codecov/patch

jdaviz/configs/cubeviz/plugins/tools.py#L139-L140

Added lines #L139 - L140 were not covered by tests
self._mark.update_xy(spectrum.spectral_axis.value, y_values)
self._spectrum_viewer.state.y_max = np.nanmax(y_values.value) * 1.2
self._spectrum_viewer.state.y_min = np.nanmin(y_values.value) * 0.8
kecnry marked this conversation as resolved.
Show resolved Hide resolved
Loading