From a266f035f1c4fb70c75b34ab0ea9f34778962942 Mon Sep 17 00:00:00 2001
From: Kyle Conroy <kyleconroy@gmail.com>
Date: Tue, 30 Apr 2024 13:39:27 -0400
Subject: [PATCH 1/2] get_data: REMOVE support for function/spatial_subset

---
 jdaviz/configs/cubeviz/helper.py              | 35 +-------
 .../plugins/tests/test_cubeviz_helper.py      | 88 ++-----------------
 .../plugins/tests/test_data_retrieval.py      |  2 +-
 .../model_fitting/tests/test_plugin.py        |  4 +-
 .../imviz/plugins/coords_info/coords_info.py  | 25 +-----
 jdaviz/configs/specviz/helper.py              | 25 +-----
 .../plugins/line_analysis/line_analysis.py    |  5 +-
 jdaviz/core/helpers.py                        | 24 ++---
 8 files changed, 23 insertions(+), 185 deletions(-)

diff --git a/jdaviz/configs/cubeviz/helper.py b/jdaviz/configs/cubeviz/helper.py
index a7f2ad83bc..ebeb59ade8 100644
--- a/jdaviz/configs/cubeviz/helper.py
+++ b/jdaviz/configs/cubeviz/helper.py
@@ -1,4 +1,3 @@
-import logging
 import numpy as np
 from astropy.io import fits
 from astropy.io import registry as io_registry
@@ -144,7 +143,7 @@ def specviz(self):
             self._specviz = Specviz(app=self.app)
         return self._specviz
 
