Repository: spacetelescope/jdaviz

Issue:
Aperture photometry: if delete all subsets, no more access to table in the GUI
**Reporter: Camilla Pacifici**

Create a subset, use simple aperture photometry on subset, entry is in table.
Delete subset, table disappears.
Create a new subset, table is back with all appropriate rows.

It would be useful to be able to see the table in GUI even when all subsets have been deleted.

 --- 

 DISCLAIMER: This issue was autocreated by the Jdaviz Issue Creation Bot on behalf of the reporter. If any information is incorrect, please contact [Duy Nguyen](https://github.com/duytnguyendtn)

Discussion:
codecov[bot]: ## [Codecov](https://codecov.io/gh/spacetelescope/jdaviz/pull/2112?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope) Report
Patch coverage: **`100.00`**% and project coverage change: **`+0.01`** :tada:
> Comparison is base [(`5510486`)](https://codecov.io/gh/spacetelescope/jdaviz/commit/5510486a3a47adb9b955de26747d3662b77fbf9d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope) 91.90% compared to head [(`0e7f50a`)](https://codecov.io/gh/spacetelescope/jdaviz/pull/2112?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope) 91.91%.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main    #2112      +/-   ##
==========================================
+ Coverage   91.90%   91.91%   +0.01%     
==========================================
  Files         143      143              
  Lines       15605    15594      -11     
==========================================
- Hits        14342    14334       -8     
+ Misses       1263     1260       -3     
```


| [Impacted Files](https://codecov.io/gh/spacetelescope/jdaviz/pull/2112?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope) | Coverage Δ | |
|---|---|---|
| [...imviz/plugins/aper\_phot\_simple/aper\_phot\_simple.py](https://codecov.io/gh/spacetelescope/jdaviz/pull/2112?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope#diff-amRhdml6L2NvbmZpZ3MvaW12aXovcGx1Z2lucy9hcGVyX3Bob3Rfc2ltcGxlL2FwZXJfcGhvdF9zaW1wbGUucHk=) | `92.83% <100.00%> (+0.61%)` | :arrow_up: |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope)

</details>

[:umbrella: View full report in Codecov by Sentry](https://codecov.io/gh/spacetelescope/jdaviz/pull/2112?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope).

kecnry: @pllim - I originally was just going to move the table out of the `v-if`, but then realized that we probably want everything to act the same and just keep showing until there are new results.  Can you think of any workflow/case where this isn't the intended/desired behavior?

And I'm not sure how to milestone or where to put the changelog - does this count as a bugfix or a change in behavior?
pllim: Will not clearing get confusing at some point? You have result from previous run, but then unsuccessful but don't bother reading the snackbar. Now you still see numbers and you are in a rush, won't you mistaken the numbers for the new run?

Patch:
diff --git a/CHANGES.rst b/CHANGES.rst
index 9c5df7fc..48b82e9d 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -4,6 +4,8 @@
 New Features
 ------------
 
+* Model fitting results are logged in a table within the plugin [#2093].
+
 Cubeviz
 ^^^^^^^
 
@@ -22,6 +24,8 @@ Specviz2d
 API Changes
 -----------
 
+- Add ``get_subsets()`` method to app level to centralize subset information retrieval. [#2087]
+
 Cubeviz
 ^^^^^^^
 
@@ -70,6 +74,9 @@ Cubeviz
 Imviz
 ^^^^^
 
+* Do not hide previous results in aperture photometry when there is a failure, but rather show
+  the failure message within the plugin UI to indicate the shown results are "out of date". [#2112]
+
 Mosviz
 ^^^^^^
 
diff --git a/README.rst b/README.rst
index a934cbbc..7e881ea4 100644
--- a/README.rst
+++ b/README.rst
@@ -96,10 +96,14 @@ The power of ``jdaviz`` is that it can integrated into your Jupyter notebook wor
     from jdaviz import Specviz
 
     specviz = Specviz()
-    specviz.app
+    specviz.show()
 
 To learn more about the various ``jdaviz`` application configurations and loading data, see the
-`specviz <https://jdaviz.readthedocs.io/en/latest/specviz/import_data.html>`_, `cubeviz <https://jdaviz.readthedocs.io/en/latest/cubeviz/import_data.html>`_, `mosviz <https://jdaviz.readthedocs.io/en/latest/mosviz/import_data.html>`_, or `imviz <https://jdaviz.readthedocs.io/en/latest/imviz/import_data.html>`_ tools.
+`Specviz <https://jdaviz.readthedocs.io/en/latest/specviz/import_data.html>`_,
+`Cubeviz <https://jdaviz.readthedocs.io/en/latest/cubeviz/import_data.html>`_,
+`Mosviz <https://jdaviz.readthedocs.io/en/latest/mosviz/import_data.html>`_,
+`Imviz <https://jdaviz.readthedocs.io/en/latest/imviz/import_data.html>`_,
+or `Specviz2D <https://jdaviz.readthedocs.io/en/latest/specviz2d/import_data.html>`_ tools.
 
 ``jdaviz`` also provides a directory of `sample notebooks <https://jdaviz.readthedocs.io/en/latest/sample_notebooks.html>`_ to test the application, located in the ``notebooks`` sub-directory
 of the git repository.  ``CubevizExample.ipynb`` is provided as an example that loads a JWST data cube with the
@@ -132,7 +136,7 @@ Cite ``jdaviz`` via our Zenodo record: https://doi.org/10.5281/zenodo.5513927.
 Contributing
 ------------
 
-We love contributions! jdaviz is open source,
+We love contributions! ``jdaviz`` is open source,
 built on open source, and we'd love to have you hang out in our community.
 
 **Imposter syndrome disclaimer**: We want your help. No, really.
diff --git a/docs/cubeviz/export_data.rst b/docs/cubeviz/export_data.rst
index 6c2419eb..70ebac4c 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/docs/specviz/export_data.rst b/docs/specviz/export_data.rst
index f7c0fab6..f87e74fc 100644
--- a/docs/specviz/export_data.rst
+++ b/docs/specviz/export_data.rst
@@ -96,6 +96,17 @@ To extract all of the model parameters:
     myparams
 
 where the ``model_label`` parameter identifies which model should be returned.
+
+Alternatively, the table of logged parameter values in the model fitting plugin can be exported to
+an :ref:`astropy table <astropy:astropy-table>`
+by calling :meth:`~jdaviz.core.template_mixin.TableMixin.export_table` (see :ref:`plugin-apis`):
+
+.. code-block:: python
+
+    model_fitting = specviz.plugins['Model Fitting']
+    model_fitting.export_table()
+
+
 .. _specviz-export-markers:
 
 Markers Table
diff --git a/docs/specviz/plugins.rst b/docs/specviz/plugins.rst
index c824c0ca..e6ed9097 100644
--- a/docs/specviz/plugins.rst
+++ b/docs/specviz/plugins.rst
@@ -103,6 +103,11 @@ show the fitted value of each parameter rather than the initial value, and
 will additionally show the standard deviation uncertainty of the fitted
 parameter value if the parameter was not set to be fixed to the initial value.
 
+Parameter values for each fitting run are stored in the plugin table.  
+To export the table into the notebook via the API, call
+:meth:`~jdaviz.core.template_mixin.TableMixin.export_table`
+(see :ref:`plugin-apis`).
+
 .. seealso::
 
     :ref:`Export Models <specviz-export-model>`
diff --git a/jdaviz/app.py b/jdaviz/app.py
index 132ebec1..6155b59b 100644
--- a/jdaviz/app.py
+++ b/jdaviz/app.py
@@ -4,12 +4,16 @@ import re
 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 @@ from glue.core.message import (DataCollectionAddMessage,
                                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 @@ class Application(VuetifyTemplate, HubListener):
                                          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}
 
@@ -811,30 +769,12 @@ class Application(VuetifyTemplate, HubListener):
                 #  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
 
                 # Get the pixel coordinate [z] of the 3D data, repeating the
@@ -887,6 +827,203 @@ class Application(VuetifyTemplate, HubListener):
 
         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
+
+        all_subsets = {}
+
+        for subset in subsets:
+            label = subset.label
+            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)
+
+            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):
+        # 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 and isinstance(data[0], Spectrum1D):
+            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):
+        _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):
+
+        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)
+                two = self.get_sub_regions(subset_state.state2)
+
+                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)
+        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)
+
     def add_data(self, data, data_label=None, notify_done=True):
         """
         Add data to the Glue ``DataCollection``.
diff --git a/jdaviz/configs/default/plugins/markers/markers.py b/jdaviz/configs/default/plugins/markers/markers.py
index 30ed4655..ed06f009 100644
--- a/jdaviz/configs/default/plugins/markers/markers.py
+++ b/jdaviz/configs/default/plugins/markers/markers.py
@@ -72,6 +72,7 @@ class Markers(PluginTemplateMixin, ViewerSelectMixin, TableMixin):
 
         self.table.headers_avail = headers
         self.table.headers_visible = headers
+        self.table._default_values_by_colname = _default_table_values
 
         # subscribe to mouse events on any new viewers
         self.hub.subscribe(self, ViewerAddedMessage, handler=self._on_viewer_added)
diff --git a/jdaviz/configs/default/plugins/model_fitting/model_fitting.py b/jdaviz/configs/default/plugins/model_fitting/model_fitting.py
index ac2ebb9b..1620db5d 100644
--- a/jdaviz/configs/default/plugins/model_fitting/model_fitting.py
+++ b/jdaviz/configs/default/plugins/model_fitting/model_fitting.py
@@ -17,7 +17,8 @@ from jdaviz.core.template_mixin import (PluginTemplateMixin,
                                         DatasetSelectMixin,
                                         DatasetSpectralSubsetValidMixin,
                                         AutoTextField,
-                                        AddResultsMixin)
+                                        AddResultsMixin,
+                                        TableMixin)
 from jdaviz.core.custom_traitlets import IntHandleEmpty
 from jdaviz.core.user_api import PluginUserApi
 from jdaviz.configs.default.plugins.model_fitting.fitting_backend import fit_model_to_spectrum
@@ -39,7 +40,7 @@ class _EmptyParam:
 @tray_registry('g-model-fitting', label="Model Fitting", viewer_requirements='spectrum')
 class ModelFitting(PluginTemplateMixin, DatasetSelectMixin,
                    SpectralSubsetSelectMixin, DatasetSpectralSubsetValidMixin,
-                   AddResultsMixin):
+                   AddResultsMixin, TableMixin):
     """
     See the :ref:`Model Fitting Plugin Documentation <specviz-model-fitting>` for more details.
 
