Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Export spectral subsets #2843

Merged
merged 17 commits into from
May 2, 2024
Merged
5 changes: 5 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ New Features

- Adding Data Quality plugin for Imviz and Cubeviz. [#2767, #2817]

- Enable exporting spectral regions to ECSV files readable by ``astropy.table.QTable`` or
``specutils.SpectralRegion`` [#2843]

Cubeviz
^^^^^^^

Expand Down Expand Up @@ -73,6 +76,8 @@ Other Changes and Additions

- Line menu in Redshift from Centroid section of Line Analysis now shows values in current units. [#2816, #2831]

- Bump required specutils version to 1.15. [#2843]

3.9.2 (unreleased)
==================

Expand Down
7 changes: 6 additions & 1 deletion docs/specviz/export_data.rst
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ those data currently back into your Jupyter notebook:

specviz.get_spectra()

which yields a either a single `specutils.Spectrum1D` object or a dictionary of
which yields a either a single `specutils.Spectrum1D` object or a dictionary of
`specutils.Spectrum1D` (if there are multiple displayed spectra) that you can
manipulate however you wish. You can then load the modified spectrum back into
the notebook via the API described in :ref:`specviz-import-api`.
Expand Down Expand Up @@ -70,6 +70,11 @@ To extract the spectral region you want:

myregion = regions["Subset 2"]

.. seealso::

:ref:`Export From Plugins <specviz-plugins>`
Spectral region subsets can also be exported to disk as an ECSV file.

.. _specviz-export-model:

Model Fits
Expand Down
6 changes: 5 additions & 1 deletion docs/specviz/plugins.rst
Original file line number Diff line number Diff line change
Expand Up @@ -320,4 +320,8 @@ using the |icon-line-select| (line selector) tool in the spectrum viewer.
Export
======

This plugin allows a given viewer's plot to be exported to various image formats.
This plugin allows exporting a the contents of a viewer or a plot within a plugin to various image formats.
rosteen marked this conversation as resolved.
Show resolved Hide resolved
Additionally, spatial and spectral regions can be exported to files, as astropy regions saves as FITS or REG
rosteen marked this conversation as resolved.
Show resolved Hide resolved
files (in the case of spatial regions), or as ECSV files in the case of spectral regions via specutils SpectralRegion.
Note that multiple spectral regions can be saved out to the same file, as long as they are subregions of a single
subset rather than independent subsets.
61 changes: 55 additions & 6 deletions jdaviz/configs/default/plugins/export/export.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,6 @@ def __init__(self, *args, **kwargs):
self.dataset.filters = ['is_not_wcs_only', 'not_child_layer',
'from_plugin']

# NOTE: if/when adding support for spectral subsets, update the languange in the UI
self.subset.filters = ['is_spatial']

viewer_format_options = ['png', 'svg']
if self.config == 'cubeviz':
if not self.app.state.settings.get('server_is_remote'):
Expand All @@ -135,7 +132,9 @@ def __init__(self, *args, **kwargs):
selected='plugin_table_format_selected',
manual_options=plugin_table_format_options)

subset_format_options = ['fits', 'reg']
subset_format_options = [{'label': 'fits', 'value': 'fits', 'disabled': False},
{'label': 'reg', 'value': 'reg', 'disabled': False},
{'label': 'ecsv', 'value': 'ecsv', 'disabled': True}]
self.subset_format = SelectPluginComponent(self,
items='subset_format_items',
selected='subset_format_selected',
Expand Down Expand Up @@ -226,6 +225,8 @@ def _sync_singleselect(self, event):
if name != attr:
setattr(self, attr, '')
if attr == 'subset_selected':
if self.subset.selected != '':
self._update_subset_format_disabled()
self._set_subset_not_supported_msg()
if attr == 'dataset_selected':
self._set_dataset_not_supported_msg()
Expand Down Expand Up @@ -258,6 +259,46 @@ def _is_filename_changed(self, event):
# Clear overwrite warning when user changes filename
self.overwrite_warn = False

def _update_subset_format_disabled(self):
new_items = []
if self.subset.selected is not None:
subset = self.app.get_subsets(self.subset.selected)
if self.app._is_subset_spectral(subset[0]):
good_formats = ["ecsv"]
else:
good_formats = ["fits", "reg"]
for item in self.subset_format_items:
if item["label"] in good_formats:
item["disabled"] = False
else:
item["disabled"] = True
if item["label"] == self.subset_format.selected:
self.subset_format.selected = good_formats[0]
new_items.append(item)
self.subset_format_items = []
self.subset_format_items = new_items

@observe('subset_format_selected')
def _disable_subset_format_combo(self, event):
# Disable selecting a bad subset+format combination from the API
if self.subset.selected == '' or self.subset.selected is None:
return
subset = self.app.get_subsets(self.subset.selected)
bad_combo = False
if self.app._is_subset_spectral(subset[0]):
if event['new'] != "ecsv":
bad_combo = True
elif event['new'] == "ecsv":
bad_combo = True

if bad_combo:
# Set back to a good value and raise error
good_format = [format["label"] for format in self.subset_format_items if
format["disabled"] is False][0]
self.subset_format.selected = good_format
raise ValueError(f"Cannot export {self.subset.selected} in {event['new']}"
f" format, reverting selection to {self.subset_format.selected}")
Comment on lines +299 to +300
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kecnry, do you think this error message makes clear enough what is happening?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you want to go the revert route, you could also use self.subset_format.select_previous() (which hopefully should be valid still... I can't think of a case where it wouldn't)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but the message makes sense to me, let me test it to see if I can find a way to break it and otherwise should be good to approve.


def _set_subset_not_supported_msg(self, msg=None):
"""
Check if selected subset is spectral or composite, and warn and
Expand All @@ -269,7 +310,7 @@ def _set_subset_not_supported_msg(self, msg=None):
if self.subset.selected == '':
self.subset_invalid_msg = ''
elif self.app._is_subset_spectral(subset[0]):
self.subset_invalid_msg = 'Export for spectral subsets not yet supported.'
self.subset_invalid_msg = ''
elif len(subset) > 1:
self.subset_invalid_msg = 'Export for composite subsets not yet supported.'
else:
Expand Down Expand Up @@ -422,7 +463,11 @@ def export(self, filename=None, show_dialog=None, overwrite=False,
if raise_error_for_overwrite:
raise FileExistsError(f"{filename} exists but overwrite=False")
return
self.save_subset_as_region(selected_subset_label, filename)

if self.subset_format.selected in ('fits', 'reg'):
self.save_subset_as_region(selected_subset_label, filename)
elif self.subset_format.selected == 'ecsv':
self.save_subset_as_table(filename)

elif len(self.dataset.selected):
filetype = self.dataset_format.selected
Expand Down Expand Up @@ -655,6 +700,10 @@ def save_subset_as_region(self, selected_subset_label, filename):

region.write(filename, overwrite=True)

def save_subset_as_table(self, filename):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want an option to overwrite? Also does this need some path wrangling magic for the "standalone app"? For example, see implementation in this method:

def _write_moment_to_fits(self, overwrite=False, *args):

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I see that this plugin already has _normalize_filename method that you wrote recently. Should that be used explicitly?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_normalize_filename is already used before we get to this point, and handling overwrite is also handled in that method.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah OK, hard to tell from the diff. Thanks for the clarification!

region = self.app.get_subsets(subset_name=self.subset.selected)
region.write(filename)

def vue_interrupt_recording(self, *args): # pragma: no cover
self.movie_interrupt = True
# TODO: this will need updating when batch/multiselect support is added
Expand Down
18 changes: 10 additions & 8 deletions jdaviz/configs/default/plugins/export/export.vue
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
:items="viewer_format_items.map(i => i.label)"
label="Format"
hint="Image format for exporting viewers."
:disabled="viewer_selected.length == 0"
:disabled="viewer_selected.length == 0"
persistent-hint
>
</v-select>
Expand Down Expand Up @@ -126,9 +126,9 @@
</div>

<div v-if="subset_items.length > 0">
<span class="export-category">Spatial Subsets</span>
<span class="export-category">Subsets</span>
<v-row>
<span class="category-content v-messages v-messages__message text--secondary">Export spatial subset as astropy region.</span>
<span class="category-content v-messages v-messages__message text--secondary">Export spatial subset as astropy region or spectral subset as specutils SpectralRegion.</span>
</v-row>
<plugin-inline-select
:items="subset_items"
Expand All @@ -150,7 +150,9 @@
:menu-props="{ left: true }"
attach
v-model="subset_format_selected"
:items="subset_format_items.map(i => i.label)"
:items="subset_format_items"
item-text="label"
item-disabled="disabled"
label="Format"
hint="Format for exporting subsets."
:disabled="subset_selected == null || subset_selected.length == 0"
Expand Down Expand Up @@ -200,10 +202,10 @@
:single_select_allow_blank="false"
>
</plugin-inline-select>
<jupyter-widget
<jupyter-widget
v-if='plugin_plot_selected_widget.length > 0'
style="position: absolute; left: -100%"
:widget="plugin_plot_selected_widget"/>
:widget="plugin_plot_selected_widget"/>
<v-row class="row-min-bottom-padding">
<v-select
class="category-content"
Expand All @@ -222,7 +224,7 @@

<div style="display: grid; position: relative"> <!-- overlay container -->
<div style="grid-area: 1/1">

<plugin-auto-label
:value.sync="filename_value"
:default="filename_default"
Expand Down Expand Up @@ -297,7 +299,7 @@
display: block;
text-align: center;
overflow: hidden;
white-space: nowrap;
white-space: nowrap;
text-transform: uppercase;
color: gray;
font-weight: 500;
Expand Down
29 changes: 23 additions & 6 deletions jdaviz/configs/default/plugins/export/tests/test_export.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def test_basic_export_subsets_imviz(self, imviz_helper):

# test that invalid file extension raises an error
with pytest.raises(ValueError,
match=re.escape("x not one of ['fits', 'reg'], reverting selection to reg")): # noqa
match=re.escape("x not one of ['fits', 'reg', 'ecsv'], reverting selection to reg")): # noqa
export_plugin.subset_format.selected = 'x'

def test_not_implemented(self, cubeviz_helper, spectral_cube_wcs):
Expand All @@ -93,10 +93,6 @@ def test_not_implemented(self, cubeviz_helper, spectral_cube_wcs):

assert export_plugin.subset_invalid_msg == 'Export for composite subsets not yet supported.'

cubeviz_helper.app.session.edit_subset_mode.mode = NewMode
cubeviz_helper.app.get_viewer("spectrum-viewer").apply_roi(XRangeROI(5, 15.5))
assert 'Subset 2' not in export_plugin.subset.choices
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does deleting this from the test mean in terms of expected behavior?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It means Subset 2 is allowed to be in the subset choices now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But is it grayed out? If so, is there a way to check?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not grayed out, no. The output format choices may be, I can add a check for that.


def test_export_subsets_wcs(self, imviz_helper, spectral_cube_wcs):

# using cube WCS instead of 2d imaging wcs for consistancy with
Expand Down Expand Up @@ -193,9 +189,13 @@ def test_basic_export_subsets_cubeviz(self, cubeviz_helper, spectral_cube_wcs):

# test that invalid file extension raises an error
with pytest.raises(ValueError,
match=re.escape("x not one of ['fits', 'reg'], reverting selection to reg")): # noqa
match=re.escape("x not one of ['fits', 'reg', 'ecsv'], reverting selection to reg")): # noqa
export_plugin.subset_format.selected = 'x'

# Test that selecting disabled option raises an error
with pytest.raises(ValueError, match="Cannot export Subset 1 in ECSV format"):
export_plugin.subset_format.selected = 'ecsv'

# test that attempting to save a composite subset raises an error
cubeviz_helper.app.session.edit_subset_mode.mode = AndMode
cubeviz_helper.app.get_viewer('flux-viewer').apply_roi(CircularROI(xc=25, yc=25, radius=5))
Expand All @@ -205,6 +205,23 @@ def test_basic_export_subsets_cubeviz(self, cubeviz_helper, spectral_cube_wcs):
match='Subset can not be exported - Export for composite subsets not yet supported.'): # noqa
export_plugin.export()

# Test saving spectral subset
cubeviz_helper.app.session.edit_subset_mode.mode = NewMode
cubeviz_helper.app.get_viewer("spectrum-viewer").apply_roi(XRangeROI(5, 15.5))
export_plugin.subset.selected = 'Subset 2'

# Format should auto-update to first non-disabled entry
assert export_plugin.subset_format.selected == 'ecsv'
for format in export_plugin.subset_format.items:
if format['label'] != 'ecsv':
assert format['disabled']
else:
assert format['disabled'] is False

export_plugin.filename_value = "test_spectral_region"
export_plugin.export()
assert os.path.isfile('test_spectral_region.ecsv')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is roundtripping a requirement? That is, is Jdaviz expected to be able to re-ingest this file and re-creates the same spectral region?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe it is.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. I guess checking for the output content is correct or not is the job of specutils, so we don't have to do that here. Thanks for the clarification!



@pytest.mark.usefixtures('_jail')
def test_export_cubeviz_spectrum_viewer(cubeviz_helper, spectrum1d_cube):
Expand Down
2 changes: 1 addition & 1 deletion jdaviz/core/template_mixin.py
Original file line number Diff line number Diff line change
Expand Up @@ -725,7 +725,7 @@ def __init__(self, *args, **kwargs):
manual_options = [default_text] + manual_options
self._manual_options = manual_options

self.items = [{"label": opt} for opt in manual_options]
self.items = [{"label": opt} if isinstance(opt, str) else opt for opt in manual_options]
# set default values for traitlets
if default_text is not None:
self.selected = default_text
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ dependencies = [
"ipywidgets>=8.0.6",
"voila>=0.4,<0.5",
"pyyaml>=5.4.1",
"specutils>=1.9",
"specutils>=1.15",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is quite a jump. Do we need a change log for this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would be happy to add it to the change log if you think so.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't hurt?

"specreduce>=1.3.0,<1.4.0",
"photutils>=1.4",
"glue-astronomy>=0.10",
Expand Down
Loading