ensure footprints display when added before linking (spacetelescope#2797

)

* when footprint added before linking, make sure they display when link is updated

* add change log

* add ability to restore overlay selection

* call _preset_args_changed instead of changing selection

* internalize bug fix and add test coverage

* add comment explaining link change loop

* change fp to footprint in comments

* address case where default overlay is created in init

* change np.ones() to np.ones(()) for testing

* trigger CI

---------

Co-authored-by: Kyle Conroy <kyleconroy@gmail.com>

CHANGES.rst

@@ -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]
 

jdaviz/configs/imviz/plugins/footprints/footprints.py

@@ -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
@@ -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
@@ -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:
@@ -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
 
@@ -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
@@ -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],

jdaviz/configs/imviz/tests/test_footprints.py

@@ -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")
 
@@ -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