-    def get_data(self, data_label=None, spatial_subset=None, spectral_subset=None, function=None,
+    def get_data(self, data_label=None, spatial_subset=None, spectral_subset=None,
                  cls=None, use_display_units=False):
         """
         Returns data with name equal to ``data_label`` of type ``cls`` with subsets applied from
@@ -155,19 +154,10 @@ def get_data(self, data_label=None, spatial_subset=None, spectral_subset=None, f
         data_label : str, optional
             Provide a label to retrieve a specific data set from data_collection.
         spatial_subset : str, optional
-            Deprecated as of 3.11.  Use the spectral extraction plugin and extract the extracted
-            spectrum instead.
-            Spatial subset applied to data.
+            Spatial subset applied to data.  Only applicable if ``data_label`` points to a cube or
+            image.  To extract a spectrum from a cube, use the spectral extraction plugin instead.
         spectral_subset : str, optional
             Spectral subset applied to data.
-        function : {True, False, 'minimum', 'maximum', 'mean', 'median', 'sum'}, optional
-            Deprecated as of 3.11.  Use the spectral extraction plugin and extract the extracted
-            spectrum instead.
-            Ignored if ``data_label`` does not point to cube-like data.
-            If True, will collapse using 'sum'.
-            If provided as a string, the cube will be collapsed with the provided
-            function.  If False, None, or not passed, the entire cube will be returned (unless there
-            are values for ``spatial_subset`` and ``spectral_subset``).
         cls : `~specutils.Spectrum1D`, `~astropy.nddata.CCDData`, optional
             The type that data will be returned as.
 
@@ -177,25 +167,8 @@ def get_data(self, data_label=None, spatial_subset=None, spectral_subset=None, f
             Data is returned as type cls with subsets applied.
 
         """
-        if function is not None or spatial_subset is not None:
-            logging.warning("DeprecationWarning: function and spatial_subset are deprecated and"
-                            " will be removed in a future release.  Use the spectral extraction"
-                            " plugin and access the extracted spectrum directly.")
-
-        # If function is a value ('sum' or 'minimum') or True and spatial and spectral
-        # are set, then we collapse the cube along the spatial subset using the function, then
-        # we apply the mask from the spectral subset.
-        # If function is any value other than False, we use specviz
-        if (function is not False and spectral_subset and spatial_subset) or function:
-            return self.specviz.get_data(data_label=data_label, spectral_subset=spectral_subset,
-                                         cls=cls, spatial_subset=spatial_subset, function=function)
-        elif function is False and spectral_subset:
-            raise ValueError("function cannot be False if spectral_subset"
-                             " is set")
-        elif function is False:
-            function = None
         return self._get_data(data_label=data_label, spatial_subset=spatial_subset,
-                              spectral_subset=spectral_subset, function=function,
+                              spectral_subset=spectral_subset,
                               cls=cls, use_display_units=use_display_units)
 
     # Need this method for Imviz Aperture Photometry plugin.
diff --git a/jdaviz/configs/cubeviz/plugins/tests/test_cubeviz_helper.py b/jdaviz/configs/cubeviz/plugins/tests/test_cubeviz_helper.py
index feae0405bb..95f4f22769 100644
--- a/jdaviz/configs/cubeviz/plugins/tests/test_cubeviz_helper.py
+++ b/jdaviz/configs/cubeviz/plugins/tests/test_cubeviz_helper.py
@@ -1,9 +1,3 @@
-import pytest
-
-from astropy import units as u
-from astropy.tests.helper import assert_quantity_allclose
-from specutils import Spectrum1D
-
 from glue.core.roi import XRangeROI
 
 from jdaviz import Cubeviz
@@ -41,49 +35,6 @@ def test_plugin_user_apis(cubeviz_helper):
             assert hasattr(plugin, attr)
 
 
-def test_invalid_function(cubeviz_helper, spectrum1d_cube):
-    cubeviz_helper.load_data(spectrum1d_cube, "test")
-    cubeviz_helper._apply_interactive_region('bqplot:ellipse', (0, 0), (9, 8))
-
-    with pytest.raises(ValueError, match='function 42 not in list of valid '):
-        cubeviz_helper.get_data(data_label="test[FLUX]", spatial_subset='Subset 1', function=42)
-
-    # Also make sure specviz redshift slider warning does not show up.
-    # https://github.com/spacetelescope/jdaviz/issues/2029
-    cubeviz_helper.specviz.y_limits(0, 3)
-
-
-def test_valid_function(cubeviz_helper, spectrum1d_cube):
-    cubeviz_helper.load_data(spectrum1d_cube, "test")
-    cubeviz_helper._apply_interactive_region('bqplot:ellipse', (0, 0), (9, 8))
-
-    results_cube = cubeviz_helper.get_data(data_label="test[FLUX]",
-                                           spatial_subset='Subset 1')
-    assert results_cube.flux.ndim == 3
-    results_false = cubeviz_helper.get_data(data_label="test[FLUX]",
-                                            spatial_subset='Subset 1', function=False)
-    assert results_false.flux.ndim == 3
-
-    results_def = cubeviz_helper.get_data(data_label="test[FLUX]",
-                                          spatial_subset='Subset 1', function=True)
-    assert results_def.flux.ndim == 1
-
-    results_min = cubeviz_helper.get_data(data_label="test[FLUX]",
-                                          spatial_subset='Subset 1', function="minimum")
-    results_max = cubeviz_helper.get_data(data_label="test[FLUX]",
-                                          spatial_subset='Subset 1', function="maximum")
-    assert isinstance(results_min, Spectrum1D)
-    assert_quantity_allclose(results_min.flux,
-                             [6., 14.] * u.Jy, atol=1e-5 * u.Jy)
-    assert_quantity_allclose(results_max.flux,
-                             [7., 15.] * u.Jy, atol=1e-5 * u.Jy)
-
-    # calling without function gives cube
-    assert cubeviz_helper.get_data(data_label="test[FLUX]").flux.ndim == 3
-    # but calling through specviz automatically collapses
-    assert cubeviz_helper.specviz.get_data(data_label="test[FLUX]").flux.ndim == 1
-
-
 def test_remote_server_disable_save_serverside():
     config = get_configuration('cubeviz')
     config['settings']['server_is_remote'] = True
@@ -98,47 +49,18 @@ def test_remote_server_disable_save_serverside():
 
 
 def test_get_data_spatial_and_spectral(cubeviz_helper, spectrum1d_cube_larger):
-    data_label = "test"
-    spatial_subset = "Subset 1"
-    spectral_subset = "Subset 2"
-    cubeviz_helper.load_data(spectrum1d_cube_larger, data_label)
+    cubeviz_helper.load_data(spectrum1d_cube_larger, data_label="test")
+    # Subset 1 (spatial)
     cubeviz_helper._apply_interactive_region('bqplot:ellipse', (0, 0), (9, 8))
 
+    # Subset 2 (spectral)
     spec_viewer = cubeviz_helper.app.get_viewer(cubeviz_helper._default_spectrum_viewer_reference_name) # noqa
     spec_viewer.apply_roi(XRangeROI(4.62440061e-07, 4.62520112e-07))
 
-    data_label = data_label + "[FLUX]"
-    # This will be the same if function is None or True
-    spatial_with_spec = cubeviz_helper.get_data(data_label=data_label,
-                                                spatial_subset=spatial_subset,
-                                                spectral_subset=spectral_subset)
+    spatial_with_spec = cubeviz_helper.get_data(data_label="Spectrum (Subset 1, sum)",
+                                                spectral_subset="Subset 2")
     assert spatial_with_spec.flux.ndim == 1
     assert list(spatial_with_spec.mask) == [True, True, False, False, True,
                                             True, True, True, True, True]
     assert max(list(spatial_with_spec.flux.value)) == 157.
     assert min(list(spatial_with_spec.flux.value)) == 13.
-
-    spatial_with_spec = cubeviz_helper.get_data(data_label=data_label,
-                                                spatial_subset=spatial_subset,
-                                                spectral_subset=spectral_subset,
-                                                function='minimum')
-    assert max(list(spatial_with_spec.flux.value)) == 78.
-    assert min(list(spatial_with_spec.flux.value)) == 6.
-
-    collapse_with_spectral = cubeviz_helper.get_data(data_label=data_label,
-                                                     spectral_subset=spectral_subset,
-                                                     function=True)
-    collapse_with_spectral2 = cubeviz_helper.get_data(data_label=data_label,
-                                                      function=True)
-
-    assert list(collapse_with_spectral.flux) == list(collapse_with_spectral2.flux)
-
-    with pytest.raises(ValueError, match=f'{spectral_subset} is not a spatial subset.'):
-        cubeviz_helper.get_data(data_label=data_label, spatial_subset=spectral_subset,
-                                function=True)
-    with pytest.raises(ValueError, match=f'{spatial_subset} is not a spectral subset.'):
-        cubeviz_helper.get_data(data_label=data_label, spectral_subset=spatial_subset,
-                                function=True)
-    with pytest.raises(ValueError, match='function cannot be False if spectral_subset'):
-        cubeviz_helper.get_data(data_label=data_label, spectral_subset=spectral_subset,
-                                function=False)
diff --git a/jdaviz/configs/cubeviz/plugins/tests/test_data_retrieval.py b/jdaviz/configs/cubeviz/plugins/tests/test_data_retrieval.py
index b5f1dfc6f5..ee1acdef34 100644
--- a/jdaviz/configs/cubeviz/plugins/tests/test_data_retrieval.py
+++ b/jdaviz/configs/cubeviz/plugins/tests/test_data_retrieval.py
@@ -28,7 +28,7 @@ def test_data_retrieval(cubeviz_helper):
     # two ways of retrieving data from the viewer.
     # They should return the same spectral values
     a1 = cubeviz_helper.app.get_viewer(spectrum_viewer_reference_name).data()
-    a2 = cubeviz_helper.get_data("contents[FLUX]", function="sum")
+    a2 = cubeviz_helper.get_data("Spectrum (sum)")
 
     test_value_1 = a1[0].data
     test_value_2 = a2.flux.value
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 03718cfb33..055f86b10b 100644
--- a/jdaviz/configs/default/plugins/model_fitting/tests/test_plugin.py
+++ b/jdaviz/configs/default/plugins/model_fitting/tests/test_plugin.py
@@ -325,9 +325,7 @@ def test_subset_masks(cubeviz_helper, spectrum1d_cube_larger):
     p.spectral_subset_selected = "Subset 2"
 
     # Get the data object again (ensures mask == None)
-    data = cubeviz_helper.app.data_collection[0].get_object(
-        cls=Spectrum1D, statistic=None
-    )
+    data = cubeviz_helper.app.data_collection[0].get_object("Spectrum (sum)")
     subset = cubeviz_helper.app.data_collection[0].get_subset_object(
         p.spectral_subset_selected, cls=Spectrum1D, statistic=None
     )
diff --git a/jdaviz/configs/imviz/plugins/coords_info/coords_info.py b/jdaviz/configs/imviz/plugins/coords_info/coords_info.py
index e8380c450e..5cfaefb44c 100644
--- a/jdaviz/configs/imviz/plugins/coords_info/coords_info.py
+++ b/jdaviz/configs/imviz/plugins/coords_info/coords_info.py
@@ -5,7 +5,6 @@
 from astropy import units as u
 from bqplot import LinearScale
 from glue.core import BaseData
-from glue.core.subset_group import GroupedSubset
 from glue_jupyter.bqplot.image.layer_artist import BqplotImageSubsetLayerArtist
 
 from jdaviz.configs.cubeviz.plugins.viewers import CubevizImageView
@@ -18,7 +17,6 @@
 from jdaviz.core.marks import PluginScatter, PluginLine
 from jdaviz.core.registries import tool_registry
 from jdaviz.core.template_mixin import TemplateMixin, DatasetSelectMixin
-from jdaviz.utils import get_subset_type
 
 __all__ = ['CoordsInfo']
 
@@ -540,33 +538,18 @@ def _copy_axes_to_spectral():
             if self.dataset.selected != 'auto' and self.dataset.selected != lyr.layer.label:
                 continue
 
-            if isinstance(lyr.layer, GroupedSubset):
-                subset_state = getattr(lyr.layer, 'subset_state', None)
-                if subset_state is None:
-                    continue
-                subset_type = get_subset_type(subset_state)
-                if subset_type == 'spectral':
-                    # then this is a SPECTRAL subset
-                    continue
-                # For use later in data retrieval
-                subset_label = lyr.layer.label
-                data_label = lyr.layer.data.label
-            elif ((not isinstance(lyr.layer, BaseData)) or (lyr.layer.ndim not in (1, 3))):
+            if ((not isinstance(lyr.layer, BaseData)) or (lyr.layer.ndim not in (1, 3))):
                 continue
-            else:
-                subset_label = None
-                data_label = lyr.layer.label
+            data_label = lyr.layer.label
 
             try:
                 # Cache should have been populated when spectrum was first plotted.
                 # But if not (maybe user changed statistic), we cache it here too.
-                statistic = getattr(viewer.state, 'function', None)
-                cache_key = (lyr.layer.label, statistic)
+                cache_key = lyr.layer.label
                 if cache_key in self.app._get_object_cache:
                     sp = self.app._get_object_cache[cache_key]
                 else:
-                    sp = self._specviz_helper.get_data(data_label=data_label,
-                                                       spatial_subset=subset_label)
+                    sp = self._specviz_helper.get_data(data_label=data_label)
                     self.app._get_object_cache[cache_key] = sp
 
                 # Calculations have to happen in the frame of viewer display units.
diff --git a/jdaviz/configs/specviz/helper.py b/jdaviz/configs/specviz/helper.py
index 42f6f8dd29..f495405348 100644
--- a/jdaviz/configs/specviz/helper.py
+++ b/jdaviz/configs/specviz/helper.py
@@ -281,7 +281,7 @@ def set_spectrum_tick_format(self, fmt, axis=None):
         ).figure.axes[axis].tick_format = fmt
 
     def get_data(self, data_label=None, spectral_subset=None, cls=None,
-                 use_display_units=False, **kwargs):
+                 use_display_units=False):
         """
         Returns data with name equal to data_label of type cls with subsets applied from
         spectral_subset.
@@ -303,26 +303,5 @@ def get_data(self, data_label=None, spectral_subset=None, cls=None,
             Data is returned as type cls with subsets applied.
 
         """
