Skip to content

Commit

Permalink
Merge pull request #2242 from duytnguyendtn/depsubsets
Browse files Browse the repository at this point in the history
Deprecate subset getters
  • Loading branch information
pllim authored Jun 21, 2023
2 parents 54ae4c8 + 06f2ed0 commit 0fdb3bd
Show file tree
Hide file tree
Showing 23 changed files with 127 additions and 87 deletions.
14 changes: 13 additions & 1 deletion CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,13 @@ Specviz2d
API Changes
-----------

- ``viz.app.get_data_from_viewer()`` is deprecated; use ``viz.get_data()``. [#2242]

- ``viz.app.get_subsets_from_viewer()`` is deprecated; use ``viz.app.get_subsets()``. [#2242]

- ``viz.get_data()`` now takes optional ``**kwargs``; e.g., you could pass in
``function="sum"`` to collapse a cube in Cubeviz. [#2242]

Cubeviz
^^^^^^^

Expand All @@ -48,10 +55,13 @@ Imviz
Mosviz
^^^^^^

- Added new ``statistic`` keyword to ``mosviz.get_viewer("spectrum-2d-viewer").data()``
to allow user to collapse 2D spectrum to 1D. [#2242]

Specviz
^^^^^^^

* Re-enabled unit conversion support. [#2127]
- Re-enabled unit conversion support. [#2127]

Specviz2d
^^^^^^^^^
Expand All @@ -61,6 +71,8 @@ Bug Fixes

- Fixed wrong elliptical region translation in ``app.get_subsets()``. [#2244]

- Fixed ``cls`` input being ignored in ``viz.get_data()``. [#2242]

Cubeviz
^^^^^^^

Expand Down
4 changes: 2 additions & 2 deletions docs/cubeviz/export_data.rst
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ can be used to extract the *spectrum* of a spatial subset named "Subset 1":

.. code-block:: python
subset1_spec1d = cubeviz.specviz.get_spectra(subset_to_apply="Subset 1")
subset1_spec1d = cubeviz.specviz.get_spectra(spectral_subset="Subset 1")
An example without accessing Specviz:

Expand All @@ -41,7 +41,7 @@ To get all subsets from the spectrum viewer:

.. code-block:: python
subset1_spec1d = cubeviz.app.get_subsets_from_viewer("spectrum-viewer")
subset1_spec1d = cubeviz.specviz.app.get_subsets()
To access the spatial regions themselves:

Expand Down
15 changes: 11 additions & 4 deletions jdaviz/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from astropy.io import fits
from astropy.coordinates import Angle
from astropy.time import Time
from astropy.utils.decorators import deprecated
from regions import PixCoord, CirclePixelRegion, RectanglePixelRegion, EllipsePixelRegion

from echo import CallbackProperty, DictCallbackProperty, ListCallbackProperty
Expand Down Expand Up @@ -662,6 +663,7 @@ def get_viewer_by_id(self, vid):
"""
return self._viewer_store.get(vid)

@deprecated(since="3.6", alternative="viz_helper.get_data")
def get_data_from_viewer(self, viewer_reference, data_label=None,
cls='default', include_subsets=True):
"""
Expand Down Expand Up @@ -769,6 +771,7 @@ def get_data_from_viewer(self, viewer_reference, data_label=None,
# If a data label was provided, return only the corresponding data, otherwise return all:
return data.get(data_label, data)

@deprecated(since="3.6", alternative="get_subsets")
def get_subsets_from_viewer(self, viewer_reference, data_label=None, subset_type=None):
"""
Returns the subsets of a specified viewer converted to astropy regions
Expand Down Expand Up @@ -803,9 +806,11 @@ def get_subsets_from_viewer(self, viewer_reference, data_label=None, subset_type
objects.
"""
viewer = self.get_viewer(viewer_reference)
data = self.get_data_from_viewer(viewer_reference,
data_label,
cls=None)
with warnings.catch_warnings():
warnings.simplefilter("ignore")
data = self.get_data_from_viewer(viewer_reference,
data_label,
cls=None)
regions = {}

if data_label is not None:
Expand Down Expand Up @@ -845,7 +850,9 @@ def get_subsets_from_viewer(self, viewer_reference, data_label=None, subset_type
regions[key] = self.get_subsets(key)
continue

temp_data = self.get_data_from_viewer(viewer_reference, value.label)
with warnings.catch_warnings():
warnings.simplefilter("ignore")
temp_data = self.get_data_from_viewer(viewer_reference, value.label)
if isinstance(temp_data, Spectrum1D):
regions[key] = self.get_subsets(key)
continue
Expand Down
13 changes: 8 additions & 5 deletions jdaviz/configs/cubeviz/plugins/tests/test_data_retrieval.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
import warnings

import pytest
import numpy as np

from astropy.utils.data import download_file


@pytest.mark.filterwarnings('ignore')
@pytest.mark.remote_data
def test_data_retrieval(cubeviz_helper):
"""The purpose of this test is to check that both methods:
- app.get_viewer('spectrum-viewer').data()
- app.get_data_from_viewer("spectrum-viewer")
- cubeviz_helper.get_data()
return the same spectrum values.
"""
Expand All @@ -20,14 +21,16 @@ def test_data_retrieval(cubeviz_helper):

spectrum_viewer_reference_name = "spectrum-viewer"
fn = download_file(URL, cache=True)
cubeviz_helper.load_data(fn)
with warnings.catch_warnings():
warnings.filterwarnings("ignore")
cubeviz_helper.load_data(fn)

# 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.app.get_data_from_viewer(spectrum_viewer_reference_name)
a2 = cubeviz_helper.get_data("contents[FLUX]", function="sum")

test_value_1 = a1[0].data
test_value_2 = list(a2.values())[0].data
test_value_2 = a2.flux.value

assert np.allclose(test_value_1, test_value_2, atol=1e-5)
2 changes: 1 addition & 1 deletion jdaviz/configs/cubeviz/plugins/tests/test_parsers.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ def test_spectrum1d_with_fake_fixed_units(spectrum1d, cubeviz_helper):
cubeviz_helper.app.add_data_to_viewer('spectrum-viewer', 'test')
cubeviz_helper.app.get_viewer('spectrum-viewer').apply_roi(XRangeROI(6600, 7400))

subsets = cubeviz_helper.app.get_subsets_from_viewer("spectrum-viewer")
subsets = cubeviz_helper.app.get_subsets()
reg = subsets.get('Subset 1')

assert len(subsets) == 1
Expand Down
12 changes: 6 additions & 6 deletions jdaviz/configs/cubeviz/plugins/tests/test_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ def test_spectrum_at_spaxel(cubeviz_helper, spectrum1d_cube):
assert len(spectrum_viewer.data()) == 2

# Check that a new subset was created
subsets = cubeviz_helper.app.get_subsets_from_viewer('flux-viewer')
reg = subsets.get('Subset 1')
subsets = cubeviz_helper.app.get_subsets()
reg = subsets.get('Subset 1')[0]['region']
assert len(subsets) == 1
assert isinstance(reg, RectanglePixelRegion)

Expand Down Expand Up @@ -64,8 +64,8 @@ def test_spectrum_at_spaxel_altkey_true(cubeviz_helper, spectrum1d_cube):
assert len(spectrum_viewer.data()) == 2

# Check that subset was created
subsets = cubeviz_helper.app.get_subsets_from_viewer('flux-viewer')
reg = subsets.get('Subset 1')
subsets = cubeviz_helper.app.get_subsets()
reg = subsets.get('Subset 1')[0]['region']
assert len(subsets) == 1
assert isinstance(reg, RectanglePixelRegion)

Expand All @@ -77,8 +77,8 @@ def test_spectrum_at_spaxel_altkey_true(cubeviz_helper, spectrum1d_cube):
assert len(flux_viewer.figure.marks) == 4
assert len(spectrum_viewer.data()) == 3

subsets = cubeviz_helper.app.get_subsets_from_viewer('flux-viewer')
reg2 = subsets.get('Subset 2')
subsets = cubeviz_helper.app.get_subsets()
reg2 = subsets.get('Subset 2')[0]['region']
assert len(subsets) == 2
assert isinstance(reg2, RectanglePixelRegion)

Expand Down
6 changes: 3 additions & 3 deletions jdaviz/configs/imviz/tests/test_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@ def test_create_new_viewer(imviz_helper, image_2d_wcs):
assert len(imviz_helper.app.get_viewer_ids()) == 2

# there should be no data in the new viewer
assert len(imviz_helper.app.get_data_from_viewer(viewer_name)) == 0
assert len(imviz_helper.app.get_viewer(viewer_name).data()) == 0

# then add data, and check that data were added to the new viewer
imviz_helper.app.add_data_to_viewer(viewer_name, data_label)
assert len(imviz_helper.app.get_data_from_viewer(viewer_name)) == 1
assert len(imviz_helper.app.get_viewer(viewer_name).data()) == 1

# remove data from the new viewer, check that it was removed
imviz_helper.app.remove_data_from_viewer(viewer_name, data_label)
assert len(imviz_helper.app.get_data_from_viewer(viewer_name)) == 0
assert len(imviz_helper.app.get_viewer(viewer_name).data()) == 0
7 changes: 4 additions & 3 deletions jdaviz/configs/mosviz/plugins/viewers.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def __init__(self, *args, **kwargs):
self.figure.fig_margin = {'left': 0, 'bottom': 0, 'top': 0, 'right': 0}

def data(self, cls=None):
return [layer_state.layer # .get_object(cls=cls or self.default_class)
return [layer_state.layer.get_object(cls=cls or self.default_class)
for layer_state in self.state.layers
if hasattr(layer_state, 'layer') and
isinstance(layer_state.layer, BaseData)]
Expand Down Expand Up @@ -207,8 +207,9 @@ def _handle_x_axis_orientation(self, *args):
x_scales.min = max(limits) if self.inverted_x_axis else min(limits)
x_scales.max = min(limits) if self.inverted_x_axis else max(limits)

def data(self, cls=None):
return [layer_state.layer.get_object(cls=cls or self.default_class)
def data(self, cls=None, statistic=None):
return [layer_state.layer.get_object(statistic=statistic,
cls=cls or self.default_class)
for layer_state in self.state.layers
if hasattr(layer_state, 'layer') and
isinstance(layer_state.layer, BaseData)]
Expand Down
54 changes: 34 additions & 20 deletions jdaviz/configs/mosviz/tests/test_data_loading.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@

from zipfile import ZipFile

from astropy.nddata import CCDData
import numpy as np
import pytest
from astropy.nddata import CCDData
from specutils import Spectrum1D

from jdaviz.utils import PRIHDR_KEY
Expand All @@ -20,12 +20,14 @@ def test_load_spectrum1d(mosviz_helper, spectrum1d):
assert dc_1.label == label
assert dc_1.meta['uncertainty_type'] == 'std'

table = mosviz_helper.app.get_viewer('table-viewer')
table = mosviz_helper.app.get_viewer(mosviz_helper._default_table_viewer_reference_name)
table.widget_table.vue_on_row_clicked(0)

data = mosviz_helper.app.get_data_from_viewer('spectrum-viewer')
data = mosviz_helper.app.get_viewer(mosviz_helper._default_spectrum_viewer_reference_name
).data()

assert isinstance(data[label], Spectrum1D)
assert len(data) == 1
assert isinstance(data[0], Spectrum1D)

with pytest.raises(AttributeError):
mosviz_helper.load_1d_spectra([1, 2, 3])
Expand All @@ -42,12 +44,14 @@ def test_load_image(mosviz_helper, mos_image):
assert PRIHDR_KEY not in dc_1.meta
assert dc_1.meta['RADESYS'] == 'ICRS'

table = mosviz_helper.app.get_viewer('table-viewer')
table = mosviz_helper.app.get_viewer(mosviz_helper._default_table_viewer_reference_name)
table.widget_table.vue_on_row_clicked(0)

data = mosviz_helper.app.get_data_from_viewer('image-viewer')
data = mosviz_helper.app.get_viewer(mosviz_helper._default_image_viewer_reference_name
).data(cls=CCDData)

dataval = data[f"{label} 0"]
assert len(data) == 1
dataval = data[0]
assert isinstance(dataval, CCDData)
assert dataval.shape == (55, 55)

Expand All @@ -63,12 +67,14 @@ def test_load_spectrum_collection(mosviz_helper, spectrum_collection):
assert dc_1.label == labels[0]
assert dc_1.meta['uncertainty_type'] == 'std'

table = mosviz_helper.app.get_viewer('table-viewer')
table = mosviz_helper.app.get_viewer(mosviz_helper._default_table_viewer_reference_name)
table.select_row(0)

data = mosviz_helper.app.get_data_from_viewer('spectrum-viewer')
data = mosviz_helper.app.get_viewer(mosviz_helper._default_spectrum_viewer_reference_name
).data()

assert isinstance(data[labels[0]], Spectrum1D)
assert len(data) == 1
assert isinstance(data[0], Spectrum1D)


def test_load_list_of_spectrum1d(mosviz_helper, spectrum1d):
Expand All @@ -83,12 +89,14 @@ def test_load_list_of_spectrum1d(mosviz_helper, spectrum1d):
assert dc_1.label == labels[0]
assert dc_1.meta['uncertainty_type'] == 'std'

table = mosviz_helper.app.get_viewer('table-viewer')
table = mosviz_helper.app.get_viewer(mosviz_helper._default_table_viewer_reference_name)
table.widget_table.vue_on_row_clicked(0)

data = mosviz_helper.app.get_data_from_viewer('spectrum-viewer')
data = mosviz_helper.app.get_viewer(mosviz_helper._default_spectrum_viewer_reference_name
).data()

assert isinstance(data[labels[0]], Spectrum1D)
assert len(data) == 1
assert isinstance(data[0], Spectrum1D)


def test_load_mos_spectrum2d(mosviz_helper, mos_spectrum2d):
Expand All @@ -102,12 +110,14 @@ def test_load_mos_spectrum2d(mosviz_helper, mos_spectrum2d):
assert dc_1.label == label
assert dc_1.meta['INSTRUME'] == 'nirspec'

table = mosviz_helper.app.get_viewer('table-viewer')
table = mosviz_helper.app.get_viewer(mosviz_helper._default_table_viewer_reference_name)
table.widget_table.vue_on_row_clicked(0)

data = mosviz_helper.app.get_data_from_viewer('spectrum-2d-viewer')
data = mosviz_helper.app.get_viewer(mosviz_helper._default_spectrum_2d_viewer_reference_name
).data()

assert data[label].shape == (1024, 15)
assert len(data) == 1
assert data[0].shape == (1024, 15)


@pytest.mark.parametrize('label', [None, "Test Label"])
Expand All @@ -118,7 +128,8 @@ def test_load_multi_image_spec(mosviz_helper, mos_image, spectrum1d, mos_spectru

mosviz_helper.load_data(spectra1d, spectra2d, images=images, images_label=label)

assert mosviz_helper.app.get_viewer("table-viewer").figure_widget.highlighted == 0
assert mosviz_helper.app.get_viewer(mosviz_helper._default_table_viewer_reference_name
).figure_widget.highlighted == 0
assert len(mosviz_helper.app.data_collection) == 10

qtable = mosviz_helper.to_table()
Expand All @@ -134,7 +145,8 @@ def test_load_multi_image_and_spec1d_only(mosviz_helper, mos_image, spectrum1d):

mosviz_helper.load_data(spectra_1d=spectra1d, images=images)

assert mosviz_helper.app.get_viewer("table-viewer").figure_widget.highlighted == 0
assert mosviz_helper.app.get_viewer(mosviz_helper._default_table_viewer_reference_name
).figure_widget.highlighted == 0
assert len(mosviz_helper.app.data_collection) == 7


Expand All @@ -144,7 +156,8 @@ def test_load_multi_image_and_spec2d_only(mosviz_helper, mos_image, mos_spectrum

mosviz_helper.load_data(spectra_2d=spectra2d, images=images)

assert mosviz_helper.app.get_viewer("table-viewer").figure_widget.highlighted == 0
assert mosviz_helper.app.get_viewer(mosviz_helper._default_table_viewer_reference_name
).figure_widget.highlighted == 0
assert len(mosviz_helper.app.data_collection) == 7


Expand All @@ -170,7 +183,8 @@ def test_load_single_image_multi_spec(mosviz_helper, mos_image, spectrum1d, mos_

mosviz_helper.load_data(spectra1d, spectra2d, images=mos_image, images_label=label)

assert mosviz_helper.app.get_viewer("table-viewer").figure_widget.highlighted == 0
assert mosviz_helper.app.get_viewer(mosviz_helper._default_table_viewer_reference_name
).figure_widget.highlighted == 0
assert len(mosviz_helper.app.data_collection) == 8

qtable = mosviz_helper.to_table()
Expand Down
Loading

0 comments on commit 0fdb3bd

Please sign in to comment.