Aperture validate after selection (#2684)

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
 ^^^^^^
 

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,

jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.vue

@@ -19,7 +19,13 @@
         hint="Select a spatial region to extract its spectrum."
       />
 
-      <div v-if="aperture_selected !== 'Entire Cube' && dev_cone_support">
+      <v-row v-if="aperture_selected !== 'Entire Cube' && !aperture_selected_validity.is_aperture && dev_cone_support">
+        <span class="v-messages v-messages__message text--secondary">
+            {{aperture_selected}} does not support wavelength dependence (cone support): {{aperture_selected_validity.aperture_message}}.
+        </span>
+      </v-row>
+
+      <div v-if="aperture_selected_validity.is_aperture && dev_cone_support">
         <v-row>
           <v-switch
             v-model="wavelength_dependent"
@@ -74,7 +80,16 @@
         </span>
       </v-row>
 
-      <div v-if="dev_cone_support && wavelength_dependent">
+      <v-row v-if="bg_selected !== 'None' && !bg_selected_validity.is_aperture && dev_cone_support">
+        <span class="v-messages v-messages__message text--secondary">
+            {{bg_selected}} does not support wavelength dependence (cone support): {{bg_selected_validity.aperture_message}}.
+        </span>
+      </v-row>
+
+      <div v-if="aperture_selected_validity.is_aperture
+                 && bg_selected_validity.is_aperture
+                 && wavelength_dependent
+                 && dev_cone_support">
         <v-row>
           <v-switch
             v-model="bg_wavelength_dependent"
@@ -102,7 +117,22 @@
 
     <div @mouseover="() => active_step='ext'">
       <j-plugin-section-header :active="active_step==='ext'">Extract</j-plugin-section-header>
-      <div v-if="dev_subpixel_support">
+
+      <v-row v-if="aperture_selected !== 'None' && !aperture_selected_validity.is_aperture && dev_subpixel_support">
+        <span class="v-messages v-messages__message text--secondary">
+            Aperture: {{aperture_selected}} does not support subpixel: {{aperture_selected_validity.aperture_message}}.
+        </span>
+      </v-row>
+      <v-row v-if="bg_selected !== 'None' && !bg_selected_validity.is_aperture && dev_subpixel_support">
+        <span class="v-messages v-messages__message text--secondary">
+            Background: {{bg_selected}} does not support subpixel: {{bg_selected_validity.aperture_message}}.
+        </span>
+      </v-row>
+
+
+      <div v-if="(aperture_selected === 'Entire Cube' || aperture_selected_validity.is_aperture)
+                 && (bg_selected === 'None' || bg_selected_validity.is_aperture)
+                 && dev_subpixel_support">
         <v-row>
           <v-switch
             v-model="subpixel"
@@ -138,6 +168,7 @@
         action_label="Extract"
         action_tooltip="Run spectral extraction with error and mask propagation"
         :action_spinner="spinner"
+        :action_disabled="aperture_selected === bg_selected"
         @click:action="spectral_extraction"
       ></plugin-add-results>
 

jdaviz/configs/imviz/plugins/aper_phot_simple/aper_phot_simple.py

@@ -286,6 +286,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
@@ -399,13 +402,21 @@ 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'):
+                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

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)."
       />
 
+      <v-row v-if="aperture_selected.length && !aperture_selected_validity.is_aperture">
+        <span class="v-messages v-messages__message text--secondary" style="color: red !important">
+            {{aperture_selected}} is not a valid aperture: {{aperture_selected_validity.aperture_message}}.
+        </span>
+      </v-row>
+
       <div v-if="aperture_selected.length > 0">
         <plugin-subset-select
           :items="background_items"
@@ -175,7 +181,7 @@
             :results_isolated_to_plugin="true"
             @click="do_aper_phot"
             :spinner="spinner"
-            :disabled="aperture_selected === background_selected"
+            :disabled="aperture_selected === background_selected || !aperture_selected_validity.is_aperture"
           >
             Calculate
           </plugin-action-button>

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()

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