-        spatial_subset = kwargs.pop("spatial_subset", None)
-        function = kwargs.pop("function", None)
-        if len(kwargs) > 0:
-            raise ValueError(f'kwargs {[x for x in kwargs.keys()]} are not valid')
-
-        if self.app.config == 'cubeviz':
-            # then this is a specviz instance inside cubeviz and we want to default to the
-            # viewer's collapse function
-            default_sp_viewer = self.app.get_viewer(self._default_spectrum_viewer_reference_name)
-            if function is True or function is None:
-                function = getattr(default_sp_viewer.state, 'function', None)
-
-            if cls is None:
-                cls = Spectrum1D
-        elif spatial_subset or function:
-            raise ValueError('kwargs spatial subset and function are not valid in specviz')
-        else:
-            spatial_subset = None
-            function = None
-
-        return self._get_data(data_label=data_label, spatial_subset=spatial_subset,
-                              spectral_subset=spectral_subset, function=function,
+        return self._get_data(data_label=data_label, spectral_subset=spectral_subset,
                               cls=cls, use_display_units=use_display_units)
diff --git a/jdaviz/configs/specviz/plugins/line_analysis/line_analysis.py b/jdaviz/configs/specviz/plugins/line_analysis/line_analysis.py
index ac09d9e1ca..28c0f1311b 100644
--- a/jdaviz/configs/specviz/plugins/line_analysis/line_analysis.py
+++ b/jdaviz/configs/specviz/plugins/line_analysis/line_analysis.py
@@ -184,10 +184,7 @@ def _on_viewer_data_changed(self, msg):
         if msg.data.label not in viewer_data_labels:
             return
 
-        get_data_kwargs = {'data_label': msg.data.label}
-        if self.config == 'cubeviz':
-            get_data_kwargs['function'] = getattr(viewer.state, 'function', None)
-        viewer_data = self.app._jdaviz_helper.get_data(**get_data_kwargs)
+        viewer_data = self.app._jdaviz_helper.get_data(data_label=msg.data.label)
 
         # If no data is currently plotted, don't attempt to update
         if viewer_data is None:
diff --git a/jdaviz/core/helpers.py b/jdaviz/core/helpers.py
index 7095f2e10f..1ae6410d2e 100644
--- a/jdaviz/core/helpers.py
+++ b/jdaviz/core/helpers.py
@@ -457,7 +457,7 @@ def show_in_new_tab(self, title=None):  # pragma: no cover
         return self.show(loc="sidecar:tab-after", title=title)
 
     def _get_data(self, data_label=None, spatial_subset=None, spectral_subset=None,
-                  mask_subset=None, function=None, cls=None, use_display_units=False):
+                  mask_subset=None, cls=None, use_display_units=False):
         def _handle_display_units(data, use_display_units):
             if use_display_units:
                 if isinstance(data, Spectrum1D):
