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.

Sign up for GitHub

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

Jump to bottom

View spectrum from single spaxel on hover #2647

Merged
merged 14 commits into from
Jan 22, 2024
Merged

View spectrum from single spaxel on hover #2647

Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
07b9799
Working on adding spectrum-from-spaxel on hover
rosteen Jan 4, 2024
956cffb
First working version of spectrum on hover
rosteen Jan 5, 2024
e448c6c
Handle NaNs properly
rosteen Jan 5, 2024
5f9b843
Update docs and changelog
rosteen Jan 5, 2024
ec03400
Add simple test
rosteen Jan 5, 2024
cb3b1ed
Revert zoom after preview view
rosteen Jan 8, 2024
1d4be47
Address review comments
rosteen Jan 16, 2024
05bd078
Only use 3D data
rosteen Jan 16, 2024
29cd90c
Fix preview spectrum for uncertainty cube
rosteen Jan 19, 2024
5145cc4
Use the same layer selection as coords_info if possible
rosteen Jan 19, 2024
b652e16
Codestyle
rosteen Jan 19, 2024
34d2c98
Use selected_obj and get first profile viewer rather than spectrum-vi…
rosteen Jan 22, 2024
bd81c8a
Remove unused import
rosteen Jan 22, 2024
2b2bd3e
Improve test coverage
rosteen Jan 22, 2024
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
31 changes: 25 additions & 6 deletions jdaviz/configs/cubeviz/plugins/tools.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

from glue.config import viewer_tool
from glue_jupyter.bqplot.image import BqplotImageView
from glue_jupyter.bqplot.image.layer_artist import BqplotImageSubsetLayerArtist
from glue.viewers.common.tool import CheckableTool
import numpy as np
from specutils import Spectrum1D
Expand Down Expand Up @@ -127,12 +128,30 @@ def on_mouse_move(self, data):
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
and len(layer.layer.shape) == 3]
if len(cube_data) == 0:
return
cube_data = cube_data[0]
# Use the selected layer from coords_info as long as it's 3D
coords_info_dataset = self.viewer.session.application._tools['g-coords-info'].dataset.selected
if coords_info_dataset == 'auto':
cube_data = self.viewer.active_image_layer.layer
elif coords_info_dataset == 'none':
cube_data = self.viewer.layers[0].layer
else:
for layer in self.viewer.layers:
if layer.layer.label == coords_info_dataset and layer.visible:
if isinstance(layer, BqplotImageSubsetLayerArtist):
# cannot expose info for spatial subset layers
continue
cube_data = layer.layer
break
Copy link
Member

Choose a reason for hiding this comment

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

can this rely on dataset.selected_obj (which will then get caching for free)? Or do you need some layer info that isn't accessible there?

Copy link
Member

Choose a reason for hiding this comment

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

That also then might let you mouseover the uncertainty viewer but access the mouseover from the flux cube (if selected manually in the mouseover cycler) which would be super cool!

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 was just copying the logic from coords_info, using selected_obj seems to work with an extra check for glue Data vs Spectrum1D. I do worry that it will be confusing for users rather than a good feature that if they select the hover option in a viewer, they might be seeing the data from another viewer if it's selected at the top level.

Copy link
Member

Choose a reason for hiding this comment

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

hmmm true, I think most people don't even use the cycler though, so probably won't run into this except for advanced use-cases. Might be worth some user-testing or just see if anyone complains about confusion.

else:
return

if cube_data.ndim != 3:
cube_data = [layer.layer for layer in self.viewer.layers if layer.state.visible
and layer.layer.ndim == 3]
if len(cube_data) == 0:
return
cube_data = cube_data[0]

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

# Note: change this when Spectrum1D.with_spectral_axis is fixed.
if spectrum.spectral_axis.unit != self._spectrum_viewer.state.x_display_unit:
Expand Down