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

ensure footprints display when added before linking #2797

Merged
merged 10 commits into from
Apr 17, 2024
4 changes: 2 additions & 2 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ Cubeviz

Imviz
^^^^^

- Fix a bug where footprints did not overlay when created via API. [#2790]
- Fix bugs where API created footprints did not overlay and only last
footprint displayed if added before linking. [#2790, #2797]

- Improved behavior when orientations are created or selected without having data loaded in the viewer. [#2789]

Expand Down
46 changes: 30 additions & 16 deletions jdaviz/configs/imviz/plugins/footprints/footprints.py
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,13 @@ def _on_link_type_updated(self, msg=None):
# toggle visibility as necessary
self._on_is_active_changed()

# When footprint(s) are added via API before WCS link. Overlays visibility & is_active
# can be True, but only last footprint will display. This ensures all footprints display
if not self.is_pixel_linked:
for choice in self.overlay.choices:
# trigger the update without actually changing the user-selection
self._change_overlay(overlay_selected=choice, center_the_overlay=False)

def vue_link_by_wcs(self, *args):
# call other plugin so that other options (wcs_use_affine, wcs_use_fallback)
# are retained. Remove this method if support for plotting footprints
Expand Down Expand Up @@ -314,37 +321,43 @@ def vue_center_on_viewer(self, viewer_ref):
self.center_on_viewer(viewer_ref)

@observe('overlay_selected')
def _change_overlay(self, *args):
def _change_overlay(self, *args, overlay_selected=None, center_the_overlay=True):
if not hasattr(self, 'overlay'): # pragma: nocover
# plugin/traitlet startup
return
if self.overlay_selected == '':
# no overlay selected (this can happen when removing all overlays)
return

if self.overlay_selected not in self._overlays:
overlay_selected = overlay_selected if overlay_selected is not None else self.overlay_selected # noqa

if overlay_selected not in self._overlays:
# _on_link_type_updated() called in init but don't want to create
# the default overlay yet, since center_on_viewer() will cause a traceback
if not center_the_overlay:
return
# create new entry with defaults (any defaults not provided here will be carried over
# from the previous selection based on current traitlet values)

# if this is the first overlay, there is a chance the traitlet for color was already set
# to something other than the default, so we want to use that, otherwise for successive
# new overlays, we want to ignore the traitlet and default back to "active" orange.
default_color = '#c75109' if len(self._overlays) else self.color
self._overlays[self.overlay_selected] = {'color': default_color}
self._overlays[overlay_selected] = {'color': default_color}

# similarly, if the user called any import APIs before opening the plugin, we want to
# respect that, but when creating successive overlays, any selection from file/region
# should be cleared for the next selection
if self.preset_selected == 'From File...' and len(self._overlays) > 1:
self._overlays[self.overlay_selected]['from_file'] = ''
self._overlays[self.overlay_selected]['preset'] = self.preset.choices[0]
self._overlays[overlay_selected]['from_file'] = ''
self._overlays[overlay_selected]['preset'] = self.preset.choices[0]

# for the first overlay only, default the position to be centered on the current
# zoom limits of the first selected viewer
if len(self._overlays) == 1 and len(self.viewer.selected):
self.center_on_viewer(self.viewer.selected[0])

fp = self._overlays[self.overlay_selected]
fp = self._overlays[overlay_selected]

# we'll temporarily disable updating the overlays so that we can set all
# traitlets simultaneously (and since we're only updating traitlets to a previously-set
Expand All @@ -365,7 +378,7 @@ def _change_overlay(self, *args):
# dict above.
fp[key] = getattr(self, attr)
self._ignore_traitlet_change = False
self._preset_args_changed()
self._preset_args_changed(overlay_selected=overlay_selected)

def _mark_visible(self, viewer_id, overlay=None):
if not self.is_active:
Expand Down Expand Up @@ -461,24 +474,25 @@ def overlay_regions(self):
return regs

@observe('preset_selected', 'from_file', 'ra', 'dec', 'pa', 'v2_offset', 'v3_offset')
def _preset_args_changed(self, msg={}):
def _preset_args_changed(self, msg={}, overlay_selected=None):
if self._ignore_traitlet_change:
return
if not self.overlay_selected:
overlay_selected = overlay_selected if overlay_selected is not None else self.overlay_selected # noqa
if not overlay_selected:
return

name = msg.get('name', '').split('_selected')[0]

if self.overlay_selected not in self._overlays:
if overlay_selected not in self._overlays:
# default dictionary has not been created yet
return

if len(name):
self._overlays[self.overlay_selected][name] = msg.get('new')
if name == 'from_file' and 'regions' in self._overlays[self.overlay_selected]:
self._overlays[overlay_selected][name] = msg.get('new')
if name == 'from_file' and 'regions' in self._overlays[overlay_selected]:
# then the file may have been changed from the API, so we need to clear the cache
# the cache will then be re-populated on the call to self.overlay_regions below
del self._overlays[self.overlay_selected]['regions']
del self._overlays[overlay_selected]['regions']

regs = self.overlay_regions

Expand All @@ -490,12 +504,12 @@ def _preset_args_changed(self, msg={}):
wcs = getattr(viewer.state.reference_data, 'coords', None)
if wcs is None:
continue
existing_overlays = self._get_marks(viewer, self.overlay_selected)
existing_overlays = self._get_marks(viewer, overlay_selected)
update_existing = len(existing_overlays) == len(regs)
if not update_existing and len(existing_overlays):
# clear any existing marks (length has changed, perhaps a new preset)
viewer.figure.marks = [m for m in viewer.figure.marks
if getattr(m, 'overlay', None) != self.overlay_selected]
if getattr(m, 'overlay', None) != overlay_selected]

# the following logic is adapted from
# https://github.com/spacetelescope/jwst_novt/blob/main/jwst_novt/interact/display.py
Expand Down Expand Up @@ -536,7 +550,7 @@ def _preset_args_changed(self, msg={}):
else:
mark = FootprintOverlay(
viewer,
self.overlay_selected,
overlay_selected,
x=x_coords,
y=y_coords,
colors=[self.color],
Expand Down
42 changes: 42 additions & 0 deletions jdaviz/configs/imviz/tests/test_footprints.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

from jdaviz.core.marks import FootprintOverlay
from jdaviz.configs.imviz.plugins.footprints.preset_regions import _all_apertures
from astropy.wcs import WCS

pytest.importorskip("pysiaf")

Expand Down Expand Up @@ -142,3 +143,44 @@ def test_user_api(imviz_helper, image_2d_wcs, tmp_path):
assert plugin._obj.is_active is False
viewer_marks = _get_markers_from_viewer(imviz_helper.default_viewer)
assert viewer_marks[0].visible is False


def test_api_after_linking(imviz_helper):
# custom image to enable visual test in a notebook
arr = np.ones((300, 300))
image_2d_wcs = WCS({'CTYPE1': 'RA---TAN', 'CUNIT1': 'deg', 'CDELT1': -0.0002777777778,
'CRPIX1': 1, 'CRVAL1': 337.5202808,
'CTYPE2': 'DEC--TAN', 'CUNIT2': 'deg', 'CDELT2': 0.0002777777778,
'CRPIX2': 1, 'CRVAL2': -20.833333059999998})

viewer = imviz_helper.app.get_viewer_by_id('imviz-0')

ndd = NDData(arr, wcs=image_2d_wcs)
imviz_helper.load_data(ndd)
imviz_helper.load_data(ndd)

plugin = imviz_helper.plugins['Footprints']
with plugin.as_active():

pointing = {'name': ['pt'], 'ra': [337.51], 'dec': [-20.77], 'pa': [0]}
plugin.color = 'green'
plugin.add_overlay(pointing['name'][0])
plugin.ra = pointing['ra'][0]
plugin.dec = pointing['dec'][0]
plugin.pa = pointing['pa'][0]

# when pixel linked are any marks displayed
viewer_marks = _get_markers_from_viewer(viewer)
no_marks_displayed = all(not mark.visible for mark in viewer_marks)
assert no_marks_displayed is True

# link by wcs and retest
imviz_helper.link_data(link_type='wcs')

viewer_marks = _get_markers_from_viewer(viewer)
# distinguish default from custom overlay with color
marks_displayed = any(mark.visible and mark.colors[0] == 'green'
for mark in viewer_marks)

# when wcs linked are any marks displayed
assert marks_displayed is True
Loading