@@ -494,12 +494,6 @@ def _handle_display_units(data, use_display_units):
                     raise NotImplementedError(f"converting {data.__class__.__name__} to display units is not supported")  # noqa
             return data
 
-        list_of_valid_function_values = ('minimum', 'maximum', 'mean',
-                                         'median', 'sum')
-        if function and function not in list_of_valid_function_values:
-            raise ValueError(f"function {function} not in list of valid"
-                             f" function values {list_of_valid_function_values}")
-
         list_of_valid_subset_names = [x.label for x in self.app.data_collection.subset_groups]
         for subset in (spatial_subset, spectral_subset, mask_subset):
             if subset and subset not in list_of_valid_subset_names:
@@ -539,17 +533,13 @@ def _handle_display_units(data, use_display_units):
             elif data.ndim in [1, 3]:
                 cls = Spectrum1D
 
-        object_kwargs = {}
-        if cls == Spectrum1D:
-            object_kwargs['statistic'] = function
-
         if not spatial_subset and not mask_subset:
             if 'Trace' in data.meta:
                 if cls is not None:  # pragma: no cover
                     raise ValueError("cls not supported for Trace object")
                 data = data.get_object()
             else:
-                data = data.get_object(cls=cls, **object_kwargs)
+                data = data.get_object(cls=cls)
 
             return _handle_display_units(data, use_display_units)
 
