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

Ensure ImViz works well with arbitrary BaseCartesianData instances #1904

Closed
wants to merge 1 commit into from
Closed

Ensure ImViz works well with arbitrary BaseCartesianData instances #1904

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 9 additions & 3 deletions jdaviz/configs/imviz/plugins/viewers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from astropy.visualization import ImageNormalize, LinearStretch, PercentileInterval
from glue.core.link_helpers import LinkSame
from glue_jupyter.bqplot.image import BqplotImageView
from glue.core import Data

from jdaviz.configs.imviz import wcs_utils
from jdaviz.configs.imviz.helper import layer_is_image_data, get_top_layer_index
Expand Down Expand Up @@ -92,7 +93,9 @@ def on_mouse_or_key_event(self, data):
# 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 = active_layer.layer
self.label_mouseover.icon = self.jdaviz_app.state.layer_icons.get(active_layer.layer.label) # noqa
icon = self.jdaviz_app.state.layer_icons.get(active_layer.layer.label)
if icon is not None:
self.label_mouseover.icon = icon
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 icon doesn't work well for arbitrary BaseCartesianData instances at the moment, this needs to be investigated in glue-core but for now this ensures a graceful fallback if an icon is not found (which we should do anyway).

Copy link
Member

Choose a reason for hiding this comment

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

We should probably set to an empty string or have the icon itself smart enough to handle None in layer_viewer_icon.vue (rather than just leave it at its previous value). I'm not sure why BaseCartesianData would have any problem with the layer icon logic though, layer_icons gets populated in

jdaviz/jdaviz/app.py

Lines 384 to 386 in f36f3b2

if layer_name not in self.state.layer_icons:
self.state.layer_icons = {**self.state.layer_icons,
layer_name: alpha_index(len(self.state.layer_icons))}
which should be called by any AddDataMessage. Is it possible the AddDataMessage isn't being sent from glue for BaseCartesianData?


# Extract data coordinates - these are pixels in the reference image
x = data['domain']['x']
Expand Down Expand Up @@ -126,8 +129,11 @@ def on_mouse_or_key_event(self, data):
and x < image.shape[1] - 0.5 and y < image.shape[0] - 0.5
and hasattr(active_layer, 'attribute')):
attribute = active_layer.attribute
value = image.get_data(attribute)[int(round(y)), int(round(x))]
unit = image.get_component(attribute).units
value = image.get_data(attribute, view=[np.atleast_1d(y), np.atleast_1d(x)])[0]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This ensures that we only ever access the relevant pixel and don't accidentally load all the data then slice it.

if isinstance(image, Data):
unit = image.get_component(attribute).units
else:
unit = ''
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 existence of components with units is only guaranteed for Data subclasses not BaseCartesianData subclasses

self.label_mouseover.value = f'{value:+10.5e} {unit}'
else:
self.label_mouseover.value = ''
Expand Down