From fd5026fa9497f6906fe72995123c972a28866f80 Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Wed, 31 Jan 2024 09:46:47 -0500 Subject: [PATCH 1/7] allow composite/annulus in dropdown, but expose as not supported * currently via property, but probably want this hooked into a traitlet so that the plugin can observe that and show message, etc --- jdaviz/core/template_mixin.py | 39 ++++++++++++++++++++++++++++------- 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/jdaviz/core/template_mixin.py b/jdaviz/core/template_mixin.py index 747c59d2c4..24b9f328a1 100644 --- a/jdaviz/core/template_mixin.py +++ b/jdaviz/core/template_mixin.py @@ -2032,11 +2032,12 @@ def __init__(self, plugin, items, selected, scale_factor, multiselect=None, items=items, selected=selected, multiselect=multiselect, - filters=['is_spatial', 'is_not_composite', 'is_not_annulus'], + filters=['is_spatial'], dataset=dataset, viewers=viewers, default_text=default_text) + self._selected_supports_aperture = False self.add_traitlets(scale_factor=scale_factor) self.add_observe('is_active', self._plugin_active_changed) @@ -2064,6 +2065,11 @@ def _set_mark_visiblities(self, visible): def _plugin_active_changed(self, *args): self._set_mark_visiblities(self.plugin.is_active) + @property + def selected_supports_aperture(self): + # TODO: make this a dict traitlet to return message back to UI? + return self._selected_supports_aperture + @property def image_viewers(self): return [viewer for viewer in self.app._viewer_store.values() @@ -2099,9 +2105,19 @@ def _get_mark_coords(self, viewer): if not len(self.selected) or not len(self.dataset.selected): return [], [] if self.selected in self._manual_options: + self._selected_supports_aperture = False + return [], [] + + multiselect = getattr(self, 'multiselect', False) + + # if any of the selected entries are composite, then _get_spatial_region + # (or selected_spatial_region) will fail. + objs = self.selected_obj if multiselect else [self.selected_obj] + if np.any([len(obj) > 1 for obj in objs]): + self._selected_supports_aperture = False return [], [] - if getattr(self, 'multiselect', False): + if multiselect: # assume first dataset (for retrieving the region object) # but iterate over all subsets spatial_regions = [self._get_spatial_region(dataset=self.dataset.selected[0], subset=subset) # noqa @@ -2131,7 +2147,7 @@ def _get_mark_coords(self, viewer): elif hasattr(roi, 'radius_x'): roi.radius_x *= self.scale_factor roi.radius_y *= self.scale_factor - elif hasattr(roi, 'center'): + elif hasattr(roi, 'center') and hasattr(roi, 'xmin') and hasattr(roi, 'xmax'): center = roi.center() half_width = abs(roi.xmax - roi.xmin) * 0.5 * self.scale_factor half_height = abs(roi.ymax - roi.ymin) * 0.5 * self.scale_factor @@ -2140,14 +2156,21 @@ def _get_mark_coords(self, viewer): roi.ymin = center[1] - half_height roi.ymax = center[1] + half_height else: # pragma: no cover - raise NotImplementedError + # known unsupported shapes: annulus + self._selected_supports_aperture = False + return [], [] - x, y = roi.to_polygon() + if hasattr(roi, 'to_polygon'): + x, y = roi.to_polygon() - # concatenate with nan between to avoid line connecting separate subsets - x_coords = np.concatenate((x_coords, np.array([np.nan]), x)) - y_coords = np.concatenate((y_coords, np.array([np.nan]), y)) + # concatenate with nan between to avoid line connecting separate subsets + x_coords = np.concatenate((x_coords, np.array([np.nan]), x)) + y_coords = np.concatenate((y_coords, np.array([np.nan]), y)) + else: + self._selected_supports_aperture = False + return [], [] + self._selected_supports_aperture = True return x_coords, y_coords def _update_mark_coords(self, *args): From 1d36b8d5c9bc895ca1298c94f7dcdf7dee631152 Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Wed, 31 Jan 2024 14:11:39 -0500 Subject: [PATCH 2/7] move validity to dict traitlet with messages exposed --- .../spectral_extraction.py | 4 +- .../aper_phot_simple/aper_phot_simple.py | 5 +++ jdaviz/core/template_mixin.py | 45 +++++++++++++------ 3 files changed, 39 insertions(+), 15 deletions(-) diff --git a/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.py b/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.py index 1c67e03434..26ee9b83ae 100644 --- a/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.py +++ b/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.py @@ -9,7 +9,7 @@ from astropy.nddata import ( NDDataArray, StdDevUncertainty, NDUncertainty ) -from traitlets import Any, Bool, Float, List, Unicode, observe +from traitlets import Any, Bool, Dict, Float, List, Unicode, observe from jdaviz.core.custom_traitlets import FloatHandleEmpty from jdaviz.core.events import SnackbarMessage, SliceWavelengthUpdatedMessage @@ -65,6 +65,7 @@ class SpectralExtraction(PluginTemplateMixin, ApertureSubsetSelectMixin, bg_items = List([]).tag(sync=True) bg_selected = Any('').tag(sync=True) + bg_selected_validity = Dict().tag(sync=True) bg_scale_factor = Float(1).tag(sync=True) bg_wavelength_dependent = Bool(False).tag(sync=True) @@ -100,6 +101,7 @@ def __init__(self, *args, **kwargs): self.background = ApertureSubsetSelect(self, 'bg_items', 'bg_selected', + 'bg_selected_validity', 'bg_scale_factor', dataset='dataset', multiselect=None, diff --git a/jdaviz/configs/imviz/plugins/aper_phot_simple/aper_phot_simple.py b/jdaviz/configs/imviz/plugins/aper_phot_simple/aper_phot_simple.py index c21667cd05..eb21912505 100644 --- a/jdaviz/configs/imviz/plugins/aper_phot_simple/aper_phot_simple.py +++ b/jdaviz/configs/imviz/plugins/aper_phot_simple/aper_phot_simple.py @@ -235,6 +235,8 @@ def _dataset_selected_changed(self, event={}): # but we still need to update the flux-scaling warning self._multiselect_flux_scaling_warning() return + if not self.aperture_selected_validity.get('is_aperture'): + return try: defaults = self._get_defaults_from_metadata() @@ -286,6 +288,9 @@ def _aperture_selected_changed(self, event={}): if self.multiselect: self._background_selected_changed() return + # NOTE: aperture_selected can be triggered here before aperture_selected_validity is updated + # so we'll still allow the snackbar to be raised as a second warning to the user and to + # avoid acting on outdated information # NOTE: aperture area is only used to determine if a warning should be shown in the UI # and so does not need to be calculated within user API calls that don't act on traitlets diff --git a/jdaviz/core/template_mixin.py b/jdaviz/core/template_mixin.py index 24b9f328a1..ffd18d157b 100644 --- a/jdaviz/core/template_mixin.py +++ b/jdaviz/core/template_mixin.py @@ -31,7 +31,7 @@ from regions import PixelRegion from specutils import Spectrum1D from specutils.manipulation import extract_region -from traitlets import Any, Bool, Float, HasTraits, List, Unicode, observe +from traitlets import Any, Bool, Dict, Float, HasTraits, List, Unicode, observe from ipywidgets import widget_serialization from ipypopout import PopoutButton @@ -1972,6 +1972,7 @@ class ApertureSubsetSelect(SubsetSelect): * ``items`` (list of dicts with keys: label, color, type) * ``selected`` (string) + * ``selected_validity`` (dict) Properties (in the object only): @@ -2005,7 +2006,8 @@ class ApertureSubsetSelect(SubsetSelect): /> """ - def __init__(self, plugin, items, selected, scale_factor, multiselect=None, + def __init__(self, plugin, items, selected, selected_validity, + scale_factor, multiselect=None, dataset=None, viewers=None, default_text=None): """ Parameters @@ -2016,6 +2018,8 @@ def __init__(self, plugin, items, selected, scale_factor, multiselect=None, the name of the items traitlet defined in ``plugin`` selected : str the name of the selected traitlet defined in ``plugin`` + selected_validity: str + the name of the selected validity dict traitlet defined in ``plugin`` scale_factor : str the name of the traitlet defining the radius factor for the drawn aperture multiselect : str @@ -2037,8 +2041,8 @@ def __init__(self, plugin, items, selected, scale_factor, multiselect=None, viewers=viewers, default_text=default_text) - self._selected_supports_aperture = False - self.add_traitlets(scale_factor=scale_factor) + self.add_traitlets(selected_validity=selected_validity, + scale_factor=scale_factor) self.add_observe('is_active', self._plugin_active_changed) self.add_observe(selected, self._update_mark_coords) @@ -2065,11 +2069,6 @@ def _set_mark_visiblities(self, visible): def _plugin_active_changed(self, *args): self._set_mark_visiblities(self.plugin.is_active) - @property - def selected_supports_aperture(self): - # TODO: make this a dict traitlet to return message back to UI? - return self._selected_supports_aperture - @property def image_viewers(self): return [viewer for viewer in self.app._viewer_store.values() @@ -2103,9 +2102,12 @@ def marks(self): def _get_mark_coords(self, viewer): if not len(self.selected) or not len(self.dataset.selected): + self.selected_validity = {'is_aperture': False, + 'aperture_message': 'no subset selected'} return [], [] if self.selected in self._manual_options: - self._selected_supports_aperture = False + self.selected_validity = {'is_aperture': False, + 'aperture_message': 'no subset selected'} return [], [] multiselect = getattr(self, 'multiselect', False) @@ -2114,7 +2116,9 @@ def _get_mark_coords(self, viewer): # (or selected_spatial_region) will fail. objs = self.selected_obj if multiselect else [self.selected_obj] if np.any([len(obj) > 1 for obj in objs]): - self._selected_supports_aperture = False + self.selected_validity = {'is_aperture': False, + 'aperture_message': 'composite subsets are not supported', + 'is_composite': True} return [], [] if multiselect: @@ -2136,12 +2140,16 @@ def _get_mark_coords(self, viewer): else: wcs = getattr(viewer.state.reference_data, 'coords', None) if wcs is None: + self.selected_validity = {'is_aperture': False, + 'aperture_message': 'invalid wcs'} return [], [] pixel_region = spatial_region.to_pixel(wcs) roi = regions2roi(pixel_region) # NOTE: this assumes that we'll apply the same radius factor to all subsets (all will # be defined at the same slice for cones in cubes) +# if self.scale_factor == 1.0: # this would catch annulus, which might cause confusion +# pass if hasattr(roi, 'radius'): roi.radius *= self.scale_factor elif hasattr(roi, 'radius_x'): @@ -2155,9 +2163,15 @@ def _get_mark_coords(self, viewer): roi.xmax = center[0] + half_width roi.ymin = center[1] - half_height roi.ymax = center[1] + half_height + elif isinstance(roi, CircularAnnulusROI): + self.selected_validity = {'is_aperture': False, + 'aperture_message': 'annulus is not a supported aperture'} + return [], [] else: # pragma: no cover # known unsupported shapes: annulus - self._selected_supports_aperture = False + # TODO: specific case for annulus + self.selected_validity = {'is_aperture': False, + 'aperture_message': 'shape does not support scale factor'} return [], [] if hasattr(roi, 'to_polygon'): @@ -2167,10 +2181,11 @@ def _get_mark_coords(self, viewer): x_coords = np.concatenate((x_coords, np.array([np.nan]), x)) y_coords = np.concatenate((y_coords, np.array([np.nan]), y)) else: - self._selected_supports_aperture = False + self.selected_validity = {'is_aperture': False, + 'aperture_message': 'could not convert roi to polygon'} return [], [] - self._selected_supports_aperture = True + self.selected_validity = {'is_aperture': True} return x_coords, y_coords def _update_mark_coords(self, *args): @@ -2207,6 +2222,7 @@ class ApertureSubsetSelectMixin(VuetifyTemplate, HubListener): """ aperture_items = List([]).tag(sync=True) aperture_selected = Any('').tag(sync=True) + aperture_selected_validity = Dict().tag(sync=True) aperture_scale_factor = Float(1).tag(sync=True) def __init__(self, *args, **kwargs): @@ -2214,6 +2230,7 @@ def __init__(self, *args, **kwargs): self.aperture = ApertureSubsetSelect(self, 'aperture_items', 'aperture_selected', + 'aperture_selected_validity', 'aperture_scale_factor', dataset='dataset' if hasattr(self, 'dataset') else None, # noqa multiselect='multiselect' if hasattr(self, 'multiselect') else None) # noqa From 7f62c9a28d458486d7b20339c84d82bb3df049b8 Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Wed, 31 Jan 2024 14:12:01 -0500 Subject: [PATCH 3/7] implement aperture rules for photometry and spectral extraction plugins --- .../spectral_extraction.vue | 37 +++++++++++++++++-- .../aper_phot_simple/aper_phot_simple.vue | 8 +++- 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.vue b/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.vue index e80426f1b4..d807663f3d 100644 --- a/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.vue +++ b/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.vue @@ -19,7 +19,13 @@ hint="Select a spatial region to extract its spectrum." /> -
+ + + {{aperture_selected}} does not support wavelength dependence (cone support): {{aperture_selected_validity.aperture_message}}. + + + +
-
+ + + {{bg_selected}} does not support wavelength dependence (cone support): {{bg_selected_validity.aperture_message}}. + + + +
Extract -
+ + + + Aperture: {{aperture_selected}} does not support subpixel: {{aperture_selected_validity.aperture_message}}. + + + + + Background: {{bg_selected}} does not support subpixel: {{bg_selected_validity.aperture_message}}. + + + + +
diff --git a/jdaviz/configs/imviz/plugins/aper_phot_simple/aper_phot_simple.vue b/jdaviz/configs/imviz/plugins/aper_phot_simple/aper_phot_simple.vue index 7d1185a63f..c38e063baf 100644 --- a/jdaviz/configs/imviz/plugins/aper_phot_simple/aper_phot_simple.vue +++ b/jdaviz/configs/imviz/plugins/aper_phot_simple/aper_phot_simple.vue @@ -56,6 +56,12 @@ hint="Select aperture region for photometry (cannot be an annulus or composite subset)." /> + + + {{aperture_selected}} is not a valid aperture: {{aperture_selected_validity.aperture_message}}. + + +
Calculate From 2f9b90fe7e1e69f278c4ed9a3f57436aec99da60 Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Wed, 31 Jan 2024 14:20:44 -0500 Subject: [PATCH 4/7] changelog entry --- CHANGES.rst | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index 223531c779..d6c556e400 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -11,7 +11,7 @@ New Features - Opacity for spatial subsets is now adjustable from within Plot Options. [#2663] -- Live-preview of aperture selection in plugins. [#2664] +- Live-preview of aperture selection in plugins. [#2664, #2684] Cubeviz ^^^^^^^ @@ -91,6 +91,8 @@ Cubeviz Imviz ^^^^^ +- Apertures that are selected and later modified to be invalid properly show a warning. [#2684] + Mosviz ^^^^^^ From b7cc3bddda808ae37742ba050d6502913ae6df3b Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Thu, 1 Feb 2024 09:39:43 -0500 Subject: [PATCH 5/7] update existing test --- .../plugins/aper_phot_simple/aper_phot_simple.py | 2 ++ jdaviz/configs/imviz/tests/test_simple_aper_phot.py | 11 +++++++++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/jdaviz/configs/imviz/plugins/aper_phot_simple/aper_phot_simple.py b/jdaviz/configs/imviz/plugins/aper_phot_simple/aper_phot_simple.py index eb21912505..f9d18844e0 100644 --- a/jdaviz/configs/imviz/plugins/aper_phot_simple/aper_phot_simple.py +++ b/jdaviz/configs/imviz/plugins/aper_phot_simple/aper_phot_simple.py @@ -411,6 +411,8 @@ def calculate_photometry(self, dataset=None, aperture=None, background=None, dataset=dataset if dataset is not None else self.dataset.selected) # noqa else: # use the pre-cached value + if not self.aperture.selected_validity.get('is_aperture'): + raise ValueError(f"Selected aperture is not valid: {self.aperture.selected_validity.get('aperture_message')}") # noqa reg = self.aperture.selected_spatial_region # Reset last fitted model diff --git a/jdaviz/configs/imviz/tests/test_simple_aper_phot.py b/jdaviz/configs/imviz/tests/test_simple_aper_phot.py index 1dd75ebdfc..598ad733db 100644 --- a/jdaviz/configs/imviz/tests/test_simple_aper_phot.py +++ b/jdaviz/configs/imviz/tests/test_simple_aper_phot.py @@ -363,11 +363,18 @@ def test_annulus_background(imviz_helper): PixCoord(x=20.5, y=37.5), inner_radius=20.5, outer_radius=30.5) imviz_helper.load_regions([ellipse_1, annulus_2]) - # Subset 4 (annulus) should be available for the background but not the aperture - assert 'Subset 4' not in phot_plugin.aperture.choices + # Subset 4 (annulus) should be available in both sets of choices, but invalid for selection as + # aperture + assert 'Subset 4' in phot_plugin.aperture.choices assert 'Subset 4' in phot_plugin.background.choices + phot_plugin.aperture_selected = 'Subset 4' + assert not phot_plugin.aperture.selected_validity.get('is_aperture', True) + with pytest.raises(ValueError, match="Selected aperture is not valid"): + phot_plugin.calculate_photometry() + phot_plugin.aperture_selected = 'Subset 3' + assert phot_plugin.aperture.selected_validity.get('is_aperture', False) phot_plugin.background_selected = 'Subset 4' # Check new annulus for four_gaussians From cc7ad75a2c9a0822c0bb33e1d777870db0bdfebc Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Thu, 1 Feb 2024 13:09:26 -0500 Subject: [PATCH 6/7] handle batch photometry case --- .../aper_phot_simple/aper_phot_simple.py | 10 ++- jdaviz/core/template_mixin.py | 78 +++++++++++-------- 2 files changed, 52 insertions(+), 36 deletions(-) diff --git a/jdaviz/configs/imviz/plugins/aper_phot_simple/aper_phot_simple.py b/jdaviz/configs/imviz/plugins/aper_phot_simple/aper_phot_simple.py index f9d18844e0..7dd1831914 100644 --- a/jdaviz/configs/imviz/plugins/aper_phot_simple/aper_phot_simple.py +++ b/jdaviz/configs/imviz/plugins/aper_phot_simple/aper_phot_simple.py @@ -404,11 +404,17 @@ def calculate_photometry(self, dataset=None, aperture=None, background=None, # we can use the pre-cached value data = self.dataset.selected_dc_item - if aperture is not None and aperture not in self.aperture.choices: - raise ValueError(f"aperture must be one of {self.aperture.choices}") + if aperture is not None: + if aperture not in self.aperture.choices: + raise ValueError(f"aperture must be one of {self.aperture.choices}") + if aperture is not None or dataset is not None: reg = self.aperture._get_spatial_region(subset=aperture if aperture is not None else self.aperture.selected, # noqa dataset=dataset if dataset is not None else self.dataset.selected) # noqa + # determine if a valid aperture (since selected_validity only applies to selected entry) + _, _, validity = self.aperture._get_mark_coords_and_validate(selected=aperture) + if not validity.get('is_aperture'): + raise ValueError(f"Selected aperture {aperture} is not valid: {validity.get('aperture_message')}") # noqa else: # use the pre-cached value if not self.aperture.selected_validity.get('is_aperture'): diff --git a/jdaviz/core/template_mixin.py b/jdaviz/core/template_mixin.py index ffd18d157b..545115c96f 100644 --- a/jdaviz/core/template_mixin.py +++ b/jdaviz/core/template_mixin.py @@ -2031,7 +2031,7 @@ def __init__(self, plugin, items, selected, selected_validity, the reference names or ids of the viewer to extract the subregion. If not provided or None, will loop through all references. """ - # NOTE: is_not_composite is assumed in _get_mark_coords + # NOTE: is_not_composite is assumed in _get_mark_coords_and_validate super().__init__(plugin, items=items, selected=selected, @@ -2086,7 +2086,7 @@ def marks(self): all_aperture_marks += matches continue - x_coords, y_coords = self._get_mark_coords(viewer) + x_coords, y_coords, self.selected_validity = self._get_mark_coords_and_validate(viewer) mark = ApertureMark( viewer, @@ -2100,32 +2100,42 @@ def marks(self): viewer.figure.marks = viewer.figure.marks + [mark] return all_aperture_marks - def _get_mark_coords(self, viewer): - if not len(self.selected) or not len(self.dataset.selected): - self.selected_validity = {'is_aperture': False, - 'aperture_message': 'no subset selected'} - return [], [] - if self.selected in self._manual_options: - self.selected_validity = {'is_aperture': False, - 'aperture_message': 'no subset selected'} - return [], [] - + def _get_mark_coords_and_validate(self, viewer=None, selected=None): multiselect = getattr(self, 'multiselect', False) + if viewer is None: + viewer = self.app._jdaviz_helper.default_viewer._obj + if selected is None: + selected = self.selected + objs = self.selected_obj if multiselect else [self.selected_obj] + else: + objs = self._get_selected_obj(selected) + if isinstance(selected, str): + selected = [selected] + objs = [objs] + + if not len(selected) or not len(self.dataset.selected): + validity = {'is_aperture': False, + 'aperture_message': 'no subset selected'} + return [], [], validity + if selected in self._manual_options: + validity = {'is_aperture': False, + 'aperture_message': 'no subset selected'} + return [], [], validity + # if any of the selected entries are composite, then _get_spatial_region # (or selected_spatial_region) will fail. - objs = self.selected_obj if multiselect else [self.selected_obj] if np.any([len(obj) > 1 for obj in objs]): - self.selected_validity = {'is_aperture': False, - 'aperture_message': 'composite subsets are not supported', - 'is_composite': True} - return [], [] + validity = {'is_aperture': False, + 'aperture_message': 'composite subsets are not supported', + 'is_composite': True} + return [], [], validity - if multiselect: + if multiselect or selected != self.selected: # assume first dataset (for retrieving the region object) # but iterate over all subsets spatial_regions = [self._get_spatial_region(dataset=self.dataset.selected[0], subset=subset) # noqa - for subset in self.selected if subset != self._manual_options] + for subset in selected if subset != self._manual_options] else: # use cached version spatial_regions = [self.selected_spatial_region] @@ -2140,9 +2150,9 @@ def _get_mark_coords(self, viewer): else: wcs = getattr(viewer.state.reference_data, 'coords', None) if wcs is None: - self.selected_validity = {'is_aperture': False, - 'aperture_message': 'invalid wcs'} - return [], [] + validity = {'is_aperture': False, + 'aperture_message': 'invalid wcs'} + return [], [], validity pixel_region = spatial_region.to_pixel(wcs) roi = regions2roi(pixel_region) @@ -2164,15 +2174,15 @@ def _get_mark_coords(self, viewer): roi.ymin = center[1] - half_height roi.ymax = center[1] + half_height elif isinstance(roi, CircularAnnulusROI): - self.selected_validity = {'is_aperture': False, - 'aperture_message': 'annulus is not a supported aperture'} - return [], [] + validity = {'is_aperture': False, + 'aperture_message': 'annulus is not a supported aperture'} + return [], [], validity else: # pragma: no cover # known unsupported shapes: annulus # TODO: specific case for annulus - self.selected_validity = {'is_aperture': False, - 'aperture_message': 'shape does not support scale factor'} - return [], [] + validity = {'is_aperture': False, + 'aperture_message': 'shape does not support scale factor'} + return [], [], validity if hasattr(roi, 'to_polygon'): x, y = roi.to_polygon() @@ -2181,16 +2191,16 @@ def _get_mark_coords(self, viewer): x_coords = np.concatenate((x_coords, np.array([np.nan]), x)) y_coords = np.concatenate((y_coords, np.array([np.nan]), y)) else: - self.selected_validity = {'is_aperture': False, - 'aperture_message': 'could not convert roi to polygon'} - return [], [] + validity = {'is_aperture': False, + 'aperture_message': 'could not convert roi to polygon'} + return [], [], validity - self.selected_validity = {'is_aperture': True} - return x_coords, y_coords + validity = {'is_aperture': True} + return x_coords, y_coords, validity def _update_mark_coords(self, *args): for viewer in self.image_viewers: - x_coords, y_coords = self._get_mark_coords(viewer) + x_coords, y_coords, self.selected_validity = self._get_mark_coords_and_validate(viewer) for mark in self.marks: if mark.viewer != viewer: continue From acd2d37363073ae4e73ec24360a2c544f2050334 Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Fri, 2 Feb 2024 08:44:17 -0500 Subject: [PATCH 7/7] don't skip applying dataset defaults based on aperture --- .../configs/imviz/plugins/aper_phot_simple/aper_phot_simple.py | 2 -- jdaviz/configs/imviz/tests/test_parser.py | 3 +++ 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/jdaviz/configs/imviz/plugins/aper_phot_simple/aper_phot_simple.py b/jdaviz/configs/imviz/plugins/aper_phot_simple/aper_phot_simple.py index 7dd1831914..5654dac582 100644 --- a/jdaviz/configs/imviz/plugins/aper_phot_simple/aper_phot_simple.py +++ b/jdaviz/configs/imviz/plugins/aper_phot_simple/aper_phot_simple.py @@ -235,8 +235,6 @@ def _dataset_selected_changed(self, event={}): # but we still need to update the flux-scaling warning self._multiselect_flux_scaling_warning() return - if not self.aperture_selected_validity.get('is_aperture'): - return try: defaults = self._get_defaults_from_metadata() diff --git a/jdaviz/configs/imviz/tests/test_parser.py b/jdaviz/configs/imviz/tests/test_parser.py index 8cf134ef38..909b42d9df 100644 --- a/jdaviz/configs/imviz/tests/test_parser.py +++ b/jdaviz/configs/imviz/tests/test_parser.py @@ -266,8 +266,10 @@ def test_parse_jwst_nircam_level2(self, imviz_helper): phot_plugin = imviz_helper.app.get_tray_item_from_name('imviz-aper-phot-simple') phot_plugin.data_selected = 'contents[DATA]' phot_plugin.aperture_selected = 'Subset 1' + assert phot_plugin.aperture.selected_validity.get('is_aperture') assert_allclose(phot_plugin.background_value, 0) phot_plugin.background_selected = 'Subset 2' + assert phot_plugin.aperture.selected_validity.get('is_aperture') assert_allclose(phot_plugin.background_value, 0.1741226315498352) # Subset 2 median # NOTE: jwst.datamodels.find_fits_keyword("PHOTMJSR") phot_plugin.counts_factor = (data.meta['photometry']['conversion_megajanskys'] / @@ -396,6 +398,7 @@ def test_parse_hst_drz(self, imviz_helper): phot_plugin = imviz_helper.app.get_tray_item_from_name('imviz-aper-phot-simple') phot_plugin.data_selected = 'contents[SCI,1]' phot_plugin.aperture_selected = 'Subset 1' + assert phot_plugin.aperture.selected_validity.get('is_aperture') phot_plugin.background_value = 0.0014 # Manual entry: Median on whole array assert_allclose(phot_plugin.pixel_area, 0.0025) # Not used but still auto-populated phot_plugin.vue_do_aper_phot()