@@ -575,15 +565,11 @@ def _handle_display_units(data, use_display_units):
                             if sub.data.label == data_label and subsets.label == spatial_subset][0]
             handler, _ = data_translator.get_handler_for(cls)
             try:
-                data = handler.to_object(real_spatial, **object_kwargs)
+                data = handler.to_object(real_spatial)
             except Exception as e:
                 warnings.warn(f"Not able to get {data_label} returned with"
                               f" subset {spatial_subset} applied of type {cls}."
                               f" Exception: {e}")
-        elif function:
-            # This covers the case where cubeviz.get_data is called using a spectral_subset
-            # with function set.
-            data = data.get_object(cls=cls, **object_kwargs)
 
         # Handle spectral subset, including case where spatial subset is also set
         if spectral_subset and not isinstance(all_subsets[spectral_subset],
@@ -597,12 +583,12 @@ def _handle_display_units(data, use_display_units):
 
             handler, _ = data_translator.get_handler_for(cls)
             try:
-                spec_subset = handler.to_object(real_spectral, **object_kwargs)
+                spec_subset = handler.to_object(real_spectral)
             except Exception as e:
                 warnings.warn(f"Not able to get {data_label} returned with"
                               f" subset {mask_subset} applied of type {cls}."
                               f" Exception: {e}")
-            if spatial_subset or function:
+            if spatial_subset:
                 # Return collapsed Spectrum1D object with spectral subset mask applied
                 data.mask = spec_subset.mask
             else:

From abe7f40628e0b7e6d39d728664a4d081765518b5 Mon Sep 17 00:00:00 2001
From: Pey Lian Lim <2090236+pllim@users.noreply.github.com>
Date: Wed, 1 May 2024 12:30:51 -0400
Subject: [PATCH 2/2] EXP: For big cube, be a minimalist and avoid loading
 uncert and correct units

[ci skip] [rtd skip]
---
 jdaviz/configs/cubeviz/plugins/parsers.py | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/jdaviz/configs/cubeviz/plugins/parsers.py b/jdaviz/configs/cubeviz/plugins/parsers.py
index b9a331e653..a5472281a4 100644
--- a/jdaviz/configs/cubeviz/plugins/parsers.py
+++ b/jdaviz/configs/cubeviz/plugins/parsers.py
@@ -18,8 +18,8 @@
 __all__ = ['parse_data']
 
 EXT_TYPES = dict(flux=['flux', 'sci', 'data'],
-                 uncert=['ivar', 'err', 'var', 'uncert'],
-                 mask=['mask', 'dq', 'quality'])
+                 uncert=['ivar', 'err', 'error', 'var', 'uncert'],
+                 mask=['mask', 'dq', 'quality', 'data_quality'])
 
 
 @data_parser_registry("cubeviz-data-parser")