@@ -63,6 +64,7 @@ class ModelFitting(PluginTemplateMixin, DatasetSelectMixin,
     * :meth:`set_model_component`
     * :meth:`reestimate_model_parameters`
     * ``equation`` (:class:`~jdaviz.core.template_mixin.AutoTextField`)
+    * :meth:`equation_components`
     * ``cube_fit``
       Only exposed for Cubeviz.  Whether to fit the model to the cube instead of to the
       collapsed spectrum.
@@ -154,6 +156,16 @@ class ModelFitting(PluginTemplateMixin, DatasetSelectMixin,
         self.residuals = AutoTextField(self, 'residuals_label', 'residuals_label_default',
                                        'residuals_label_auto', 'residuals_label_invalid_msg')
 
+        headers = ['model', 'data_label', 'spectral_subset', 'equation']
+        if self.config == 'cubeviz':
+            headers += ['spatial_subset', 'cube_fit']
+
+        self.table.headers_avail = headers
+        self.table.headers_visible = headers
+        # when model parameters are added as columns, only show the value columns by default
+        # (other columns can be show in the dropdown by the user)
+        self.table._new_col_visible = lambda colname: colname.split(':')[-1] not in ('unit', 'fixed', 'uncert', 'std')  # noqa
+
         # set the filter on the viewer options
         self._update_viewer_filters()
 
@@ -165,10 +177,11 @@ class ModelFitting(PluginTemplateMixin, DatasetSelectMixin,
         expose += ['spectral_subset', 'model_component', 'poly_order', 'model_component_label',
                    'model_components', 'create_model_component', 'remove_model_component',
                    'get_model_component', 'set_model_component', 'reestimate_model_parameters',
-                   'equation', 'add_results', 'residuals_calculate', 'residuals']
+                   'equation', 'equation_components',
+                   'add_results', 'residuals_calculate', 'residuals']
         if self.config == "cubeviz":
             expose += ['cube_fit']
-        expose += ['calculate_fit']
+        expose += ['calculate_fit', 'clear_table', 'export_table']
         return PluginUserApi(self, expose=expose)
 
     def _param_units(self, param, model_type=None):
@@ -531,6 +544,7 @@ class ModelFitting(PluginTemplateMixin, DatasetSelectMixin,
         comp = {"model_type": model_component['model_type'],
                 "parameters": {p['name']: {'value': p['value'],
                                            'unit': p['unit'],
+                                           'std': p.get('std', np.nan),
                                            'fixed': p['fixed']} for p in model_component['parameters']}}  # noqa
 
         if parameter is not None:
@@ -630,6 +644,13 @@ class ModelFitting(PluginTemplateMixin, DatasetSelectMixin,
         """
         return [x["id"] for x in self.component_models]
 
+    @property
+    def equation_components(self):
+        """
+        List of the labels of model components in the current equation
+        """
+        return re.split('[+*/-]', self.equation.value)
+
     def vue_add_model(self, event):
         self.create_model_component()
 
@@ -688,9 +709,39 @@ class ModelFitting(PluginTemplateMixin, DatasetSelectMixin,
             raise ValueError(f"spectral subset '{self.spectral_subset.selected}' {subset_range} is outside data range of '{self.dataset.selected}' {spec_range}")  # noqa
 
         if self.cube_fit:
-            return self._fit_model_to_cube(add_data=add_data)
+            ret = self._fit_model_to_cube(add_data=add_data)
         else:
-            return self._fit_model_to_spectrum(add_data=add_data)
+            ret = self._fit_model_to_spectrum(add_data=add_data)
+
+        if ret is None:  # pragma: no cover
+            # something went wrong in the fitting call and (hopefully) already raised a warning,
+            # but we don't have anything to add to the table
+            return ret
+
+        if self.cube_fit:
+            # cube fits are currently unsupported in tables
+            return ret
+
+        row = {'model': self.results_label if add_data else '',
+               'data_label': self.dataset_selected,
+               'spectral_subset': self.spectral_subset_selected,
+               'equation': self.equation.value}
+        if self.app.config == 'cubeviz':
+            row['spatial_subset'] = self.spatial_subset_selected
+            row['cube_fit'] = self.cube_fit
+
+        equation_components = self.equation_components
+        for comp_ind, comp in enumerate(equation_components):
+            for param_name, param_dict in self.get_model_component(comp).get('parameters', {}).items():  # noqa
+                colprefix = f"{comp}:{param_name}_{comp_ind}"
+                row[colprefix] = param_dict.get('value')
+                row[f"{colprefix}:unit"] = param_dict.get('unit')
+                row[f"{colprefix}:fixed"] = param_dict.get('fixed')
+                row[f"{colprefix}:std"] = param_dict.get('std')
+
+        self.table.add_item(row)
+
+        return ret
 
     def vue_apply(self, event):
         self.calculate_fit()
diff --git a/jdaviz/configs/default/plugins/model_fitting/model_fitting.vue b/jdaviz/configs/default/plugins/model_fitting/model_fitting.vue
index b0a1dd29..e42107cf 100644
--- a/jdaviz/configs/default/plugins/model_fitting/model_fitting.vue
+++ b/jdaviz/configs/default/plugins/model_fitting/model_fitting.vue
@@ -219,6 +219,12 @@
         ></v-switch>
       </v-row>
 
+      <v-row v-if="cube_fit">
+        <span class="v-messages v-messages__message text--secondary">
+            Note: cube fit results are not logged to table.
+        </span>
+      </v-row>
+
       <plugin-add-results
         :label.sync="results_label"
         :label_default="results_label_default"
@@ -267,6 +273,9 @@
             If fit is not sufficiently converged, click Fit Model again to run additional iterations.
         </span>
       </v-row>
+
+      <j-plugin-section-header>Results History</j-plugin-section-header>
+      <jupyter-widget :widget="table_widget"></jupyter-widget> 
     </div>
   </j-tray-plugin>
 </template>
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 d9994f33..9616567d 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
@@ -45,6 +45,7 @@ class SimpleAperturePhotometry(PluginTemplateMixin, DatasetSelectMixin, TableMix
     counts_factor = Any(0).tag(sync=True)
     flux_scaling = Any(0).tag(sync=True)
     result_available = Bool(False).tag(sync=True)
+    result_failed_msg = Unicode("").tag(sync=True)
     results = List().tag(sync=True)
     plot_types = List([]).tag(sync=True)
     current_plot_type = Unicode().tag(sync=True)
@@ -90,19 +91,11 @@ class SimpleAperturePhotometry(PluginTemplateMixin, DatasetSelectMixin, TableMix
         self.session.hub.subscribe(self, SubsetUpdateMessage, handler=self._on_subset_update)
         self.session.hub.subscribe(self, LinkUpdatedMessage, handler=self._on_link_update)
 
-    def reset_results(self):
-        self.result_available = False
-        self.results = []
-        self.plot_available = False
-        self.radial_plot = ''
-        bqplot_clear_figure(self._fig)
-
     @observe('dataset_selected')
     def _dataset_selected_changed(self, event={}):
         try:
             self._selected_data = self.dataset.selected_dc_item
             if self._selected_data is None:
-                self.reset_results()
                 return
             self.counts_factor = 0
             self.pixel_area = 0
@@ -143,7 +136,6 @@ class SimpleAperturePhotometry(PluginTemplateMixin, DatasetSelectMixin, TableMix
                     self.pixel_area = 0.04 * 0.04
 
         except Exception as e:
-            self.reset_results()
             self._selected_data = None
             self.hub.broadcast(SnackbarMessage(
                 f"Failed to extract {self.dataset_selected}: {repr(e)}",
@@ -174,7 +166,6 @@ class SimpleAperturePhotometry(PluginTemplateMixin, DatasetSelectMixin, TableMix
     def _subset_selected_changed(self, event={}):
         subset_selected = event.get('new', self.subset_selected)
         if self._selected_data is None or subset_selected == '':
-            self.reset_results()
             return
 
         try:
@@ -196,7 +187,6 @@ class SimpleAperturePhotometry(PluginTemplateMixin, DatasetSelectMixin, TableMix
 
         except Exception as e:
             self._selected_subset = None
-            self.reset_results()
             self.hub.broadcast(SnackbarMessage(
                 f"Failed to extract {subset_selected}: {repr(e)}", color='error', sender=self))
 
@@ -249,7 +239,6 @@ class SimpleAperturePhotometry(PluginTemplateMixin, DatasetSelectMixin, TableMix
 
     def vue_do_aper_phot(self, *args, **kwargs):
         if self._selected_data is None or self._selected_subset is None:
-            self.reset_results()
             self.hub.broadcast(SnackbarMessage(
                 "No data for aperture photometry", color='error', sender=self))
             return
@@ -430,14 +419,15 @@ class SimpleAperturePhotometry(PluginTemplateMixin, DatasetSelectMixin, TableMix
                 else:
                     bqplot_marks = [bqplot_line]
 
-            self._fig.marks = bqplot_marks
-
         except Exception as e:  # pragma: no cover
-            self.reset_results()
-            self.hub.broadcast(SnackbarMessage(
-                f"Aperture photometry failed: {repr(e)}", color='error', sender=self))
-
+            bqplot_clear_figure(self._fig)
+            msg = f"Aperture photometry failed: {repr(e)}"
+            self.hub.broadcast(SnackbarMessage(msg, color='error', sender=self))
+            self.result_failed_msg = msg
         else:
+            self.result_failed_msg = ''
+            self._fig.marks = bqplot_marks
+
             # Parse results for GUI.
             tmp = []
             for key in phot_table.colnames:
diff --git a/jdaviz/configs/imviz/plugins/aper_phot_simple/aper_phot_simple.vue b/jdaviz/configs/imviz/plugins/aper_phot_simple/aper_phot_simple.vue
index fb49bba3..690ef28c 100644
--- a/jdaviz/configs/imviz/plugins/aper_phot_simple/aper_phot_simple.vue
+++ b/jdaviz/configs/imviz/plugins/aper_phot_simple/aper_phot_simple.vue
@@ -139,6 +139,12 @@
       </div>
     </div>
 
+    <v-row v-if="result_failed_msg.length > 0">
+      <span class="v-messages v-messages__message text--secondary" style="color: red !important">
+          <b>WARNING</b>: {{result_failed_msg}}
+      </span>
+    </v-row>
+
     <v-row v-if="plot_available">
       <!-- NOTE: the internal bqplot widget defaults to 480 pixels, so if choosing something else,
            we will likely need to override that with custom CSS rules in order to avoid the initial
diff --git a/jdaviz/configs/specviz/helper.py b/jdaviz/configs/specviz/helper.py
index 84f7ce21..04db731d 100644
--- a/jdaviz/configs/specviz/helper.py
+++ b/jdaviz/configs/specviz/helper.py
@@ -139,9 +139,7 @@ class Specviz(ConfigHelper, LineListMixin):
             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/line_analysis.py b/jdaviz/configs/specviz/plugins/line_analysis/line_analysis.py
index 7373a5ab..134a9b91 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 @@ class LineAnalysis(PluginTemplateMixin, DatasetSelectMixin, SpectralSubsetSelect
             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 @@ class LineAnalysis(PluginTemplateMixin, DatasetSelectMixin, SpectralSubsetSelect
                 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 @@ class LineAnalysis(PluginTemplateMixin, DatasetSelectMixin, SpectralSubsetSelect
                           '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 @@ class LineAnalysis(PluginTemplateMixin, DatasetSelectMixin, SpectralSubsetSelect
                     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
diff --git a/jdaviz/core/template_mixin.py b/jdaviz/core/template_mixin.py
index 7007f6c8..722ddfb7 100644
--- a/jdaviz/core/template_mixin.py
+++ b/jdaviz/core/template_mixin.py
@@ -2293,6 +2293,8 @@ class Table(PluginSubcomponent):
     """
     template_file = __file__, "../components/plugin_table.vue"
 
+    _default_values_by_colname = {}
+
     headers_visible = List([]).tag(sync=True)  # list of strings
     headers_avail = List([]).tag(sync=True)   # list of strings
     items = List().tag(sync=True)  # list of dictionaries, pass single dict to add_row
@@ -2301,6 +2303,21 @@ class Table(PluginSubcomponent):
         self._qtable = None
         super().__init__(plugin, 'Table', *args, **kwargs)
 
+    def default_value_for_column(self, colname=None, value=None):
+        if colname in self._default_values_by_colname:
+            return self._default_values_by_colname.get(colname)
+        if isinstance(value, (tuple, list)):
+            return [self.default_value_for_column(value=v) for v in value]
+        if isinstance(value, (float, int)):
+            return np.nan
+        if isinstance(value, str):
+            return ''
+        return None
+
+    @staticmethod
+    def _new_col_visible(colname):
+        return True
+
     def add_item(self, item):
         """
         Add an item/row to the table.
@@ -2351,10 +2368,21 @@ class Table(PluginSubcomponent):
         if self._qtable is None:
             self._qtable = QTable([item])
         else:
-            # NOTE: this does not support adding columns that did not exist in the first
-            # call to add_row since the last call to clear_table
+            # add any missing columns with a default value for all previous rows
+            for colname, value in item.items():
+                if colname in self._qtable.colnames:
+                    continue
+                default_value = self.default_value_for_column(colname=colname,
+                                                              value=value)
+                self._qtable.add_column(default_value, name=colname)
+
             self._qtable.add_row(item)
 
+        missing_headers = [k for k in item.keys() if k not in self.headers_avail]
+        if len(missing_headers):
+            self.headers_avail = self.headers_avail + missing_headers
+            self.headers_visible = self.headers_visible + [m for m in missing_headers if self._new_col_visible(m)]  # noqa
+
         # clean data to show in the UI
         self.items = self.items + [{k: json_safe(k, v) for k, v in item.items()}]
 


Test patch:
diff --git a/jdaviz/configs/cubeviz/plugins/tests/test_parsers.py b/jdaviz/configs/cubeviz/plugins/tests/test_parsers.py
index 5a131ab9..48b9ba81 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_fitting.py b/jdaviz/configs/default/plugins/model_fitting/tests/test_fitting.py
index 9b066408..583c409a 100644
--- a/jdaviz/configs/default/plugins/model_fitting/tests/test_fitting.py
+++ b/jdaviz/configs/default/plugins/model_fitting/tests/test_fitting.py
@@ -263,3 +263,42 @@ def test_cube_fitting_backend(cubeviz_helper, unc, tmp_path):
     data_mask = cubeviz_helper.app.data_collection["fitted_cube.fits[MASK]"]
     flux_mask = data_mask.get_component("flux")
     assert_array_equal(flux_mask.data, mask)
+
+
+@pytest.mark.filterwarnings(r"ignore:Model is linear in parameters.*")
+@pytest.mark.filterwarnings(r"ignore:The fit may be unsuccessful.*")
+def test_results_table(specviz_helper, spectrum1d):
+    data_label = 'test'
+    specviz_helper.load_data(spectrum1d, data_label=data_label)
+
+    mf = specviz_helper.plugins['Model Fitting']
+    mf.create_model_component('Linear1D')
+
+    mf.add_results.label = 'linear model'
+    mf.calculate_fit(add_data=True)
+    mf_table = mf.export_table()
+    assert len(mf_table) == 1
+    assert mf_table['equation'][-1] == 'L'
+    assert mf_table.colnames == ['model', 'data_label', 'spectral_subset', 'equation',
+                                 'L:slope_0', 'L:slope_0:unit',
+                                 'L:slope_0:fixed', 'L:slope_0:std',
+                                 'L:intercept_0', 'L:intercept_0:unit',
+                                 'L:intercept_0:fixed', 'L:intercept_0:std']
+
+    mf.create_model_component('Gaussian1D')
+    mf.add_results.label = 'composite model'
+    mf.calculate_fit(add_data=True)
+    mf_table = mf.export_table()
+    assert len(mf_table) == 2
+    assert mf_table['equation'][-1] == 'L+G'
+    assert mf_table.colnames == ['model', 'data_label', 'spectral_subset', 'equation',
+                                 'L:slope_0', 'L:slope_0:unit',
+                                 'L:slope_0:fixed', 'L:slope_0:std',
+                                 'L:intercept_0', 'L:intercept_0:unit',
+                                 'L:intercept_0:fixed', 'L:intercept_0:std',
+                                 'G:amplitude_1', 'G:amplitude_1:unit',
+                                 'G:amplitude_1:fixed', 'G:amplitude_1:std',
+                                 'G:mean_1', 'G:mean_1:unit',
+                                 'G:mean_1:fixed', 'G:mean_1:std',
+                                 'G:stddev_1', 'G:stddev_1:unit',
+                                 'G:stddev_1:fixed', 'G:stddev_1:std']
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 62621b91..42232520 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/plugins/line_analysis/tests/test_line_analysis.py b/jdaviz/configs/specviz/plugins/line_analysis/tests/test_line_analysis.py
index a6f6d13a..34812a24 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
@@ -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 e7dd0ef4..82543695 100644
--- a/jdaviz/configs/specviz/tests/test_helper.py
+++ b/jdaviz/configs/specviz/tests/test_helper.py
@@ -5,7 +5,7 @@ import pytest
 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 @@ class TestSpecvizHelper:
         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 1ecefb7f..35238969 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 c8fd0ae9..e8e69d10 100644
--- a/jdaviz/tests/test_subsets.py
+++ b/jdaviz/tests/test_subsets.py
@@ -4,9 +4,11 @@ from astropy import units as u
 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,114 @@ 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, 10)), 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, 10)), 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
+
+
+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}
+
+
+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)