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.

Sign up for GitHub

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

BUG: Simple Aperture Photometry plugin to update when Subset updates #1447

Merged
merged 1 commit into from
Jul 6, 2022
Merged

BUG: Simple Aperture Photometry plugin to update when Subset updates #1447

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions CHANGES.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
^^^^^^

Expand Down
17 changes: 16 additions & 1 deletion jdaviz/configs/imviz/plugins/aper_phot_simple/aper_phot_simple.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 = []
Expand Down Expand Up @@ -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:
Expand All @@ -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)
Expand Down Expand Up @@ -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.
Expand Down
8 changes: 8 additions & 0 deletions jdaviz/configs/imviz/tests/test_simple_aper_phot.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devs, I am not familiar with the edit mode syntax, so please let me know if it is correct to test this way.

Would also be nice if you can tell me how to programmatically control which Subset to put in ReplaceMode.

Thanks!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To programmatically set the subset in replace mode, you can do something like:

imviz_helper.app.session.edit_subset_mode.edit_subset = [imviz_helper.app.data_collection.subset_groups[0]]

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)
Expand Down