From e7ccc9bb8b0b75d54cf015c8d00799326badb518 Mon Sep 17 00:00:00 2001 From: Jesse Averbukh Date: Mon, 6 Mar 2023 13:50:39 -0500 Subject: [PATCH 1/8] Retrieve accurate spectral subset bounds Add get_subsets method which replaces get_subsets_from_viewer Remove unused imports Fix specviz helper tests Fix failing tests and address review comments Address review comments Ignore ipykernel deprecation warning Add tests covering composite spectral regions Add check if subset label not in dc subset groups Rename to object_only --- jdaviz/app.py | 280 ++++++++++++++---- .../cubeviz/plugins/tests/test_parsers.py | 4 +- .../model_fitting/tests/test_plugin.py | 2 +- jdaviz/configs/specviz/helper.py | 4 +- .../line_analysis/tests/test_line_analysis.py | 6 +- jdaviz/configs/specviz/tests/test_helper.py | 104 +++++-- jdaviz/core/tests/test_template_mixin.py | 4 +- jdaviz/tests/test_subsets.py | 97 +++++- 8 files changed, 398 insertions(+), 103 deletions(-) diff --git a/jdaviz/app.py b/jdaviz/app.py index 132ebec1c8..c1ea1ff5a8 100644 --- a/jdaviz/app.py +++ b/jdaviz/app.py @@ -4,12 +4,16 @@ import uuid import warnings from inspect import isclass +import operator from ipywidgets import widget_serialization import ipyvue from astropy.nddata import CCDData, NDData from astropy.io import fits +from astropy import units as u +from astropy.coordinates import Angle +from regions import PixCoord, CirclePixelRegion, RectanglePixelRegion, EllipsePixelRegion from echo import CallbackProperty, DictCallbackProperty, ListCallbackProperty from ipygoldenlayout import GoldenLayout @@ -31,7 +35,9 @@ SubsetUpdateMessage, SubsetDeleteMessage) from glue.core.state_objects import State -from glue.core.subset import Subset, RangeSubsetState, RoiSubsetState +from glue.core.subset import (Subset, RangeSubsetState, RoiSubsetState, + CompositeSubsetState, InvertState) +from glue.core.roi import CircularROI, EllipticalROI, RectangularROI from glue_astronomy.spectral_coordinates import SpectralCoordinates from glue_jupyter.app import JupyterApplication from glue_jupyter.common.toolbar_vuetify import read_icon @@ -729,54 +735,6 @@ def get_subsets_from_viewer(self, viewer_reference, data_label=None, subset_type cls=None) regions = {} - def _get_all_subregions(mask, spec_axis_data): - """ - Return all subregions within a subset. - - Parameters - ---------- - mask : list - List of indices in spec_axis_data that are part of the subset. - spec_axis_data : list - List of spectral axis values. - Returns - ------- - combined_spec_region : `~specutils.SpectralRegion` - SpectralRegion object containing all subregions of the subset. - """ - if len(mask) == 0: - # Mask should only be 0 if ApplyROI is used to incorrectly - # create subsets via the API - raise ValueError("Mask has length 0, ApplyROI may have been used incorrectly") - - current_edge = 0 - combined_spec_region = None - for index in range(1, len(mask)): - # Find spot where mask == True is for a different region of the subset - # i.e. mask = [0, 1, 4, 5] - # mask[2] != mask[1] + 1 - if mask[index] != mask[index - 1] + 1: - subset_region = spec_axis_data[mask[current_edge]: mask[index - 1] + 1] - if not combined_spec_region: - combined_spec_region = SpectralRegion(min(subset_region), - max(subset_region)) - else: - combined_spec_region += SpectralRegion(min(subset_region), - max(subset_region)) - current_edge = index - - # Get last region within the subset - if current_edge != index: - subset_region = spec_axis_data[mask[current_edge]: mask[index] + 1] - # No if check here because len(mask) must be greater than 1 - # so combined_spec_region will have been instantiated in the for loop - if combined_spec_region is None: - combined_spec_region = SpectralRegion(min(subset_region), max(subset_region)) - else: - combined_spec_region += SpectralRegion(min(subset_region), max(subset_region)) - - return combined_spec_region - if data_label is not None: data = {data_label: data} @@ -815,26 +773,29 @@ def _get_all_subregions(mask, spec_axis_data): # TODO: this needs to be much simpler; i.e. data units in # the glue component objects # Cases where there is a single subset - if '_orig_spec' in value.data.meta: # Hack for Cubeviz WCS propagation - data_wcs = value.data.meta['_orig_spec'] - else: - data_wcs = value.data.coords - subregions_in_subset = _get_all_subregions( - np.where(value.to_mask() == True)[0], # noqa - data_wcs.spectral_axis) - - regions[key] = subregions_in_subset + # if '_orig_spec' in value.data.meta: # Hack for Cubeviz WCS propagation + # data_wcs = value.data.meta['_orig_spec'] + # else: + # data_wcs = value.data.coords + # + # subregions_in_subset = _get_all_subregions( + # np.where(value.to_mask() == True)[0], # noqa + # data_wcs.spectral_axis) + # + # # regions[key] = subregions_in_subset + regions[key] = self.get_subsets(key) continue temp_data = self.get_data_from_viewer(viewer_reference, value.label) if isinstance(temp_data, Spectrum1D): - # Note that we look for mask == False here, rather than True above, - # because specutils masks are the reverse of Glue (of course) - subregions_in_subset = _get_all_subregions( - np.where(~temp_data.mask)[0], # noqa - temp_data.spectral_axis) - regions[key] = subregions_in_subset + # # Note that we look for mask == False here, rather than True above, + # # because specutils masks are the reverse of Glue (of course) + # subregions_in_subset = _get_all_subregions( + # np.where(~temp_data.mask)[0], # noqa + # temp_data.spectral_axis) + # regions[key] = subregions_in_subset + regions[key] = self.get_subsets(key) continue # Get the pixel coordinate [z] of the 3D data, repeating the @@ -887,6 +848,197 @@ def _get_all_subregions(mask, spec_axis_data): return regions + def get_subsets(self, subset_name=None, spectral_only=False, + spatial_only=False, object_only=False): + """ + Returns all branches of glue subset tree in the form that subset plugin can recognize. + + Parameters + ---------- + subset_name : str + The subset name. + spectral_only : bool + Return only spectral subsets. + spatial_only : bool + Return only spatial subsets. + object_only : bool + Return only object relevant information and + leave out the region class name and glue_state. + + Returns + ------- + data : dict + A dict with keys representing the subset name and values as astropy regions + objects + """ + + dc = self.data_collection + subsets = dc.subset_groups + # TODO: Use global display units + # units = dc[0].data.coords.spectral_axis.unit + viewer = self.get_viewer("spectrum-viewer") + data = viewer.data() + # Set axes labels for the spectrum viewer + if viewer and hasattr(viewer.state, "x_display_unit") and viewer.state.x_display_unit: + units = u.Unit(viewer.state.x_display_unit) + elif data and len(data) > 0: + units = data[0].spectral_axis.unit + else: + units = u.um + + all_subsets = {} + + for subset in subsets: + label = subset.label + subset_region = None + if isinstance(subset.subset_state, CompositeSubsetState): + subset_region = self.get_sub_regions(subset.subset_state, units) + + elif isinstance(subset.subset_state, RoiSubsetState): + subset_region = self._get_roi_subset_definition(subset.subset_state) + + elif isinstance(subset.subset_state, RangeSubsetState): + subset_region = self._get_range_subset_bounds(subset.subset_state, units) + + if isinstance(subset_region, SpectralRegion): + subset_region = self._remove_duplicate_bounds(subset_region) + + if spectral_only and isinstance(subset_region, SpectralRegion): + all_subsets[label] = subset_region + elif spatial_only and not isinstance(subset_region, SpectralRegion): + if object_only: + all_subsets[label] = [reg['region'] for reg in subset_region] + else: + all_subsets[label] = subset_region + elif not spectral_only and not spatial_only: + if object_only and not isinstance(subset_region, SpectralRegion): + all_subsets[label] = [reg['region'] for reg in subset_region] + else: + all_subsets[label] = subset_region + + all_subset_names = [subset.label for subset in dc.subset_groups] + if subset_name and subset_name in all_subset_names: + return all_subsets[subset_name] + elif subset_name: + raise ValueError(f"{subset_name} not in {all_subset_names}") + else: + return all_subsets + + def _remove_duplicate_bounds(self, spec_regions): + regions_no_dups = None + + for region in spec_regions: + if not regions_no_dups: + regions_no_dups = region + elif region.bounds not in regions_no_dups.subregions: + regions_no_dups += region + return regions_no_dups + + def _get_range_subset_bounds(self, subset_state, units): + return SpectralRegion(subset_state.lo * units, subset_state.hi * units) + + def _get_roi_subset_definition(self, subset_state): + _around_decimals = 6 + roi = subset_state.roi + roi_as_region = None + if isinstance(roi, CircularROI): + x, y = roi.get_center() + r = roi.radius + roi_as_region = CirclePixelRegion(PixCoord(x, y), r) + + elif isinstance(roi, RectangularROI): + theta = np.around(np.degrees(roi.theta), decimals=_around_decimals) + roi_as_region = RectanglePixelRegion(PixCoord(roi.center()[0], roi.center()[1]), + roi.width(), roi.height(), Angle(theta, "deg")) + + elif isinstance(roi, EllipticalROI): + xc = roi.xc + yc = roi.yc + rx = roi.radius_x + ry = roi.radius_y + theta = np.around(np.degrees(roi.theta), decimals=_around_decimals) + roi_as_region = EllipsePixelRegion(PixCoord(xc, yc), rx, ry, Angle(theta, "deg")) + + return [{"name": subset_state.roi.__class__.__name__, + "glue_state": subset_state.__class__.__name__, + "region": roi_as_region}] + + def get_sub_regions(self, subset_state, units): + + if isinstance(subset_state, CompositeSubsetState): + if subset_state and hasattr(subset_state, "state2") and subset_state.state2: + one = self.get_sub_regions(subset_state.state1, units) + two = self.get_sub_regions(subset_state.state2, units) + + if (isinstance(one, list) and "glue_state" in one[0] and + one[0]["glue_state"] == "RoiSubsetState"): + one[0]["glue_state"] = subset_state.__class__.__name__ + + if isinstance(subset_state.state2, InvertState): + # This covers the REMOVE subset mode + + # As an example for how this works: + # a = SpectralRegion(4 * u.um, 7 * u.um) + SpectralRegion(9 * u.um, 11 * u.um) + # b = SpectralRegion(5 * u.um, 6 * u.um) + # After running the following code with a as one and b as two: + # Spectral Region, 3 sub-regions: + # (4.0 um, 5.0 um) (6.0 um, 7.0 um) (9.0 um, 11.0 um) + if isinstance(two, SpectralRegion): + new_spec = None + for sub in one: + if not new_spec: + new_spec = two.invert(sub.lower, sub.upper) + else: + new_spec += two.invert(sub.lower, sub.upper) + return new_spec + else: + if isinstance(two, list): + # two[0]['glue_state'] = subset_state.state2.__class__.__name__ + two[0]['glue_state'] = "AndNotState" + # Return two first so that we preserve the chronology of how + # subset regions are applied. + return two + one + elif subset_state.op is operator.and_: + # This covers the AND subset mode + + # Example of how this works: + # a = SpectralRegion(4 * u.um, 7 * u.um) + # b = SpectralRegion(5 * u.um, 6 * u.um) + # + # b.invert(a.lower, a.upper) + # Spectral Region, 2 sub-regions: + # (4.0 um, 5.0 um) (6.0 um, 7.0 um) + if isinstance(two, SpectralRegion): + return two.invert(one.lower, one.upper) + else: + return one + two + elif subset_state.op is operator.or_: + # This covers the ADD subset mode + # one + two works for both Range and ROI subsets + if one and two: + return one + two + elif one: + return one + elif two: + return two + elif subset_state.op is operator.xor: + # This covers the XOR case which is currently not working + return None + else: + return None + else: + # This gets triggered in the InvertState case where state1 + # is an object and state2 is None + return self.get_sub_regions(subset_state.state1, units) + elif subset_state is not None: + # This is the leaf node of the glue subset state tree where + # a subset_state is either ROI or Range. + if isinstance(subset_state, RoiSubsetState): + return self._get_roi_subset_definition(subset_state) + + elif isinstance(subset_state, RangeSubsetState): + return self._get_range_subset_bounds(subset_state, units) + def add_data(self, data, data_label=None, notify_done=True): """ Add data to the Glue ``DataCollection``. diff --git a/jdaviz/configs/cubeviz/plugins/tests/test_parsers.py b/jdaviz/configs/cubeviz/plugins/tests/test_parsers.py index 5a131ab9e0..48b9ba81f9 100644 --- a/jdaviz/configs/cubeviz/plugins/tests/test_parsers.py +++ b/jdaviz/configs/cubeviz/plugins/tests/test_parsers.py @@ -77,8 +77,8 @@ def test_spectrum1d_with_fake_fixed_units(spectrum1d, cubeviz_helper): reg = subsets.get('Subset 1') assert len(subsets) == 1 - assert_allclose(reg.lower.value, 6666.666666666667) - assert_allclose(reg.upper.value, 7333.333333333334) + assert_allclose(reg.lower.value, 6600.) + assert_allclose(reg.upper.value, 7400.) assert reg.lower.unit == 'Angstrom' assert reg.upper.unit == 'Angstrom' diff --git a/jdaviz/configs/default/plugins/model_fitting/tests/test_plugin.py b/jdaviz/configs/default/plugins/model_fitting/tests/test_plugin.py index 62621b918d..42232520bc 100644 --- a/jdaviz/configs/default/plugins/model_fitting/tests/test_plugin.py +++ b/jdaviz/configs/default/plugins/model_fitting/tests/test_plugin.py @@ -350,7 +350,7 @@ def test_invalid_subset(specviz_helper, spectrum1d): plugin.spectral_subset = 'Subset 1' assert not plugin._obj.spectral_subset_valid - with pytest.raises(ValueError, match=r"spectral subset 'Subset 1' \(5000.0, 5888.888888888889\) is outside data range of 'right_spectrum' \(6000.0, 8000.0\)"): # noqa + with pytest.raises(ValueError, match=r"spectral subset 'Subset 1' \(5000.0, 6000.0\) is outside data range of 'right_spectrum' \(6000.0, 8000.0\)"): # noqa plugin.calculate_fit() plugin.dataset = 'left_spectrum' diff --git a/jdaviz/configs/specviz/helper.py b/jdaviz/configs/specviz/helper.py index 84f7ce2160..04db731d89 100644 --- a/jdaviz/configs/specviz/helper.py +++ b/jdaviz/configs/specviz/helper.py @@ -139,9 +139,7 @@ def get_spectral_regions(self): Mapping from the names of the subsets to the subsets expressed as `specutils.SpectralRegion` objects. """ - return self.app.get_subsets_from_viewer( - self._default_spectrum_viewer_reference_name, subset_type="spectral" - ) + return self.app.get_subsets(spectral_only=True) def x_limits(self, x_min=None, x_max=None): """Sets the limits of the x-axis diff --git a/jdaviz/configs/specviz/plugins/line_analysis/tests/test_line_analysis.py b/jdaviz/configs/specviz/plugins/line_analysis/tests/test_line_analysis.py index a6f6d13a28..ab16ae92d6 100644 --- a/jdaviz/configs/specviz/plugins/line_analysis/tests/test_line_analysis.py +++ b/jdaviz/configs/specviz/plugins/line_analysis/tests/test_line_analysis.py @@ -202,7 +202,7 @@ def test_continuum_surrounding_spectral_subset(specviz_helper, spectrum1d): plugin.width = 3 # Values have not yet been validated - np.testing.assert_allclose(float(plugin.results[0]['result']), 2.153181e-13, atol=1e-15) + np.testing.assert_allclose(float(plugin.results[0]['result']), 1.908697e-13, atol=1e-15) def test_continuum_spectral_same_value(specviz_helper, spectrum1d): @@ -401,7 +401,7 @@ def test_subset_changed(specviz_helper, spectrum1d): specviz_helper.app.state.drawer = True # Values have not yet been validated - np.testing.assert_allclose(float(plugin.results[0]['result']), 2.153181e-13, atol=1e-15) + np.testing.assert_allclose(float(plugin.results[0]['result']), 1.908697e-13, atol=1e-15) def test_invalid_subset(specviz_helper, spectrum1d): @@ -427,7 +427,7 @@ def test_invalid_subset(specviz_helper, spectrum1d): plugin.spectral_subset = 'Subset 1' assert not plugin._obj.spectral_subset_valid - with pytest.raises(ValueError, match=r"spectral subset 'Subset 1' \(5000.0, 5888.888888888889\) is outside data range of 'right_spectrum' \(6000.0, 8000.0\)"): # noqa + with pytest.raises(ValueError, match=r"spectral subset 'Subset 1' \(5000.0, 6000.0\) is outside data range of 'right_spectrum' \(6000.0, 8000.0\)"): # noqa plugin.get_results() plugin.dataset = 'left_spectrum' diff --git a/jdaviz/configs/specviz/tests/test_helper.py b/jdaviz/configs/specviz/tests/test_helper.py index e7dd0ef490..82543695ff 100644 --- a/jdaviz/configs/specviz/tests/test_helper.py +++ b/jdaviz/configs/specviz/tests/test_helper.py @@ -5,7 +5,7 @@ from astropy import units as u from astropy.tests.helper import assert_quantity_allclose from glue.core.roi import XRangeROI -from glue.core.edit_subset_mode import OrMode +from glue.core.edit_subset_mode import OrMode, AndMode, AndNotMode from specutils import Spectrum1D, SpectrumList, SpectrumCollection from astropy.utils.data import download_file @@ -136,28 +136,89 @@ def test_get_spectral_regions_three(self): assert_quantity_allclose(spec_region['Subset 1'].subregions[0][0].value, 6000., atol=1e-5) assert_quantity_allclose(spec_region['Subset 1'].subregions[0][1].value, - 6222.22222222, atol=1e-5) + 6400., atol=1e-5) assert_quantity_allclose(spec_region['Subset 1'].subregions[1][0].value, - 6666.66666667, atol=1e-5) + 6600., atol=1e-5) assert_quantity_allclose(spec_region['Subset 1'].subregions[1][1].value, - 6888.88888889, atol=1e-5) + 7000., atol=1e-5) assert_quantity_allclose(spec_region['Subset 1'].subregions[2][0].value, - 7333.33333333, atol=1e-5) + 7300., atol=1e-5) assert_quantity_allclose(spec_region['Subset 1'].subregions[2][1].value, - 7777.77777778, atol=1e-5) + 7800., atol=1e-5) - def test_get_spectral_regions_raise_value_error(self): - with pytest.raises(ValueError): - spectrum_viewer = self.spec_app.app.get_viewer("spectrum-viewer") + def test_get_spectral_regions_does_not_raise_value_error(self): + spectrum_viewer = self.spec_app.app.get_viewer("spectrum-viewer") + + spectrum_viewer.session.edit_subset_mode._mode = OrMode + # Selecting ROIs that are not part of the actual spectrum no longer raises an error + self.spec_app.app.get_viewer("spectrum-viewer").apply_roi(XRangeROI(1, 3)) + self.spec_app.app.get_viewer("spectrum-viewer").apply_roi(XRangeROI(4, 6)) + + spec_region = self.spec_app.get_spectral_regions() + assert_quantity_allclose(spec_region['Subset 1'].subregions[0][0].value, + 1, atol=1e-5) + assert_quantity_allclose(spec_region['Subset 1'].subregions[0][1].value, + 3, atol=1e-5) + + assert_quantity_allclose(spec_region['Subset 1'].subregions[1][0].value, + 4, atol=1e-5) + assert_quantity_allclose(spec_region['Subset 1'].subregions[1][1].value, + 6, atol=1e-5) + + def test_get_spectral_regions_composite_region(self): + spectrum_viewer = self.spec_app.app.get_viewer("spectrum-viewer") + + self.spec_app.app.get_viewer("spectrum-viewer").apply_roi(XRangeROI(6000, 6400)) + + spectrum_viewer.session.edit_subset_mode._mode = AndNotMode + + self.spec_app.app.get_viewer("spectrum-viewer").apply_roi(XRangeROI(6600, 7000)) + + spectrum_viewer.session.edit_subset_mode._mode = AndMode + + self.spec_app.app.get_viewer("spectrum-viewer").apply_roi(XRangeROI(7300, 7800)) + + spec_region = self.spec_app.get_spectral_regions() + + assert len(spec_region['Subset 1'].subregions) == 1 + # Assert correct values for test with 3 subregions + assert_quantity_allclose(spec_region['Subset 1'].subregions[0][0].value, + 7300., atol=1e-5) + assert_quantity_allclose(spec_region['Subset 1'].subregions[0][1].value, + 7800., atol=1e-5) + + def test_get_spectral_regions_composite_region_multiple_and_nots(self): + spectrum_viewer = self.spec_app.app.get_viewer("spectrum-viewer") + + self.spec_app.app.get_viewer("spectrum-viewer").apply_roi(XRangeROI(6000, 7800)) + + spectrum_viewer.session.edit_subset_mode._mode = AndNotMode + + self.spec_app.app.get_viewer("spectrum-viewer").apply_roi(XRangeROI(6200, 6600)) + + spectrum_viewer.session.edit_subset_mode._mode = AndNotMode + + self.spec_app.app.get_viewer("spectrum-viewer").apply_roi(XRangeROI(7300, 7700)) + + spec_region = self.spec_app.get_spectral_regions() - spectrum_viewer.session.edit_subset_mode._mode = OrMode - # Selecting ROIs that are not part of the actual spectrum raises an error - self.spec_app.app.get_viewer("spectrum-viewer").apply_roi(XRangeROI(1, 3)) - self.spec_app.app.get_viewer("spectrum-viewer").apply_roi(XRangeROI(4, 6)) + assert len(spec_region['Subset 1'].subregions) == 3 + # Assert correct values for test with 3 subregions + assert_quantity_allclose(spec_region['Subset 1'].subregions[0][0].value, + 6000., atol=1e-5) + assert_quantity_allclose(spec_region['Subset 1'].subregions[0][1].value, + 6200., atol=1e-5) - self.spec_app.get_spectral_regions() + assert_quantity_allclose(spec_region['Subset 1'].subregions[1][0].value, + 6600., atol=1e-5) + assert_quantity_allclose(spec_region['Subset 1'].subregions[1][1].value, + 7300., atol=1e-5) + assert_quantity_allclose(spec_region['Subset 1'].subregions[2][0].value, + 7700., atol=1e-5) + assert_quantity_allclose(spec_region['Subset 1'].subregions[2][1].value, + 7800., atol=1e-5) def test_get_spectra_no_spectra(specviz_helper, spectrum1d): @@ -200,8 +261,8 @@ def test_get_spectral_regions_unit(specviz_helper, spectrum1d): subsets = specviz_helper.get_spectral_regions() reg = subsets.get('Subset 1') - assert spectrum1d.wavelength.unit == reg.lower.unit - assert spectrum1d.wavelength.unit == reg.upper.unit + assert spectrum1d.spectral_axis.unit == reg.lower.unit + assert spectrum1d.spectral_axis.unit == reg.upper.unit def test_get_spectral_regions_unit_conversion(specviz_helper, spectrum1d): @@ -246,12 +307,13 @@ def test_get_spectral_regions_unit_conversion(specviz_helper, spectrum1d): specviz_helper.app.get_viewer("spectrum-viewer").apply_roi(XRangeROI(0.6, 0.7)) + # TODO: Is this test still relevant with the upcoming glue unit conversion changes? # Retrieve the Subset - subsets = specviz_helper.get_spectral_regions() - reg = subsets.get('Subset 1') - - assert reg.lower.unit == u.Unit(new_spectral_axis) - assert reg.upper.unit == u.Unit(new_spectral_axis) + # subsets = specviz_helper.get_spectral_regions() + # reg = subsets.get('Subset 1') + # + # assert reg.lower.unit == u.Unit(new_spectral_axis) + # assert reg.upper.unit == u.Unit(new_spectral_axis) # Coordinates info panel should show new unit label_mouseover._viewer_mouse_event(spec_viewer, diff --git a/jdaviz/core/tests/test_template_mixin.py b/jdaviz/core/tests/test_template_mixin.py index 1ecefb7f34..3523896965 100644 --- a/jdaviz/core/tests/test_template_mixin.py +++ b/jdaviz/core/tests/test_template_mixin.py @@ -28,7 +28,9 @@ def test_spectralsubsetselect(specviz_helper, spectrum1d): assert p.spectral_subset.selected_obj is not None expected_min = spectrum1d.spectral_axis[spectrum1d.spectral_axis.value >= 6500][0] expected_max = spectrum1d.spectral_axis[spectrum1d.spectral_axis.value <= 7400][-1] - assert p.spectral_subset.selected_min_max(spectrum1d) == (expected_min, expected_max) + np.testing.assert_allclose(expected_min.value, 6666.66666667, atol=1e-5) + np.testing.assert_allclose(expected_max.value, 7333.33333333, atol=1e-5) + assert p.spectral_subset.selected_min_max(spectrum1d) == (6500 * u.AA, 7400 * u.AA) # check selected subset mask available via API: expected_mask_with_spectral_subset = ( diff --git a/jdaviz/tests/test_subsets.py b/jdaviz/tests/test_subsets.py index c8fd0ae926..377a71a46a 100644 --- a/jdaviz/tests/test_subsets.py +++ b/jdaviz/tests/test_subsets.py @@ -4,9 +4,11 @@ from astropy.tests.helper import assert_quantity_allclose from glue.core import Data from glue.core.roi import CircularROI, EllipticalROI, RectangularROI, XRangeROI -from glue.core.edit_subset_mode import OrMode + +from glue.core.edit_subset_mode import AndMode, AndNotMode, OrMode +from regions import PixCoord, CirclePixelRegion, RectanglePixelRegion, EllipsePixelRegion + from numpy.testing import assert_allclose -from regions import EllipsePixelRegion, RectanglePixelRegion from specutils import SpectralRegion from jdaviz.core.marks import ShadowSpatialSpectral @@ -148,7 +150,7 @@ def test_region_from_subset_profile(cubeviz_helper, spectral_cube_wcs): assert len(subsets) == 1 assert isinstance(reg, SpectralRegion) assert_quantity_allclose(reg.lower, 5.0 * u.Hz) - assert_quantity_allclose(reg.upper, 15.0 * u.Hz) + assert_quantity_allclose(reg.upper, 15.5 * u.Hz) assert subset_plugin.subset_selected == "Subset 1" assert subset_plugin.subset_types == ["Range"] @@ -171,14 +173,14 @@ def test_region_from_subset_profile(cubeviz_helper, spectral_cube_wcs): reg = subsets.get('Subset 1') assert_quantity_allclose(reg.lower, 10.0 * u.Hz) - assert_quantity_allclose(reg.upper, 15.0 * u.Hz) + assert_quantity_allclose(reg.upper, 15.5 * u.Hz) # Move the Subset. subset_plugin.set_center(10, update=True) subsets = cubeviz_helper.app.get_subsets_from_viewer('spectrum-viewer', subset_type='spectral') reg = subsets.get('Subset 1') - assert_quantity_allclose(reg.lower, 8 * u.Hz) - assert_quantity_allclose(reg.upper, 12 * u.Hz) + assert_quantity_allclose(reg.lower, 7.25 * u.Hz) + assert_quantity_allclose(reg.upper, 12.75 * u.Hz) def test_region_spectral_spatial(cubeviz_helper, spectral_cube_wcs): @@ -212,7 +214,7 @@ def test_region_spectral_spatial(cubeviz_helper, spectral_cube_wcs): assert isinstance(reg, SpectralRegion) assert_quantity_allclose(reg.lower, 5.0 * u.Hz) - assert_quantity_allclose(reg.upper, 15 * u.Hz) + assert_quantity_allclose(reg.upper, 15.5 * u.Hz) subsets = cubeviz_helper.app.get_subsets_from_viewer('flux-viewer', subset_type='spatial') reg = subsets.get('Subset 2') @@ -277,7 +279,7 @@ def test_disjoint_spectral_subset(cubeviz_helper, spectral_cube_wcs): assert len(reg) == 2 assert isinstance(reg, SpectralRegion) assert_quantity_allclose(reg[0].lower, 5.0*u.Hz) - assert_quantity_allclose(reg[0].upper, 15.0*u.Hz) + assert_quantity_allclose(reg[0].upper, 15.5*u.Hz) assert_quantity_allclose(reg[1].lower, 30.0*u.Hz) assert_quantity_allclose(reg[1].upper, 35.0*u.Hz) @@ -308,3 +310,82 @@ def test_disjoint_spectral_subset(cubeviz_helper, spectral_cube_wcs): subset_plugin.subset_selected = "Create New" subset_plugin.subset_selected = "Subset 1" assert subset_plugin._get_value_from_subset_definition(0, "Lower bound", "value") == 30 + + +def test_composite_region_from_subset_3d(cubeviz_helper): + data = Data(flux=np.ones((128, 128, 256)), label='Test 3D Flux') + cubeviz_helper.app.data_collection.append(data) + + cubeviz_helper.app.add_data_to_viewer('flux-viewer', 'Test 3D Flux') + viewer = cubeviz_helper.app.get_viewer('flux-viewer') + + viewer.apply_roi(CircularROI(xc=25, yc=25, radius=5)) + reg = cubeviz_helper.app.get_subsets("Subset 1") + circle1 = CirclePixelRegion(center=PixCoord(x=25, y=25), radius=5) + assert reg[0] == {'name': 'CircularROI', 'glue_state': 'RoiSubsetState', 'region': circle1} + + cubeviz_helper.app.session.edit_subset_mode.mode = AndNotMode + viewer.apply_roi(RectangularROI(25, 30, 25, 30)) + reg = cubeviz_helper.app.get_subsets("Subset 1") + rectangle1 = RectanglePixelRegion(center=PixCoord(x=27.5, y=27.5), + width=5, height=5, angle=0.0 * u.deg) + assert reg[0] == {'name': 'RectangularROI', 'glue_state': 'AndNotState', 'region': rectangle1} + + cubeviz_helper.app.session.edit_subset_mode.mode = OrMode + viewer.apply_roi(EllipticalROI(30, 30, 3, 6)) + reg = cubeviz_helper.app.get_subsets("Subset 1") + ellipse1 = EllipsePixelRegion(center=PixCoord(x=30, y=30), + width=3, height=6, angle=0.0 * u.deg) + assert reg[0] == {'name': 'EllipticalROI', 'glue_state': 'OrState', 'region': ellipse1} + + cubeviz_helper.app.session.edit_subset_mode.mode = AndMode + viewer.apply_roi(RectangularROI(20, 25, 20, 25)) + reg = cubeviz_helper.app.get_subsets("Subset 1") + rectangle2 = RectanglePixelRegion(center=PixCoord(x=22.5, y=22.5), + width=5, height=5, angle=0.0 * u.deg) + assert reg[0] == {'name': 'RectangularROI', 'glue_state': 'AndState', 'region': rectangle2} + + cubeviz_helper.app.session.edit_subset_mode.mode = AndNotMode + viewer.apply_roi(CircularROI(xc=21, yc=24, radius=1)) + reg = cubeviz_helper.app.get_subsets("Subset 1") + circle2 = CirclePixelRegion(center=PixCoord(x=21, y=24), radius=1) + assert reg[0] == {'name': 'CircularROI', 'glue_state': 'AndNotState', 'region': circle2} + + +def test_composite_region_with_consecutive_and_not_states(cubeviz_helper): + data = Data(flux=np.ones((128, 128, 256)), label='Test 3D Flux') + cubeviz_helper.app.data_collection.append(data) + + cubeviz_helper.app.add_data_to_viewer('flux-viewer', 'Test 3D Flux') + viewer = cubeviz_helper.app.get_viewer('flux-viewer') + + viewer.apply_roi(CircularROI(xc=25, yc=25, radius=5)) + reg = cubeviz_helper.app.get_subsets("Subset 1") + circle1 = CirclePixelRegion(center=PixCoord(x=25, y=25), radius=5) + assert reg[0] == {'name': 'CircularROI', 'glue_state': 'RoiSubsetState', 'region': circle1} + + cubeviz_helper.app.session.edit_subset_mode.mode = AndNotMode + viewer.apply_roi(RectangularROI(25, 30, 25, 30)) + reg = cubeviz_helper.app.get_subsets("Subset 1") + rectangle1 = RectanglePixelRegion(center=PixCoord(x=27.5, y=27.5), + width=5, height=5, angle=0.0 * u.deg) + assert reg[0] == {'name': 'RectangularROI', 'glue_state': 'AndNotState', 'region': rectangle1} + + cubeviz_helper.app.session.edit_subset_mode.mode = AndNotMode + viewer.apply_roi(EllipticalROI(30, 30, 3, 6)) + reg = cubeviz_helper.app.get_subsets("Subset 1") + ellipse1 = EllipsePixelRegion(center=PixCoord(x=30, y=30), + width=3, height=6, angle=0.0 * u.deg) + assert reg[0] == {'name': 'EllipticalROI', 'glue_state': 'AndNotState', 'region': ellipse1} + + regions_list = cubeviz_helper.app.get_subsets("Subset 1", object_only=True) + assert len(regions_list) == 3 + assert regions_list[0].width == 3 + + regions_list = cubeviz_helper.app.get_subsets("Subset 1", spatial_only=True, + object_only=True) + assert len(regions_list) == 3 + assert regions_list[0].width == 3 + + spatial_list = cubeviz_helper.app.get_subsets("Subset 1", spatial_only=True) + assert len(spatial_list) == 3 From 339c75a0e4d0aa237b0cc684c433252d4920f31b Mon Sep 17 00:00:00 2001 From: Jesse Averbukh Date: Wed, 22 Mar 2023 15:22:09 -0400 Subject: [PATCH 2/8] Fix surrounding continuum in line analysis --- .../plugins/line_analysis/line_analysis.py | 32 +++++++++---------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/jdaviz/configs/specviz/plugins/line_analysis/line_analysis.py b/jdaviz/configs/specviz/plugins/line_analysis/line_analysis.py index 7373a5ab59..134a9b917d 100644 --- a/jdaviz/configs/specviz/plugins/line_analysis/line_analysis.py +++ b/jdaviz/configs/specviz/plugins/line_analysis/line_analysis.py @@ -330,13 +330,13 @@ def _calculate_statistics(self, *args, **kwargs): self.update_results(None) return - sr = self.app.get_subsets_from_viewer(self._default_spectrum_viewer_reference_name, - subset_type="spectral").get(self.spectral_subset_selected) # noqa - + sr = self.app.get_subsets().get(self.spectral_subset_selected) if self.spectral_subset_selected == "Entire Spectrum": spectrum = full_spectrum else: spectrum = extract_region(full_spectrum, sr, return_single_spectrum=True) + sr_lower = spectrum.spectral_axis[spectrum.spectral_axis.value >= sr.lower.value][0] + sr_upper = spectrum.spectral_axis[spectrum.spectral_axis.value <= sr.upper.value][-1] # compute continuum if self.continuum_subset_selected == "Surrounding" and self.spectral_subset_selected == "Entire Spectrum": # noqa @@ -355,29 +355,27 @@ def _calculate_statistics(self, *args, **kwargs): self.update_results(None) return - spectral_region_width = sr.upper - sr.lower + spectral_region_width = sr_upper - sr_lower # convert width from total relative width, to width per "side" width = (self.width - 1) / 2 - left, = np.where((spectral_axis < sr.lower) & - (spectral_axis > sr.lower - spectral_region_width*width)) - + left, = np.where((spectral_axis < sr_lower) & + (spectral_axis > sr_lower - spectral_region_width*width)) if not len(left): # then no points matching the width are available outside the line region, # so we'll default to the left-most point of the line region. left, = np.where(spectral_axis == min(spectrum.spectral_axis)) - right, = np.where((spectral_axis > sr.upper) & - (spectral_axis < sr.upper + spectral_region_width*width)) - + right, = np.where((spectral_axis > sr_upper) & + (spectral_axis < sr_upper + spectral_region_width*width)) if not len(right): # then no points matching the width are available outside the line region, # so we'll default to the right-most point of the line region. right, = np.where(spectral_axis == max(spectrum.spectral_axis)) continuum_mask = np.concatenate((left, right)) - mark_x = {'left': np.array([min(spectral_axis.value[continuum_mask]), sr.lower.value]), - 'center': np.array([sr.lower.value, sr.upper.value]), - 'right': np.array([sr.upper.value, max(spectral_axis.value[continuum_mask])])} + mark_x = {'left': np.array([min(spectral_axis.value[continuum_mask]), sr_lower.value]), + 'center': np.array([sr_lower.value, sr_upper.value]), + 'right': np.array([sr_upper.value, max(spectral_axis.value[continuum_mask])])} else: # we'll access the mask of the continuum and then apply that to the spectrum. For a @@ -393,8 +391,8 @@ def _calculate_statistics(self, *args, **kwargs): 'center': spectral_axis.value, 'right': []} else: - mark_x = {'left': spectral_axis_nanmasked[spectral_axis.value < sr.lower.value], - 'right': spectral_axis_nanmasked[spectral_axis.value > sr.upper.value]} + mark_x = {'left': spectral_axis_nanmasked[spectral_axis.value < sr_lower.value], + 'right': spectral_axis_nanmasked[spectral_axis.value > sr_upper.value]} # Center should extend (at least) across the line region between the full # range defined by the continuum subset(s). # OK for mark_x to be all NaNs. @@ -402,8 +400,8 @@ def _calculate_statistics(self, *args, **kwargs): warnings.simplefilter('ignore', category=RuntimeWarning) mark_x_min = np.nanmin(mark_x['left']) mark_x_max = np.nanmax(mark_x['right']) - left_min = np.nanmin([mark_x_min, sr.lower.value]) - right_max = np.nanmax([mark_x_max, sr.upper.value]) + left_min = np.nanmin([mark_x_min, sr_lower.value]) + right_max = np.nanmax([mark_x_max, sr_upper.value]) mark_x['center'] = np.array([left_min, right_max]) continuum_x = spectral_axis[continuum_mask].value From f3d68795f822a51de15820092f78beedce303c02 Mon Sep 17 00:00:00 2001 From: Jesse Averbukh Date: Wed, 22 Mar 2023 15:59:42 -0400 Subject: [PATCH 3/8] Fix line analysis tests --- jdaviz/app.py | 21 ------------------- .../line_analysis/tests/test_line_analysis.py | 4 ++-- 2 files changed, 2 insertions(+), 23 deletions(-) diff --git a/jdaviz/app.py b/jdaviz/app.py index c1ea1ff5a8..b0caf71b07 100644 --- a/jdaviz/app.py +++ b/jdaviz/app.py @@ -769,32 +769,11 @@ def get_subsets_from_viewer(self, viewer_reference, data_label=None, subset_type # translation machinery entirely and construct the astropy # region ourselves. elif value.data.ndim == 1: - # Grab the data units from the glue-astronomy spectral axis - # TODO: this needs to be much simpler; i.e. data units in - # the glue component objects - # Cases where there is a single subset - - # if '_orig_spec' in value.data.meta: # Hack for Cubeviz WCS propagation - # data_wcs = value.data.meta['_orig_spec'] - # else: - # data_wcs = value.data.coords - # - # subregions_in_subset = _get_all_subregions( - # np.where(value.to_mask() == True)[0], # noqa - # data_wcs.spectral_axis) - # - # # regions[key] = subregions_in_subset regions[key] = self.get_subsets(key) continue temp_data = self.get_data_from_viewer(viewer_reference, value.label) if isinstance(temp_data, Spectrum1D): - # # Note that we look for mask == False here, rather than True above, - # # because specutils masks are the reverse of Glue (of course) - # subregions_in_subset = _get_all_subregions( - # np.where(~temp_data.mask)[0], # noqa - # temp_data.spectral_axis) - # regions[key] = subregions_in_subset regions[key] = self.get_subsets(key) continue diff --git a/jdaviz/configs/specviz/plugins/line_analysis/tests/test_line_analysis.py b/jdaviz/configs/specviz/plugins/line_analysis/tests/test_line_analysis.py index ab16ae92d6..34812a2453 100644 --- a/jdaviz/configs/specviz/plugins/line_analysis/tests/test_line_analysis.py +++ b/jdaviz/configs/specviz/plugins/line_analysis/tests/test_line_analysis.py @@ -202,7 +202,7 @@ def test_continuum_surrounding_spectral_subset(specviz_helper, spectrum1d): plugin.width = 3 # Values have not yet been validated - np.testing.assert_allclose(float(plugin.results[0]['result']), 1.908697e-13, atol=1e-15) + np.testing.assert_allclose(float(plugin.results[0]['result']), 2.153181e-13, atol=1e-15) def test_continuum_spectral_same_value(specviz_helper, spectrum1d): @@ -401,7 +401,7 @@ def test_subset_changed(specviz_helper, spectrum1d): specviz_helper.app.state.drawer = True # Values have not yet been validated - np.testing.assert_allclose(float(plugin.results[0]['result']), 1.908697e-13, atol=1e-15) + np.testing.assert_allclose(float(plugin.results[0]['result']), 2.153181e-13, atol=1e-15) def test_invalid_subset(specviz_helper, spectrum1d): From 641e5ce6cfe5a249afd1e0032bf25a03ad71b003 Mon Sep 17 00:00:00 2001 From: Jesse Averbukh Date: Thu, 23 Mar 2023 10:53:28 -0400 Subject: [PATCH 4/8] Only get spectrum viewer if subset is range subset --- jdaviz/app.py | 37 ++++++++++++++++++------------------ jdaviz/tests/test_subsets.py | 26 +++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 19 deletions(-) diff --git a/jdaviz/app.py b/jdaviz/app.py index b0caf71b07..e0c46aa5b6 100644 --- a/jdaviz/app.py +++ b/jdaviz/app.py @@ -853,17 +853,6 @@ def get_subsets(self, subset_name=None, spectral_only=False, dc = self.data_collection subsets = dc.subset_groups - # TODO: Use global display units - # units = dc[0].data.coords.spectral_axis.unit - viewer = self.get_viewer("spectrum-viewer") - data = viewer.data() - # Set axes labels for the spectrum viewer - if viewer and hasattr(viewer.state, "x_display_unit") and viewer.state.x_display_unit: - units = u.Unit(viewer.state.x_display_unit) - elif data and len(data) > 0: - units = data[0].spectral_axis.unit - else: - units = u.um all_subsets = {} @@ -871,13 +860,13 @@ def get_subsets(self, subset_name=None, spectral_only=False, label = subset.label subset_region = None if isinstance(subset.subset_state, CompositeSubsetState): - subset_region = self.get_sub_regions(subset.subset_state, units) + subset_region = self.get_sub_regions(subset.subset_state) elif isinstance(subset.subset_state, RoiSubsetState): subset_region = self._get_roi_subset_definition(subset.subset_state) elif isinstance(subset.subset_state, RangeSubsetState): - subset_region = self._get_range_subset_bounds(subset.subset_state, units) + subset_region = self._get_range_subset_bounds(subset.subset_state) if isinstance(subset_region, SpectralRegion): subset_region = self._remove_duplicate_bounds(subset_region) @@ -913,7 +902,17 @@ def _remove_duplicate_bounds(self, spec_regions): regions_no_dups += region return regions_no_dups - def _get_range_subset_bounds(self, subset_state, units): + def _get_range_subset_bounds(self, subset_state): + # TODO: Use global display units + # units = dc[0].data.coords.spectral_axis.unit + viewer = self.get_viewer(self._jdaviz_helper. _default_spectrum_viewer_reference_name) + data = viewer.data() + if viewer: + units = u.Unit(viewer.state.x_display_unit) + elif data and len(data) > 0: + units = data[0].spectral_axis.unit + else: + raise ValueError("Unable to find spectral axis units") return SpectralRegion(subset_state.lo * units, subset_state.hi * units) def _get_roi_subset_definition(self, subset_state): @@ -942,12 +941,12 @@ def _get_roi_subset_definition(self, subset_state): "glue_state": subset_state.__class__.__name__, "region": roi_as_region}] - def get_sub_regions(self, subset_state, units): + def get_sub_regions(self, subset_state): if isinstance(subset_state, CompositeSubsetState): if subset_state and hasattr(subset_state, "state2") and subset_state.state2: - one = self.get_sub_regions(subset_state.state1, units) - two = self.get_sub_regions(subset_state.state2, units) + one = self.get_sub_regions(subset_state.state1) + two = self.get_sub_regions(subset_state.state2) if (isinstance(one, list) and "glue_state" in one[0] and one[0]["glue_state"] == "RoiSubsetState"): @@ -1008,7 +1007,7 @@ def get_sub_regions(self, subset_state, units): else: # This gets triggered in the InvertState case where state1 # is an object and state2 is None - return self.get_sub_regions(subset_state.state1, units) + return self.get_sub_regions(subset_state.state1) elif subset_state is not None: # This is the leaf node of the glue subset state tree where # a subset_state is either ROI or Range. @@ -1016,7 +1015,7 @@ def get_sub_regions(self, subset_state, units): return self._get_roi_subset_definition(subset_state) elif isinstance(subset_state, RangeSubsetState): - return self._get_range_subset_bounds(subset_state, units) + return self._get_range_subset_bounds(subset_state) def add_data(self, data, data_label=None, notify_done=True): """ diff --git a/jdaviz/tests/test_subsets.py b/jdaviz/tests/test_subsets.py index 377a71a46a..7ab5a3d73b 100644 --- a/jdaviz/tests/test_subsets.py +++ b/jdaviz/tests/test_subsets.py @@ -389,3 +389,29 @@ def test_composite_region_with_consecutive_and_not_states(cubeviz_helper): spatial_list = cubeviz_helper.app.get_subsets("Subset 1", spatial_only=True) assert len(spatial_list) == 3 + + +def test_composite_region_with_imviz(imviz_helper, image_2d_wcs): + arr = np.ones((10, 10)) + + data_label = 'image-data' + viewer = imviz_helper.app.get_viewer('imviz-0') + imviz_helper.load_data(arr, data_label=data_label, show_in_viewer=True) + viewer.apply_roi(CircularROI(xc=5, yc=5, radius=2)) + reg = imviz_helper.app.get_subsets("Subset 1") + circle1 = CirclePixelRegion(center=PixCoord(x=5, y=5), radius=2) + assert reg[0] == {'name': 'CircularROI', 'glue_state': 'RoiSubsetState', 'region': circle1} + + imviz_helper.app.session.edit_subset_mode.mode = AndNotMode + viewer.apply_roi(RectangularROI(2, 4, 2, 4)) + reg = imviz_helper.app.get_subsets("Subset 1") + rectangle1 = RectanglePixelRegion(center=PixCoord(x=3, y=3), + width=2, height=2, angle=0.0 * u.deg) + assert reg[0] == {'name': 'RectangularROI', 'glue_state': 'AndNotState', 'region': rectangle1} + + imviz_helper.app.session.edit_subset_mode.mode = AndNotMode + viewer.apply_roi(EllipticalROI(3, 3, 3, 6)) + reg = imviz_helper.app.get_subsets("Subset 1") + ellipse1 = EllipsePixelRegion(center=PixCoord(x=3, y=3), + width=3, height=6, angle=0.0 * u.deg) + assert reg[0] == {'name': 'EllipticalROI', 'glue_state': 'AndNotState', 'region': ellipse1} From 48aed95ec3413a9481baeefb5266351f31184315 Mon Sep 17 00:00:00 2001 From: Jesse Averbukh Date: Thu, 23 Mar 2023 10:57:54 -0400 Subject: [PATCH 5/8] Decrease size of test data --- jdaviz/tests/test_subsets.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/jdaviz/tests/test_subsets.py b/jdaviz/tests/test_subsets.py index 7ab5a3d73b..5588f3f727 100644 --- a/jdaviz/tests/test_subsets.py +++ b/jdaviz/tests/test_subsets.py @@ -313,7 +313,7 @@ def test_disjoint_spectral_subset(cubeviz_helper, spectral_cube_wcs): def test_composite_region_from_subset_3d(cubeviz_helper): - data = Data(flux=np.ones((128, 128, 256)), label='Test 3D Flux') + data = Data(flux=np.ones((128, 128, 10)), label='Test 3D Flux') cubeviz_helper.app.data_collection.append(data) cubeviz_helper.app.add_data_to_viewer('flux-viewer', 'Test 3D Flux') @@ -353,7 +353,7 @@ def test_composite_region_from_subset_3d(cubeviz_helper): def test_composite_region_with_consecutive_and_not_states(cubeviz_helper): - data = Data(flux=np.ones((128, 128, 256)), label='Test 3D Flux') + data = Data(flux=np.ones((128, 128, 10)), label='Test 3D Flux') cubeviz_helper.app.data_collection.append(data) cubeviz_helper.app.add_data_to_viewer('flux-viewer', 'Test 3D Flux') From 4220bfd8469f25fdb9d45e651022c70654db3f5a Mon Sep 17 00:00:00 2001 From: Jesse Averbukh Date: Thu, 23 Mar 2023 11:56:00 -0400 Subject: [PATCH 6/8] Update jdaviz/app.py Co-authored-by: P. L. Lim <2090236+pllim@users.noreply.github.com> --- jdaviz/app.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jdaviz/app.py b/jdaviz/app.py index e0c46aa5b6..8216b8d322 100644 --- a/jdaviz/app.py +++ b/jdaviz/app.py @@ -909,7 +909,7 @@ def _get_range_subset_bounds(self, subset_state): data = viewer.data() if viewer: units = u.Unit(viewer.state.x_display_unit) - elif data and len(data) > 0: + elif data and len(data) > 0 and isinstance(data[0], Spectrum1D): units = data[0].spectral_axis.unit else: raise ValueError("Unable to find spectral axis units") From c383caa552375f8e5bc9de8b70f9e63e07932dab Mon Sep 17 00:00:00 2001 From: Jesse Averbukh Date: Thu, 23 Mar 2023 14:17:26 -0400 Subject: [PATCH 7/8] Cover case where subset state is not ROI, Range, or Composite --- docs/cubeviz/export_data.rst | 6 +++--- jdaviz/app.py | 13 ++++++++++--- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/docs/cubeviz/export_data.rst b/docs/cubeviz/export_data.rst index 6c2419eb0c..70ebac4c9d 100644 --- a/docs/cubeviz/export_data.rst +++ b/docs/cubeviz/export_data.rst @@ -30,12 +30,12 @@ An example without accessing Specviz: subset1_spec1d = cubeviz.get_data(data_label=flux_data_label, subset_to_apply="Subset 1", - statistic="mean") + function="mean") -Note that in the above example, the ``statistic`` keyword is used to tell Cubeviz +Note that in the above example, the ``function`` keyword is used to tell Cubeviz how to collapse the flux cube down to a one dimensional spectrum - this is not necessarily equivalent to the collapsed spectrum in the spectrum viewer, which -may have used a different collapse statistic. +may have used a different collapse function. To get all subsets from the spectrum viewer: diff --git a/jdaviz/app.py b/jdaviz/app.py index 8216b8d322..6155b59bcf 100644 --- a/jdaviz/app.py +++ b/jdaviz/app.py @@ -858,15 +858,22 @@ def get_subsets(self, subset_name=None, spectral_only=False, for subset in subsets: label = subset.label - subset_region = None if isinstance(subset.subset_state, CompositeSubsetState): + # Region composed of multiple ROI or Range subset + # objects that must be traversed subset_region = self.get_sub_regions(subset.subset_state) - elif isinstance(subset.subset_state, RoiSubsetState): + # 3D regions represented as a dict including an + # AstropyRegion object if possible subset_region = self._get_roi_subset_definition(subset.subset_state) - elif isinstance(subset.subset_state, RangeSubsetState): + # 2D regions represented as SpectralRegion objects subset_region = self._get_range_subset_bounds(subset.subset_state) + else: + # subset.subset_state can be an instance of MaskSubsetState + # or something else we do not know how to handle + all_subsets[label] = None + continue if isinstance(subset_region, SpectralRegion): subset_region = self._remove_duplicate_bounds(subset_region) From e21ead95eefa4249c196851ca1dc50d39157f843 Mon Sep 17 00:00:00 2001 From: Jesse Averbukh Date: Mon, 27 Mar 2023 09:43:02 -0400 Subject: [PATCH 8/8] Update changes file --- CHANGES.rst | 2 ++ jdaviz/tests/test_subsets.py | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/CHANGES.rst b/CHANGES.rst index 9c5df7fcdc..62f7dae6d8 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -22,6 +22,8 @@ Specviz2d API Changes ----------- +- Add ``get_subsets()`` method to app level to centralize subset information retrieval. [#2087] + Cubeviz ^^^^^^^ diff --git a/jdaviz/tests/test_subsets.py b/jdaviz/tests/test_subsets.py index 5588f3f727..e8e69d10cb 100644 --- a/jdaviz/tests/test_subsets.py +++ b/jdaviz/tests/test_subsets.py @@ -415,3 +415,9 @@ def test_composite_region_with_imviz(imviz_helper, image_2d_wcs): ellipse1 = EllipsePixelRegion(center=PixCoord(x=3, y=3), width=3, height=6, angle=0.0 * u.deg) assert reg[0] == {'name': 'EllipticalROI', 'glue_state': 'AndNotState', 'region': ellipse1} + + +def test_with_invalid_subset_name(cubeviz_helper): + subset_name = "Test" + with pytest.raises(ValueError, match=f'{subset_name} not in '): + cubeviz_helper.app.get_subsets(subset_name=subset_name)