@@ -214,6 +214,8 @@ def _parse_hdulist(app, hdulist, file_name=None,
         if hdu.data is None or not hdu.is_image or hdu.data.ndim != 3:
             continue
 
+        is_big_cube = hdu.data.size > 8_000_000
+
         data_type = _get_data_type_by_hdu(hdu)
         if not data_type:
             continue
@@ -228,6 +230,8 @@ def _parse_hdulist(app, hdulist, file_name=None,
         if data_type == 'flux':
             wcs = WCS(hdu.header, hdulist)
             wcs_sci = wcs
+        elif is_big_cube:
+            continue
         else:
             wcs = wcs_sci
 
@@ -249,7 +253,11 @@ def _parse_hdulist(app, hdulist, file_name=None,
         if hdu.name != 'PRIMARY' and 'PRIMARY' in hdulist:
             metadata[PRIHDR_KEY] = standardize_metadata(hdulist['PRIMARY'].header)
 
-        sc = _return_spectrum_with_correct_units(flux, wcs, metadata, data_type, hdulist=hdulist)
+        if not is_big_cube:
+            sc = _return_spectrum_with_correct_units(
+                flux, wcs, metadata, data_type, hdulist=hdulist)
+        else:
+            sc = Spectrum1D(flux=flux, wcs=wcs, meta=metadata)
 
         # store original WCS in metadata. this is a hacky workaround for converting subsets
         # to sky regions, where the parent data of the subset might have dropped spatial WCS info