From 6b8e0b94a16bb1a0d4561ee1454b0a012f67b620 Mon Sep 17 00:00:00 2001
From: "Pey Lian Lim (Github)" <2090236+pllim@users.noreply.github.com>
Date: Mon, 4 Jul 2022 17:43:34 -0400
Subject: [PATCH] BUG: Aper phot to update when Subset updates

---
 CHANGES.rst                                     |  4 ++++
 .../aper_phot_simple/aper_phot_simple.py        | 17 ++++++++++++++++-
 .../imviz/tests/test_simple_aper_phot.py        |  8 ++++++++
 3 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/CHANGES.rst b/CHANGES.rst
index b459a63079..03129ab63e 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -105,6 +105,10 @@ Imviz
 - Fixed a bug where some custom colormap added to Imviz is inaccessible
   via ``viewer.set_colormap()`` API. [#1440]
 
+- Fixed a bug where Simple Aperture Photometry plugin does not know
+  an existing Subset has been modified until it is reselected from
+  the dropdown menu. [#1447]
+
 Mosviz
 ^^^^^^
 
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 a49eba7569..fdc7711430 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
@@ -10,6 +10,7 @@
 from astropy.modeling.models import Gaussian1D
 from astropy.table import QTable
 from astropy.time import Time
+from glue.core.message import SubsetUpdateMessage
 from ipywidgets import widget_serialization
 from photutils.aperture import (ApertureStats, CircularAperture, EllipticalAperture,
                                 RectangularAperture)
@@ -72,6 +73,8 @@ def __init__(self, *args, **kwargs):
         self.current_plot_type = self.plot_types[0]
         self._fitted_model_name = 'phot_radial_profile'
 
+        self.session.hub.subscribe(self, SubsetUpdateMessage, handler=self._on_subset_update)
+
     def reset_results(self):
         self.result_available = False
         self.results = []
@@ -131,7 +134,6 @@ def _dataset_selected_changed(self, event={}):
         # Update self._selected_subset with the new self._selected_data
         # and auto-populate background, if applicable.
         self._subset_selected_changed()
-        self._bg_subset_selected_changed()
 
     def _get_region_from_subset(self, subset):
         for subset_grp in self.app.data_collection.subset_groups:
@@ -144,6 +146,16 @@ def _get_region_from_subset(self, subset):
         else:
             raise ValueError(f'Subset "{subset}" not found')
 
+    def _on_subset_update(self, msg):
+        if self.dataset_selected == '' or self.subset_selected == '':
+            return
+
+        sbst = msg.subset
+        if sbst.label == self.subset_selected and sbst.data.label == self.dataset_selected:
+            self._subset_selected_changed()
+        elif sbst.label == self.bg_subset_selected and sbst.data.label == self.dataset_selected:
+            self._bg_subset_selected_changed()
+
     @observe('subset_selected')
     def _subset_selected_changed(self, event={}):
         subset_selected = event.get('new', self.subset_selected)
@@ -172,6 +184,9 @@ def _subset_selected_changed(self, event={}):
             self.hub.broadcast(SnackbarMessage(
                 f"Failed to extract {subset_selected}: {repr(e)}", color='error', sender=self))
 
+        else:
+            self._bg_subset_selected_changed()
+
     def _calc_bg_subset_median(self, reg):
         # Basically same way image stats are calculated in vue_do_aper_phot()
         # except here we only care about one stat for the background.
diff --git a/jdaviz/configs/imviz/tests/test_simple_aper_phot.py b/jdaviz/configs/imviz/tests/test_simple_aper_phot.py
index ac602550b4..35ef1c9f1d 100644
--- a/jdaviz/configs/imviz/tests/test_simple_aper_phot.py
+++ b/jdaviz/configs/imviz/tests/test_simple_aper_phot.py
@@ -2,6 +2,7 @@
 import numpy as np
 from astropy import units as u
 from astropy.tests.helper import assert_quantity_allclose
+from glue.core.edit_subset_mode import ReplaceMode
 from numpy.testing import assert_allclose, assert_array_equal
 from photutils.aperture import (ApertureStats, CircularAperture, EllipticalAperture,
                                 RectangularAperture, EllipticalAnnulus)
@@ -249,6 +250,13 @@ def test_annulus_background(imviz_helper):
     phot_plugin.bg_annulus_width = 5
     assert_allclose(phot_plugin.background_value, 4.894003242594493)
 
+    # Move the last created Subset (ellipse) and make sure background updates
+    imviz_helper.app.session.edit_subset_mode.mode = ReplaceMode
+    imviz_helper._apply_interactive_region('bqplot:ellipse', (0, 30), (51, 55))
+    assert_allclose(phot_plugin.bg_annulus_inner_r, 40)
+    assert_allclose(phot_plugin.bg_annulus_width, 5)
+    assert_allclose(phot_plugin.background_value, 4.894003)
+
     # Bad annulus should not crash plugin
     phot_plugin.bg_annulus_inner_r = -1
     assert_allclose(phot_plugin.background_value, 0)