Skip to content

Commit

Permalink
Centralize data label generation
Browse files Browse the repository at this point in the history
  • Loading branch information
javerbukh committed Oct 11, 2022
1 parent ac58644 commit c946d8f
Show file tree
Hide file tree
Showing 12 changed files with 340 additions and 228 deletions.
3 changes: 3 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ New Features

- Add support for nonstandard viewer reference names [#1681]

- Centralize data label generation if user does not provide a label with data load. Also
prevent duplicate data labels from being added to data collection. [#1672]

Cubeviz
^^^^^^^

Expand Down
250 changes: 127 additions & 123 deletions jdaviz/app.py

Large diffs are not rendered by default.

15 changes: 7 additions & 8 deletions jdaviz/configs/cubeviz/plugins/parsers.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ def parse_data(app, file_obj, data_type=None, data_label=None):
for ext, viewer_name in (('SCI', flux_viewer_reference_name),
('ERR', uncert_viewer_reference_name),
('DQ', None)):
data_label = f'{file_name}[{ext}]'
data_label = app.return_data_label(file_name, ext)
_parse_jwst_s3d(
app, hdulist, data_label, ext=ext, viewer_name=viewer_name,
flux_viewer_reference_name=flux_viewer_reference_name,
Expand All @@ -84,13 +84,12 @@ def parse_data(app, file_obj, data_type=None, data_label=None):
for ext, viewer_name in (('DATA', flux_viewer_reference_name),
('ERR', uncert_viewer_reference_name),
('QUALITY', None)):
data_label = f'{file_name}[{ext}]'
data_label = app.return_data_label(file_name, ext)
_parse_esa_s3d(
app, hdulist, data_label, ext=ext, viewer_name=viewer_name,
flux_viewer_reference_name=flux_viewer_reference_name,
spectrum_viewer_reference_name=spectrum_viewer_reference_name
)

else:
_parse_hdulist(
app, hdulist, file_name=data_label or file_name,
Expand Down Expand Up @@ -183,7 +182,7 @@ def _parse_hdulist(app, hdulist, file_name=None,
continue

is_loaded.append(data_type)
data_label = f"{file_name}[{hdu.name}]"
data_label = app.return_data_label(file_name, hdu.name)

if data_type == 'flux':
wcs = WCS(hdu.header, hdulist)
Expand Down Expand Up @@ -346,7 +345,7 @@ def _parse_spectrum1d_3d(app, file_obj, data_label=None,
s1d = Spectrum1D(flux=flux, wcs=file_obj.wcs,
meta=standardize_metadata(file_obj.meta))

cur_data_label = f"{data_label}[{attr.upper()}]"
cur_data_label = app.return_data_label(data_label, attr.upper())
app.add_data(s1d, cur_data_label)

if attr == 'flux':
Expand All @@ -361,13 +360,13 @@ def _parse_spectrum1d_3d(app, file_obj, data_label=None,

def _parse_spectrum1d(app, file_obj, data_label=None, spectrum_viewer_reference_name=None):
if data_label is None:
data_label = "Unknown spectrum object"
data_label = app.return_data_label(file_obj)

# TODO: glue-astronomy translators only look at the flux property of
# specutils Spectrum1D objects. Fix to support uncertainties and masks.

app.add_data(file_obj, f"{data_label}[FLUX]")
app.add_data_to_viewer(spectrum_viewer_reference_name, f"{data_label}[FLUX]")
app.add_data(file_obj, data_label)
app.add_data_to_viewer(spectrum_viewer_reference_name, data_label)


def _get_data_type_by_hdu(hdu):
Expand Down
8 changes: 4 additions & 4 deletions jdaviz/configs/cubeviz/plugins/tests/test_parsers.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ def test_fits_image_hdu_parse(image_cube_hdu_obj, cubeviz_helper):
cubeviz_helper.load_data(image_cube_hdu_obj)

assert len(cubeviz_helper.app.data_collection) == 3
assert cubeviz_helper.app.data_collection[0].label.endswith('[FLUX]')
assert cubeviz_helper.app.data_collection[0].label == "Unknown HDU object[FLUX]"

# first load should be successful; subsequent attempts should fail
with pytest.raises(RuntimeError, match=r".?only one (data)?cube.?"):
Expand Down Expand Up @@ -88,7 +88,7 @@ def test_fits_image_hdu_parse_from_file(tmpdir, image_cube_hdu_obj, cubeviz_help
cubeviz_helper.load_data(path)

assert len(cubeviz_helper.app.data_collection) == 3
assert cubeviz_helper.app.data_collection[0].label.endswith('[FLUX]')
assert cubeviz_helper.app.data_collection[0].label == "test_fits_image.fits[FLUX]"

# This tests the same data as test_fits_image_hdu_parse above.

Expand Down Expand Up @@ -121,7 +121,7 @@ def test_spectrum3d_parse(image_cube_hdu_obj, cubeviz_helper):
cubeviz_helper.load_data(sc)

assert len(cubeviz_helper.app.data_collection) == 1
assert cubeviz_helper.app.data_collection[0].label.endswith('[FLUX]')
assert cubeviz_helper.app.data_collection[0].label == "Unknown spectrum object[FLUX]"

# Same as flux viewer data in test_fits_image_hdu_parse_from_file
flux_viewer = cubeviz_helper.app.get_viewer(cubeviz_helper._default_flux_viewer_reference_name)
Expand All @@ -142,7 +142,7 @@ def test_spectrum1d_parse(spectrum1d, cubeviz_helper):
cubeviz_helper.load_data(spectrum1d)

assert len(cubeviz_helper.app.data_collection) == 1
assert cubeviz_helper.app.data_collection[0].label.endswith('[FLUX]')
assert cubeviz_helper.app.data_collection[0].label.endswith('Spectrum1D')
assert cubeviz_helper.app.data_collection[0].meta['uncertainty_type'] == 'std'

# Coordinate display is only for spatial image, which is missing here.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -760,8 +760,10 @@ def _fit_model_to_cube(self, add_data):
count = max(map(lambda s: int(next(iter(re.findall(r"\d$", s)), 0)),
self.data_collection.labels)) + 1

label = self.app.return_data_label(f"{self.results_label}[Cube]", ext=count)

# Create new glue data object
output_cube = Data(label=f"{self.results_label} [Cube] {count}",
output_cube = Data(label=label,
coords=data.coords)
output_cube['flux'] = fitted_spectrum.flux.value
output_cube.get_component('flux').units = fitted_spectrum.flux.unit.to_string()
Expand Down
4 changes: 2 additions & 2 deletions jdaviz/configs/imviz/helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ def load_data(self, data, data_label=None, do_link=True, show_in_viewer=True, **

# This will only overwrite if not provided.
if not data_label:
kw['data_label'] = cur_data_label
kw['data_label'] = None
else:
kw['data_label'] = data_label

Expand All @@ -165,7 +165,7 @@ def load_data(self, data, data_label=None, do_link=True, show_in_viewer=True, **

# This will only append index to data label if provided.
if data_label:
kw['data_label'] = f'{data_label}_{i}'
kw['data_label'] = data_label

self.app.load_data(data[i, :, :], parser_reference='imviz-data-parser', **kw)

Expand Down
18 changes: 4 additions & 14 deletions jdaviz/configs/imviz/plugins/parsers.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
import base64
import os
import uuid

import numpy as np
from asdf.fits_embed import AsdfInFits
Expand Down Expand Up @@ -56,8 +54,6 @@ def parse_data(app, file_obj, ext=None, data_label=None):
with fits.open(file_obj) as pf:
_parse_image(app, pf, data_label, ext=ext)
else:
if data_label is None:
data_label = f'imviz_data|{str(base64.b85encode(uuid.uuid4().bytes), "utf-8")}'
_parse_image(app, file_obj, data_label, ext=ext)


Expand Down Expand Up @@ -121,20 +117,14 @@ def get_image_data_iterator(app, file_obj, data_label, ext=None):


def _parse_image(app, file_obj, data_label, ext=None):
from jdaviz.core.helpers import _next_subset_num

if app is None:
raise ValueError("app is None, cannot proceed")
if data_label is None:
raise NotImplementedError('data_label should be set by now')

data_label = app.return_data_label(file_obj, ext, alt_name="image_data")
data_iter = get_image_data_iterator(app, file_obj, data_label, ext=ext)

for data, data_label in data_iter:

# avoid duplicate data labels in collection
if data_label in app.data_collection.labels:
i = _next_subset_num(data_label, app.data_collection)
data_label = f'{data_label} {i}'

data_label = app.return_data_label(data_label, alt_name="image_data")
app.add_data(data, data_label)

# Do not run link_image_data here. We do it at the end in Imviz.load_data()
Expand Down
15 changes: 8 additions & 7 deletions jdaviz/configs/imviz/tests/test_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ def setup_class(self):
self.jwst_asdf_url_2 = 'https://stsci.box.com/shared/static/d5k9z5j05dgfv6ljgie483w21kmpevni.fits' # noqa: E501

def test_no_data_label(self):
with pytest.raises(NotImplementedError, match='should be set'):
with pytest.raises(ValueError, match='app is None'):
_parse_image(None, None, None, False)

def test_hdulist_no_image(self, imviz_helper):
Expand Down Expand Up @@ -94,13 +94,14 @@ def test_parse_numpy_array_3d(self, imviz_helper, manual_loop):
n_slices = 5
slice_shape = (2, 3)
arr = np.stack([np.zeros(slice_shape) + i for i in range(n_slices)])
data_label = 'my_slices'

if not manual_loop:
# We use higher level load_data() here to make sure linking does not crash.
imviz_helper.load_data(arr, data_label='my_slices')
imviz_helper.load_data(arr, data_label=data_label)
else:
for i in range(n_slices):
imviz_helper.load_data(arr[i, :, :], data_label=f'my_slices_{i}', do_link=False)
imviz_helper.load_data(arr[i, :, :], data_label='my_slices', do_link=False)
imviz_helper.link_data(error_on_fail=True)

assert len(imviz_helper.app.data_collection) == n_slices
Expand All @@ -109,7 +110,7 @@ def test_parse_numpy_array_3d(self, imviz_helper, manual_loop):
for i in range(n_slices):
data = imviz_helper.app.data_collection[i]
comp = data.get_component('DATA')
assert data.label == f'my_slices_{i}'
assert data.label == (f'my_slices ({i})' if i > 0 else 'my_slices')
assert data.shape == slice_shape
assert_array_equal(comp.data, i)

Expand Down Expand Up @@ -336,7 +337,7 @@ def test_parse_jwst_nircam_level2(self, imviz_helper):

imviz_helper.load_data(pf, ext='SCI', data_label='TEST', show_in_viewer=False)
data = imviz_helper.app.data_collection[1]
assert data.label.endswith('[DATA] 1')
assert data.label.endswith('[DATA] (1)')

# Load all extensions
imviz_helper.app.data_collection.clear()
Expand Down Expand Up @@ -453,14 +454,14 @@ def test_parse_hst_drz(self, imviz_helper):
# Default behavior: Load first image
imviz_helper.load_data(pf, show_in_viewer=False)
data = imviz_helper.app.data_collection[3]
assert data.label.startswith('imviz_data|') and data.label.endswith('[SCI,1]')
assert data.label.startswith('Unknown HDU object') and data.label.endswith('[SCI,1]')
assert_allclose(data.meta['PHOTFLAM'], 7.8711728E-20)
assert 'SCI,1' in data.components

# Request specific extension (name only), use given label
imviz_helper.load_data(pf, ext='CTX', show_in_viewer=False)
data = imviz_helper.app.data_collection[4]
assert data.label.startswith('imviz_data|') and data.label.endswith('[CTX,1]')
assert data.label.startswith('Unknown HDU object') and data.label.endswith('[CTX,1]')
assert data.meta['EXTNAME'] == 'CTX'
assert 'CTX,1' in data.components

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,8 @@ def test_cubeviz_collapse_fluxunits(spectrum1d_cube_custom_fluxunit, spectra_flu
cubeviz_helper.app.get_viewer('spectrum-viewer').state.function = function

lineflux_result = _calculate_line_flux(cubeviz_helper)

autocollapsed_spectrum_unit = cubeviz_helper.specviz.get_spectra()[data_label + "[FLUX]"
].flux.unit
autocollapsed_spectrum_unit = (cubeviz_helper.
specviz.get_spectra()[f"{data_label}[FLUX]"].flux.unit)
# Futureproofing: Eventually Cubeviz autocollapse will change the flux units of the
# spectra depending on whether the spectrum was collapsed OVER SPAXELS or not. Only
# do the assertion check if we KNOW what the expected lineflux results should be
Expand Down
12 changes: 4 additions & 8 deletions jdaviz/configs/specviz/plugins/parsers.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
import base64
import pathlib
import uuid

import numpy as np
from astropy.io.registry import IORegistryError
Expand Down Expand Up @@ -31,18 +29,16 @@ def specviz_spectrum1d_parser(app, data, data_label=None, format=None, show_in_v
Reference name for the viewer
"""
spectrum_viewer_reference_name = app._jdaviz_helper._default_spectrum_viewer_reference_name
# If no data label is assigned, give it a unique identifier
# If no data label is assigned, give it a unique name
if not data_label:
data_label = "specviz_data|" + str(
base64.b85encode(uuid.uuid4().bytes), "utf-8"
)
data_label = app.return_data_label(data, alt_name="specviz_data")

if isinstance(data, SpectrumCollection):
raise TypeError("SpectrumCollection detected."
" Please provide a Spectrum1D or SpectrumList")
elif isinstance(data, Spectrum1D):
data = [data]
data_label = [data_label]
data_label = [app.return_data_label(data_label, alt_name="specviz_data")]
# No special processing is needed in this case, but we include it for completeness
elif isinstance(data, SpectrumList):
pass
Expand All @@ -54,7 +50,7 @@ def specviz_spectrum1d_parser(app, data, data_label=None, format=None, show_in_v
if path.is_file():
try:
data = [Spectrum1D.read(str(path), format=format)]
data_label = [data_label]
data_label = [app.return_data_label(data_label, alt_name="specviz_data")]
except IORegistryError:
# Multi-extension files may throw a registry error
data = SpectrumList.read(str(path), format=format)
Expand Down
6 changes: 5 additions & 1 deletion jdaviz/configs/specviz2d/plugins/parsers.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,11 @@ def spec2d_1d_parser(app, data_obj, data_label=None, show_in_viewer=True):

data_obj = Spectrum1D(flux, spectral_axis=spectral_axis, meta=metadata)

app.data_collection[data_label] = data_obj
data_label = app.return_data_label(data_label, alt_name="specviz2d_data")
app.data_collection[data_label] = data_obj

else:
raise NotImplementedError("Spectrum2d parser only takes a filename")

if show_in_viewer:
app.add_data_to_viewer(
Expand Down
Loading

0 comments on commit c946d8f

Please sign in to comment.