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

Imviz: astrowidgets markers API #699

Merged
merged 7 commits into from
Jul 2, 2021
Merged
Show file tree
Hide file tree
Changes from 2 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
162 changes: 156 additions & 6 deletions jdaviz/configs/imviz/helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,30 @@
from astropy.wcs import NoConvergence
from astropy.wcs.wcsapi import BaseHighLevelWCS
from echo import delay_callback
from glue.core import BaseData
from glue.core import BaseData, Data

from jdaviz.core.events import SnackbarMessage
from jdaviz.core.helpers import ConfigHelper

__all__ = ['Imviz']

ASTROPY_LT_4_3 = not minversion('astropy', '4.3')
RESERVED_MARKER_SET_NAMES = ['all']


class Imviz(ConfigHelper):
"""Imviz Helper class"""
_default_configuration = 'imviz'

def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)

# Markers
self._marktags = set()
self._default_mark_tag_name = 'default-marker-name'
# TODO: {'marker': 's', 'markersize': 10} does not work. How to fix?
self.marker = {'color': 'red', 'alpha': 1.0}

def load_data(self, data, parser_reference=None, **kwargs):
"""Load data into Imviz.

Expand Down Expand Up @@ -109,7 +119,7 @@ def center_on(self, point):
if isinstance(point, SkyCoord):
i_top = get_top_layer_index(viewer)
image = viewer.layers[i_top].layer
if hasattr(image, 'coords') and isinstance(image.coords, BaseHighLevelWCS):
if data_has_valid_wcs(image):
try:
pix = image.coords.world_to_pixel(point) # 0-indexed X, Y
except NoConvergence as e: # pragma: no cover
Expand All @@ -124,7 +134,7 @@ def center_on(self, point):

# Disallow centering outside of display.
if (pix[0] < viewer.state.x_min or pix[0] > viewer.state.x_max
or pix[1] < viewer.state.y_min or pix[1] > viewer.state.y_max):
or pix[1] < viewer.state.y_min or pix[1] > viewer.state.y_max): # pragma: no cover
self.app.hub.broadcast(SnackbarMessage(
f'{pix} is out of bounds', color="warning", sender=self.app))
return
Expand Down Expand Up @@ -163,7 +173,7 @@ def offset_to(self, dx, dy, skycoord_offset=False):
if skycoord_offset:
i_top = get_top_layer_index(viewer)
image = viewer.layers[i_top].layer
if hasattr(image, 'coords') and isinstance(image.coords, BaseHighLevelWCS):
if data_has_valid_wcs(image):
# To avoid distortion headache, assume offset is relative to
# displayed center.
x_cen = viewer.state.x_min + (width * 0.5)
Expand All @@ -185,6 +195,138 @@ def offset_to(self, dx, dy, skycoord_offset=False):
viewer.state.x_max = viewer.state.x_min + width
viewer.state.y_max = viewer.state.y_min + height

@property
def marker(self):
eteq marked this conversation as resolved.
Show resolved Hide resolved
"""Marker to use.

Marker can be set as follows; e.g.::

{'color': 'red', 'alpha': 1.0}
{'color': '#ff0000', 'alpha': 0.5}
{'color': (1, 0, 0), 'alpha': 0.75}
{'color': '0.0', 'alpha': 0.1}

Also see: https://docs.glueviz.org/en/stable/api/glue.core.visual.VisualAttributes.html
pllim marked this conversation as resolved.
Show resolved Hide resolved

"""
return self._marker_dict

@marker.setter
def marker(self, val):
# TODO: Add glupyter specific validation.
# Only set this once we have successfully created a marker
self._marker_dict = val

def _validate_marker_name(self, marker_name):
"""Raise an error if the marker_name is not allowed."""
if marker_name in RESERVED_MARKER_SET_NAMES:
raise ValueError(
f"The marker name {marker_name} is not allowed. Any name is "
f"allowed except these: {', '.join(RESERVED_MARKER_SET_NAMES)}")

def add_markers(self, table, x_colname='x', y_colname='y',
skycoord_colname='coord', use_skycoord=False,
marker_name=None):
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should get an extra keyword marker_style, which in the current implementation would basically just be:

if marker_style is not None:
    self.marker = marker_style

at the top of this function. See my comment above about the marker property for the motivation.

Copy link
Contributor

Choose a reason for hiding this comment

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

Out-of-band, @pllim talked me out of this idea. Instead, a clear note in the docstring saying something like ""wondering how to set the style? see self.markers" is the way to go so as not to deviate too far from astrowidgets.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we have agreed to pursue this over at astropy/astrowidgets#141 first and open follow-up PR here if necessary.

"""Creates markers w.r.t. the reference image at given points
in the table.

Parameters
----------
table : `~astropy.table.Table`
Table containing marker locations.

x_colname, y_colname : str
Column names for X and Y.
Coordinates must be 0-indexed.

skycoord_colname : str
Column name with `~astropy.coordinates.SkyCoord` objects.

use_skycoord : bool
If `True`, use ``skycoord_colname`` to mark.
Otherwise, use ``x_colname`` and ``y_colname``.

