diff --git a/CHANGES.rst b/CHANGES.rst index 2e60768a29..722f4eed63 100644 --- a/CHANGES.rst +++ b/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] diff --git a/jdaviz/configs/imviz/plugins/footprints/footprints.py b/jdaviz/configs/imviz/plugins/footprints/footprints.py index cbc23995a9..fe5a9cf35d 100644 --- a/jdaviz/configs/imviz/plugins/footprints/footprints.py +++ b/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,7 +321,7 @@ 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 @@ -322,7 +329,13 @@ def _change_overlay(self, *args): # 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) @@ -330,21 +343,21 @@ def _change_overlay(self, *args): # 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], diff --git a/jdaviz/configs/imviz/tests/test_footprints.py b/jdaviz/configs/imviz/tests/test_footprints.py index c9ef88051d..c30094e08e 100644 --- a/jdaviz/configs/imviz/tests/test_footprints.py +++ b/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