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

fix footprint defaults when changing traitlets before plugin active #2413

Merged
merged 5 commits into from
Sep 6, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ Imviz
image. [#2388]

- Footprints plugin for plotting overlays of instrument footprints or custom regions in the image
viewer. [#2341, #2377]
viewer. [#2341, #2377, #2413]

- Add a curve to stretch histograms in the Plot Options plugin representing the colormap
stretch function. [#2390]
Expand Down
33 changes: 25 additions & 8 deletions jdaviz/configs/imviz/plugins/footprints/footprints.py
Original file line number Diff line number Diff line change
Expand Up @@ -323,13 +323,23 @@ def _change_overlay(self, *args):
if self.overlay_selected not in self._overlays:
# create new entry with defaults (any defaults not provided here will be carried over
# from the previous selection based on current traitlet values)
self._overlays[self.overlay_selected] = {'color': '#c75109'}
if self.preset_selected == 'From File...':
# don't carry over the imported file/region to the next selection

# 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}

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

# 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):
# default to the center of the current zoom limits of the first selected viewer
self.center_on_viewer(self.viewer.selected[0])

fp = self._overlays[self.overlay_selected]
Expand Down Expand Up @@ -377,11 +387,12 @@ def _display_args_changed(self, msg={}):
if self.overlay_selected not in self._overlays:
# default dictionary has not been created yet
return
if not self.is_active:
return
self._ensure_first_overlay()
name = msg.get('name', '').split('_selected')[0]
if len(name):
self._overlays[self.overlay_selected][name] = msg.get('new')
if not self.is_active:
return

# update existing mark (or add/remove from viewers)
for viewer_id, viewer in self.app._viewer_store.items():
Expand Down Expand Up @@ -430,8 +441,14 @@ def overlay_regions(self):
# we need to cache these locally in order to support multiple files/regions between
# different overlay entries all selecting From File...
overlay = self._overlays.get(self.overlay_selected, {})
if 'regions' not in overlay and isinstance(self.preset.selected_obj, regions.Regions):
overlay['regions'] = self.preset.selected_obj
if ('regions' not in overlay
and isinstance(self.preset.selected_obj, (regions.Region, regions.Regions))):
regs = self.preset.selected_obj
if not isinstance(regs, regions.Regions):
# then this is a single region, but to be compatible with looping logic,
# let's just put as a single entry in a list
regs = [regs]
overlay['regions'] = regs
regs = overlay.get('regions', [])
elif self.has_pysiaf and self.preset_selected in preset_regions._instruments:
regs = preset_regions.jwst_footprint(self.preset_selected, **callable_kwargs)
Expand Down
19 changes: 16 additions & 3 deletions jdaviz/configs/imviz/tests/test_footprints.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ def test_user_api(imviz_helper, image_2d_wcs, tmp_path):
imviz_helper.load_data(ndd)

plugin = imviz_helper.plugins['Footprints']
default_color = plugin.color
default_opacity = plugin.fill_opacity
plugin.color = '#333333'
with plugin.as_active():
assert plugin._obj.is_pixel_linked is True
plugin._obj.vue_link_by_wcs()
Expand All @@ -35,9 +38,12 @@ def test_user_api(imviz_helper, image_2d_wcs, tmp_path):
viewer_marks = _get_markers_from_viewer(imviz_helper.default_viewer)
assert len(viewer_marks) == len(_all_apertures.get(preset))

# regression test for user-set traitlets (specifically color) being reset
# when the plugin is opened
assert plugin.color == '#333333'
assert viewer_marks[0].colors == ['#333333']

# change the color/opacity of the first/default overlay
default_color = plugin.color
default_opacity = plugin.fill_opacity
plugin.color = '#ffffff'
plugin.fill_opacity = 0.5

Expand Down Expand Up @@ -88,7 +94,13 @@ def test_user_api(imviz_helper, image_2d_wcs, tmp_path):
reg = plugin.overlay_regions
plugin.import_region(reg)
assert plugin.preset.selected == 'From File...'
viewer_marks = _get_markers_from_viewer(imviz_helper.default_viewer)
assert len(viewer_marks) == len(reg)
# test that importing a different region updates the marks and also that
# a single region is supported
plugin.import_region(reg[0])
viewer_marks = _get_markers_from_viewer(imviz_helper.default_viewer)
assert len(viewer_marks) == 1
# clearing the file should default to the PREVIOUS preset (last from the for-loop above)
plugin._obj.vue_file_import_cancel()
assert plugin.preset.selected == preset
Expand All @@ -109,8 +121,9 @@ def test_user_api(imviz_helper, image_2d_wcs, tmp_path):
height=3 * u.deg, width=2 * u.deg)
plugin.import_region(valid_region_sky)

tmp_invalid_path = str(tmp_path / 'invalid_path.reg')
with pytest.raises(ValueError):
plugin.import_region('./invalid_path.reg')
plugin.import_region(tmp_invalid_path)
with pytest.raises(TypeError):
plugin.import_region(5)

Expand Down