From 1c2baab9ddbd3371a12037173ac43059e640fe8b Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Tue, 20 Aug 2024 11:43:10 -0400 Subject: [PATCH 01/11] Debugging Cubeviz batch photometry --- .../aper_phot_simple/aper_phot_simple.py | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 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 418bbadfef..29462ce0b3 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 @@ -145,10 +145,17 @@ def _on_dataset_selected_changed(self, event={}): if self.config != "cubeviz": return # self.dataset might not exist when app is setting itself up. - if hasattr(self, "dataset") and self.dataset.selected_dc_item.ndim > 2: - self.is_cube = True - else: - self.is_cube = False + if hasattr(self, "dataset"): + if isinstance(self.dataset.selected_dc_item, list): + datasets = self.dataset.selected_dc_item + else: + datasets = [self.dataset.selected_dc_item] + for dataset in datasets: + if dataset.ndim > 2: + self.is_cube = True + break + else: + self.is_cube = False def _on_display_units_changed(self, event={}): @@ -544,6 +551,8 @@ def calculate_photometry(self, dataset=None, aperture=None, background=None, if self.config == 'cubeviz': display_unit = u.Unit(self.display_flux_or_sb_unit) + print(img_unit, display_unit) + if background is not None and background not in self.background.choices: # pragma: no cover raise ValueError(f"background must be one of {self.background.choices}") if background_value is not None: @@ -1060,6 +1069,7 @@ def calculate_batch_photometry(self, options=[], add_to_table=True, update_plots option.setdefault('pixel_area', defaults.get('pixel_area', 0)) if self.flux_scaling_multi_auto: option.setdefault('flux_scaling', defaults.get('flux_scaling', 0)) + try: self.calculate_photometry(add_to_table=add_to_table, update_plots=this_update_plots, From 547d831d1c29d3afffda45b54afc704d287a6a12 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Thu, 22 Aug 2024 10:23:35 -0400 Subject: [PATCH 02/11] Fix changing units when in batch mode --- .../imviz/plugins/aper_phot_simple/aper_phot_simple.py | 4 ++-- 1 file changed, 2 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 29462ce0b3..74d4ad8273 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 @@ -215,6 +215,8 @@ def _set_display_unit_of_selected_dataset(self): return data = self.dataset.selected_dc_item + if isinstance(data, list): + data = data[0] comp = data.get_component(data.main_components[0]) if comp.units: # if data is something-per-solid-angle, its a SB unit and we should @@ -551,8 +553,6 @@ def calculate_photometry(self, dataset=None, aperture=None, background=None, if self.config == 'cubeviz': display_unit = u.Unit(self.display_flux_or_sb_unit) - print(img_unit, display_unit) - if background is not None and background not in self.background.choices: # pragma: no cover raise ValueError(f"background must be one of {self.background.choices}") if background_value is not None: From 37087c41179c29b29aca45ed8089641b6368a2f4 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Thu, 22 Aug 2024 11:47:20 -0400 Subject: [PATCH 03/11] Working on adding cubeviz batch photometry test --- CHANGES.rst | 2 ++ .../imviz/tests/test_simple_aper_phot.py | 27 +++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/CHANGES.rst b/CHANGES.rst index fbbdb5b603..b44ef6fa01 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -113,6 +113,8 @@ Bug Fixes - Aperture Photometry plugin no longer allows negative counts conversion factor. [#3154] +- Batch mode aperture photometry now works in Cubeviz. [#3163] + Cubeviz ^^^^^^^ diff --git a/jdaviz/configs/imviz/tests/test_simple_aper_phot.py b/jdaviz/configs/imviz/tests/test_simple_aper_phot.py index 0b1a49cfb6..bf90def766 100644 --- a/jdaviz/configs/imviz/tests/test_simple_aper_phot.py +++ b/jdaviz/configs/imviz/tests/test_simple_aper_phot.py @@ -4,6 +4,7 @@ from astropy.io import fits from astropy.tests.helper import assert_quantity_allclose from astropy.utils.data import get_pkg_data_filename +from glue.core.roi import CircularROI from numpy.testing import assert_allclose, assert_array_equal from photutils.aperture import (ApertureStats, CircularAperture, EllipticalAperture, RectangularAperture, EllipticalAnnulus) @@ -489,3 +490,29 @@ def test_curve_of_growth(with_unit): with pytest.raises(TypeError, match='Unsupported aperture'): _curve_of_growth(data, cen, EllipticalAnnulus(cen, 3, 8, 5), 100, pixarea_fac=pixarea_fac) + + +def test_cubeviz_batch(cubeviz_helper, spectrum1d_cube): + cubeviz_helper.load_data(spectrum1d_cube, data_label='test') + subset_plugin = cubeviz_helper.plugins['Subset Tools']._obj + phot_plugin = cubeviz_helper.plugins['Aperture Photometry']._obj + uc_plugin = cubeviz_helper.plugins['Unit Conversion'] + + fv = cubeviz_helper.app.get_viewer('flux-viewer') + fv.apply_roi(CircularROI(xc=5, yc=5, radius=2)) + + subset_plugin.subset_selected = 'Create New' + fv.apply_roi(CircularROI(xc=3, yc=3, radius=2)) + + phot_plugin.dataset_selected = 'test[FLUX]' + phot_plugin.multiselect = True + phot_plugin.aperture.selected = ['Subset 1', 'Subset 2'] + + print(uc_plugin.flux_unit) + uc_plugin.flux_unit = 'MJy' + print(uc_plugin.flux_unit) + + phot_plugin.calculate_batch_photometry() + + print() + raise ValueError \ No newline at end of file From b847c24ad14a2894358ac4ffdf07a44aa91e8aaa Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Thu, 22 Aug 2024 15:28:53 -0400 Subject: [PATCH 04/11] Add simple test --- .../imviz/plugins/aper_phot_simple/aper_phot_simple.py | 3 ++- jdaviz/configs/imviz/tests/test_simple_aper_phot.py | 9 +++------ 2 files changed, 5 insertions(+), 7 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 74d4ad8273..fdbcb498e7 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 @@ -113,7 +113,8 @@ def valid_cubeviz_datasets(data): acceptable_types = ['spectral flux density wav', 'photon flux density wav', 'spectral flux density', - 'photon flux density'] + 'photon flux density', + 'surface brightness'] return ((data.ndim in (2, 3)) and ((img_unit == (u.MJy / u.sr)) or (img_unit.physical_type in acceptable_types))) diff --git a/jdaviz/configs/imviz/tests/test_simple_aper_phot.py b/jdaviz/configs/imviz/tests/test_simple_aper_phot.py index bf90def766..84a327086b 100644 --- a/jdaviz/configs/imviz/tests/test_simple_aper_phot.py +++ b/jdaviz/configs/imviz/tests/test_simple_aper_phot.py @@ -492,8 +492,8 @@ def test_curve_of_growth(with_unit): pixarea_fac=pixarea_fac) -def test_cubeviz_batch(cubeviz_helper, spectrum1d_cube): - cubeviz_helper.load_data(spectrum1d_cube, data_label='test') +def test_cubeviz_batch(cubeviz_helper, spectrum1d_cube_fluxunit_jy_per_steradian): + cubeviz_helper.load_data(spectrum1d_cube_fluxunit_jy_per_steradian, data_label='test') subset_plugin = cubeviz_helper.plugins['Subset Tools']._obj phot_plugin = cubeviz_helper.plugins['Aperture Photometry']._obj uc_plugin = cubeviz_helper.plugins['Unit Conversion'] @@ -508,11 +508,8 @@ def test_cubeviz_batch(cubeviz_helper, spectrum1d_cube): phot_plugin.multiselect = True phot_plugin.aperture.selected = ['Subset 1', 'Subset 2'] - print(uc_plugin.flux_unit) uc_plugin.flux_unit = 'MJy' - print(uc_plugin.flux_unit) phot_plugin.calculate_batch_photometry() - print() - raise ValueError \ No newline at end of file + assert(len(phot_plugin.table)) == 2 From bad39c04e151838726152da14af6dade0cda8ab6 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Thu, 22 Aug 2024 15:31:18 -0400 Subject: [PATCH 05/11] Codestyle --- jdaviz/configs/imviz/tests/test_simple_aper_phot.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jdaviz/configs/imviz/tests/test_simple_aper_phot.py b/jdaviz/configs/imviz/tests/test_simple_aper_phot.py index 84a327086b..e729a59ec5 100644 --- a/jdaviz/configs/imviz/tests/test_simple_aper_phot.py +++ b/jdaviz/configs/imviz/tests/test_simple_aper_phot.py @@ -512,4 +512,4 @@ def test_cubeviz_batch(cubeviz_helper, spectrum1d_cube_fluxunit_jy_per_steradian phot_plugin.calculate_batch_photometry() - assert(len(phot_plugin.table)) == 2 + assert len(phot_plugin.table) == 2 From c8fe010bf6725f79b28427fe6ac0ac2bf25a7c2d Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Fri, 23 Aug 2024 09:22:53 -0400 Subject: [PATCH 06/11] Add comment about future changes --- .../configs/imviz/plugins/aper_phot_simple/aper_phot_simple.py | 2 ++ 1 file changed, 2 insertions(+) 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 fdbcb498e7..b2d807dac9 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 @@ -152,6 +152,8 @@ def _on_dataset_selected_changed(self, event={}): else: datasets = [self.dataset.selected_dc_item] for dataset in datasets: + # This assumes all cubes, or no cubes. If we allow photometry on collapsed cubes + # or images this will need to change. if dataset.ndim > 2: self.is_cube = True break From b6417282cf25bc82306444b817f9de592e052fc1 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Fri, 23 Aug 2024 09:59:16 -0400 Subject: [PATCH 07/11] Move changelog to 3.10.4 --- CHANGES.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index b44ef6fa01..ee25b56c56 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -113,8 +113,6 @@ Bug Fixes - Aperture Photometry plugin no longer allows negative counts conversion factor. [#3154] -- Batch mode aperture photometry now works in Cubeviz. [#3163] - Cubeviz ^^^^^^^ @@ -231,6 +229,8 @@ Bug Fixes Cubeviz ^^^^^^^ +- Fixed multiple select handling for batch mode aperture photometry in Cubeviz. [#3163] + Imviz ^^^^^ From c8f07ef0d9096f2ac0f49eac20f3afe035e7765c Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Mon, 26 Aug 2024 15:51:07 -0400 Subject: [PATCH 08/11] Don't convert if display unit hasn't been initialized --- .../aper_phot_simple/aper_phot_simple.py | 48 ++++++++++--------- 1 file changed, 25 insertions(+), 23 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 b2d807dac9..d3538f0dd0 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 @@ -151,14 +151,14 @@ def _on_dataset_selected_changed(self, event={}): datasets = self.dataset.selected_dc_item else: datasets = [self.dataset.selected_dc_item] + + self.is_cube = False for dataset in datasets: # This assumes all cubes, or no cubes. If we allow photometry on collapsed cubes # or images this will need to change. if dataset.ndim > 2: self.is_cube = True break - else: - self.is_cube = False def _on_display_units_changed(self, event={}): @@ -569,7 +569,7 @@ def calculate_photometry(self, dataset=None, aperture=None, background=None, # cubeviz: background_value set in plugin is in display units # convert temporarily to image units for calculations - if (self.config == 'cubeviz') and (img_unit is not None): + if (self.config == 'cubeviz') and (img_unit is not None) and display_unit != '': background_value = (background_value * display_unit).to_value( img_unit, u.spectral_density(self._cube_wave)) @@ -580,7 +580,7 @@ def calculate_photometry(self, dataset=None, aperture=None, background=None, # cubeviz: background_value set in plugin is in display units # convert temporarily to image units for calculations - if (self.config == 'cubeviz') and (img_unit is not None): + if (self.config == 'cubeviz') and (img_unit is not None) and display_unit != '': background_value = (background_value * display_unit).to_value( img_unit, u.spectral_density(self._cube_wave)) else: @@ -590,7 +590,7 @@ def calculate_photometry(self, dataset=None, aperture=None, background=None, # cubeviz: computed background median will be in display units, # convert temporarily back to image units for calculations - if (self.config == 'cubeviz') and (img_unit is not None): + if (self.config == 'cubeviz') and (img_unit is not None) and display_unit != '': background_value = (background_value * display_unit).to_value( img_unit, u.spectral_density(self._cube_wave)) try: @@ -738,25 +738,26 @@ def calculate_photometry(self, dataset=None, aperture=None, background=None, # convert units of certain columns in aperture phot. output table # to reflect display units (i.e if data units are MJy / sr, but # Jy / sr is selected in Unit Conversion plugin) - phot_table['background'] = phot_table['background'].to( - display_unit, u.spectral_density(self._cube_wave)) - - if include_pixarea_fac: - phot_table['sum'] = phot_table['sum'].to( - (display_unit * pixarea_fac).unit, u.spectral_density(self._cube_wave)) - else: - phot_table['sum'] = phot_table['sum'].to( + if display_unit != '': + phot_table['background'] = phot_table['background'].to( display_unit, u.spectral_density(self._cube_wave)) - for key in ['min', 'max', 'mean', 'median', 'mode', 'std', - 'mad_std', 'biweight_location']: - phot_table[key] = phot_table[key].to( - display_unit, u.spectral_density(self._cube_wave)) - for key in ['var', 'biweight_midvariance']: - try: - phot_table[key] = phot_table[key].to(display_unit**2) - # FIXME: Can fail going between per-wave and per-freq - except u.UnitConversionError: - pass + + if include_pixarea_fac: + phot_table['sum'] = phot_table['sum'].to( + (display_unit * pixarea_fac).unit, u.spectral_density(self._cube_wave)) + else: + phot_table['sum'] = phot_table['sum'].to( + display_unit, u.spectral_density(self._cube_wave)) + for key in ['min', 'max', 'mean', 'median', 'mode', 'std', + 'mad_std', 'biweight_location']: + phot_table[key] = phot_table[key].to( + display_unit, u.spectral_density(self._cube_wave)) + for key in ['var', 'biweight_midvariance']: + try: + phot_table[key] = phot_table[key].to(display_unit**2) + # FIXME: Can fail going between per-wave and per-freq + except u.UnitConversionError: + pass if add_to_table: try: @@ -1078,6 +1079,7 @@ def calculate_batch_photometry(self, options=[], add_to_table=True, update_plots update_plots=this_update_plots, **option) except Exception as e: + raise failed_iters.append(i) if full_exceptions: exceptions.append(e) From 4556ff7481ade740f149fb8680d34979e6b9aecc Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Mon, 26 Aug 2024 16:21:59 -0400 Subject: [PATCH 09/11] Update test --- .../imviz/tests/test_simple_aper_phot.py | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/jdaviz/configs/imviz/tests/test_simple_aper_phot.py b/jdaviz/configs/imviz/tests/test_simple_aper_phot.py index e729a59ec5..4623938565 100644 --- a/jdaviz/configs/imviz/tests/test_simple_aper_phot.py +++ b/jdaviz/configs/imviz/tests/test_simple_aper_phot.py @@ -498,18 +498,26 @@ def test_cubeviz_batch(cubeviz_helper, spectrum1d_cube_fluxunit_jy_per_steradian phot_plugin = cubeviz_helper.plugins['Aperture Photometry']._obj uc_plugin = cubeviz_helper.plugins['Unit Conversion'] - fv = cubeviz_helper.app.get_viewer('flux-viewer') - fv.apply_roi(CircularROI(xc=5, yc=5, radius=2)) - - subset_plugin.subset_selected = 'Create New' - fv.apply_roi(CircularROI(xc=3, yc=3, radius=2)) + cubeviz_helper.load_regions(CirclePixelRegion(center=PixCoord(x=5, y=5), radius=2)) + cubeviz_helper.load_regions(CirclePixelRegion(center=PixCoord(x=3, y=3), radius=2)) phot_plugin.dataset_selected = 'test[FLUX]' phot_plugin.multiselect = True phot_plugin.aperture.selected = ['Subset 1', 'Subset 2'] + phot_plugin.calculate_batch_photometry(full_exceptions=True) + assert len(phot_plugin.table) == 2 + tbl = cubeviz_helper.get_aperture_photometry_results() + assert_quantity_allclose(tbl['sum'], [5.980836e-12, 2.037396e-10] * u.Jy, rtol=1e-4) + + # Test that it still works after unit conversion uc_plugin.flux_unit = 'MJy' - phot_plugin.calculate_batch_photometry() + phot_plugin.calculate_batch_photometry(full_exceptions=True) - assert len(phot_plugin.table) == 2 + assert len(phot_plugin.table) == 4 + tbl = cubeviz_helper.get_aperture_photometry_results() + # get_aperture_photometry_results converts all to the same units + assert_quantity_allclose(tbl['sum'], + [5.980836e-12, 2.037396e-10, 5.980836e-12, 2.037396e-10] * u.Jy, + rtol=1e-4) From 9328fdbc42dca0d4893c7cd6b9f1d490f87c0313 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Mon, 26 Aug 2024 16:52:31 -0400 Subject: [PATCH 10/11] Codestyle --- jdaviz/configs/imviz/tests/test_simple_aper_phot.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/jdaviz/configs/imviz/tests/test_simple_aper_phot.py b/jdaviz/configs/imviz/tests/test_simple_aper_phot.py index 4623938565..2f00530d21 100644 --- a/jdaviz/configs/imviz/tests/test_simple_aper_phot.py +++ b/jdaviz/configs/imviz/tests/test_simple_aper_phot.py @@ -4,7 +4,6 @@ from astropy.io import fits from astropy.tests.helper import assert_quantity_allclose from astropy.utils.data import get_pkg_data_filename -from glue.core.roi import CircularROI from numpy.testing import assert_allclose, assert_array_equal from photutils.aperture import (ApertureStats, CircularAperture, EllipticalAperture, RectangularAperture, EllipticalAnnulus) @@ -494,7 +493,6 @@ def test_curve_of_growth(with_unit): def test_cubeviz_batch(cubeviz_helper, spectrum1d_cube_fluxunit_jy_per_steradian): cubeviz_helper.load_data(spectrum1d_cube_fluxunit_jy_per_steradian, data_label='test') - subset_plugin = cubeviz_helper.plugins['Subset Tools']._obj phot_plugin = cubeviz_helper.plugins['Aperture Photometry']._obj uc_plugin = cubeviz_helper.plugins['Unit Conversion'] From e0223c4aab8798b83a97e9147a875d131220b928 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen <39831871+rosteen@users.noreply.github.com> Date: Tue, 27 Aug 2024 09:33:30 -0400 Subject: [PATCH 11/11] Update jdaviz/configs/imviz/plugins/aper_phot_simple/aper_phot_simple.py Co-authored-by: P. L. Lim <2090236+pllim@users.noreply.github.com> --- .../configs/imviz/plugins/aper_phot_simple/aper_phot_simple.py | 1 - 1 file changed, 1 deletion(-) 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 d3538f0dd0..7a9768a6a9 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 @@ -1079,7 +1079,6 @@ def calculate_batch_photometry(self, options=[], add_to_table=True, update_plots update_plots=this_update_plots, **option) except Exception as e: - raise failed_iters.append(i) if full_exceptions: exceptions.append(e)