marker_name : str, optional
Name to assign the markers in the table. Providing a name
allows markers to be removed by name at a later time.

Raises
------
AttributeError
Sky coordinates are given but reference image does not have a valid WCS.

ValueError
Invalid marker name.

"""
if marker_name is None:
marker_name = self._default_mark_tag_name

self._validate_marker_name(marker_name)
viewer = self.app.get_viewer("viewer-1")
jglue = self.app.session.application

# TODO: How to link to invidual images separately? add_link in loop does not work.
pllim marked this conversation as resolved.
Show resolved Hide resolved
# Link markers to reference image data.
image = viewer.state.reference_data

# TODO: Is Glue smart enough to no-op if link already there?
if use_skycoord:
if not data_has_valid_wcs(image):
raise AttributeError(f'{image.label} does not have a valid WCS')
sky = table[skycoord_colname]
t_glue = Data(marker_name, ra=sky.ra.deg, dec=sky.dec.deg)
jglue.data_collection[marker_name] = t_glue
jglue.add_link(t_glue, 'ra', image, 'Right Ascension')
jglue.add_link(t_glue, 'dec', image, 'Declination')
else:
t_glue = Data(marker_name, **table[x_colname, y_colname])
jglue.data_collection[marker_name] = t_glue
jglue.add_link(t_glue, x_colname, image, image.pixel_component_ids[1].label)
jglue.add_link(t_glue, y_colname, image, image.pixel_component_ids[0].label)

try:
viewer.add_data(t_glue, **self.marker)
except Exception as e: # pragma: no cover
self.app.hub.broadcast(SnackbarMessage(
f"Failed to add markers '{marker_name}': {repr(e)}",
color="warning", sender=self.app))
else:
self._marktags.add(marker_name)

def remove_markers(self, marker_name=None):
"""Remove some but not all of the markers by name used when
adding the markers.

Parameters
----------
marker_name : str
Name used when the markers were added.
If not given, will delete markers added under default name.

"""
if marker_name is None:
marker_name = self._default_mark_tag_name

try:
i = self.app.data_collection.labels.index(marker_name)
except ValueError as e: # pragma: no cover
self.app.hub.broadcast(SnackbarMessage(
f"Failed to remove markers '{marker_name}': {repr(e)}",
color="warning", sender=self.app))
return

data = self.app.data_collection[i]
self.app.data_collection.remove(data)
self._marktags.remove(marker_name)

def reset_markers(self):
"""Delete all markers."""
# Grab the entire list of marker names before iterating
# otherwise what we are iterating over changes.
for marker_name in list(self._marktags):
self.remove_markers(marker_name=marker_name)


def split_filename_with_fits_ext(filename):
"""Split a ``filename[ext]`` input into filename and FITS extension.
Expand Down Expand Up @@ -233,10 +375,18 @@ def split_filename_with_fits_ext(filename):
return filepath, ext, data_label


def data_has_valid_wcs(data):
return hasattr(data, 'coords') and isinstance(data.coords, BaseHighLevelWCS)


def layer_is_image_data(layer):
return isinstance(layer, BaseData) and layer.ndim == 2


def get_top_layer_index(viewer):
"""Get index of the top visible layer in Imviz.
"""Get index of the top visible image layer in Imviz.
This is because when blinked, first layer might not be top visible layer.

"""
return [i for i, lyr in enumerate(viewer.layers)
if lyr.visible and isinstance(lyr.layer, BaseData)][-1]
if lyr.visible and layer_is_image_data(lyr.layer)][-1]
10 changes: 4 additions & 6 deletions jdaviz/configs/imviz/plugins/viewers.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
import numpy as np

from astropy.wcs.wcsapi import BaseHighLevelWCS

from glue.core import BaseData
from glue_jupyter.bqplot.image import BqplotImageView

from jdaviz.configs.imviz.helper import data_has_valid_wcs, layer_is_image_data
from jdaviz.core.events import SnackbarMessage
from jdaviz.core.registries import viewer_registry

Expand Down Expand Up @@ -74,7 +72,7 @@ def on_mouse_or_key_event(self, data):

self.label_mouseover.pixel = (fmt.format(x, y))

if hasattr(image, 'coords') and isinstance(image.coords, BaseHighLevelWCS):
if data_has_valid_wcs(image):
# Convert these to a SkyCoord via WCS - note that for other datasets
# we aren't actually guaranteed to get a SkyCoord out, just for images
# with valid celestial WCS
Expand Down Expand Up @@ -116,7 +114,7 @@ def blink_once(self):

# Exclude Subsets: They are global
valid = [ilayer for ilayer, layer in enumerate(self.state.layers)
if isinstance(layer.layer, BaseData)]
if layer_is_image_data(layer.layer)]
n_layers = len(valid)

if n_layers == 1:
Expand Down Expand Up @@ -156,4 +154,4 @@ def data(self, cls=None):
return [layer_state.layer # .get_object(cls=cls or self.default_class)
for layer_state in self.state.layers
if hasattr(layer_state, 'layer') and
isinstance(layer_state.layer, BaseData)]
layer_is_image_data(layer_state.layer)]
Loading