Skip to content

Commit

Permalink
Remove UI elements deprecated since 3.9 (#3256)
Browse files Browse the repository at this point in the history
* Remove UI elements deprecated since 3.9

* Add change log

* Remove deprecated save as fits in spectral extraction plugin

* Remove unused tooltips

* Remove unused vue write methods

* Update tests after removing the deprecated methods

* Update change log

* Update tests and fix code style

* Update changelog

* Update the changelog
  • Loading branch information
haticekaratay authored and rosteen committed Oct 29, 2024
1 parent 12a409d commit 1a8c878
Show file tree
Hide file tree
Showing 12 changed files with 16 additions and 409 deletions.
1 change: 1 addition & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ Bug Fixes

Cubeviz
^^^^^^^
- Removed the deprecated ``save as fits`` option from the Collapse, Moment Maps, and Spectral Extraction plugins; use the Export plugin instead. [#3256]

Imviz
^^^^^
Expand Down
2 changes: 1 addition & 1 deletion jdaviz/app.vue
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@
<div v-if="trayItem.is_relevant && trayItemVisible(trayItem, state.tray_items_filter)">
<v-expansion-panel-header >
<j-tooltip :tipid="trayItem.name">
{{ trayItem.label == 'Orientation' ? 'Orientation (prev. Links Control)' : trayItem.label }}
{{ trayItem.label }}
</j-tooltip>
</v-expansion-panel-header>
<v-expansion-panel-content style="margin-left: -12px; margin-right: -12px;">
Expand Down
2 changes: 0 additions & 2 deletions jdaviz/components/tooltip.vue
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,7 @@ const tooltips = {
'plugin-line-lists-spectral-range': 'Toggle filter to only lines observable within the range of the Spectrum Viewer',
'plugin-line-analysis-sync-identify': 'Lock/unlock selection with identified line',
'plugin-line-analysis-assign': 'Assign the centroid wavelength and update the redshift',
'plugin-moment-save-fits': 'Save moment map as FITS file',
'plugin-extract-save-fits': 'Save spectral extraction as FITS file',
'plugin-collapse-save-fits': 'Save collapsed cube as FITS file',
'plugin-link-apply': 'Apply linking to data',
'plugin-footprints-color-picker': 'Change the color of the footprint overlay',
'plugin-dq-show-all': 'Show all quality flags',
Expand Down
43 changes: 0 additions & 43 deletions jdaviz/configs/cubeviz/plugins/moment_maps/moment_maps.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
import os
from pathlib import Path

import numpy as np
import specutils
from astropy import units as u
Expand Down Expand Up @@ -80,7 +77,6 @@ class MomentMap(PluginTemplateMixin, DatasetSelectMixin, SpectralSubsetSelectMix
n_moment = IntHandleEmpty(0).tag(sync=True)
filename = Unicode().tag(sync=True)
moment_available = Bool(False).tag(sync=True)
overwrite_warn = Bool(False).tag(sync=True)
output_unit_items = List().tag(sync=True)
output_radio_items = List().tag(sync=True)
output_unit_selected = Unicode().tag(sync=True)
Expand Down Expand Up @@ -360,45 +356,6 @@ def spectral_unit_selected(self):
def vue_calculate_moment(self, *args):
self.calculate_moment(add_data=True)

def vue_save_as_fits(self, *args):
self._write_moment_to_fits()

def vue_overwrite_fits(self, *args):
"""Attempt to force writing the moment map if the user confirms the desire to overwrite."""
self.overwrite_warn = False
self._write_moment_to_fits(overwrite=True)

def _write_moment_to_fits(self, overwrite=False, *args):
if self.moment is None or not self.filename: # pragma: no cover
return
if not self.export_enabled:
# this should never be triggered since this is intended for UI-disabling and the
# UI section is hidden, but would prevent any JS-hacking
raise ValueError("Writing out moment map to file is currently disabled")

# Make sure file does not end up in weird places in standalone mode.
path = os.path.dirname(self.filename)
if path and not os.path.exists(path):
raise ValueError(f"Invalid path={path}")
elif (not path or path.startswith("..")) and os.environ.get("JDAVIZ_START_DIR", ""): # noqa: E501 # pragma: no cover
filename = Path(os.environ["JDAVIZ_START_DIR"]) / self.filename
else:
filename = Path(self.filename).resolve()

if filename.exists():
if overwrite:
# Try to delete the file
filename.unlink()
if filename.exists():
# Warn the user if the file still exists
raise FileExistsError(f"Unable to delete {filename}. Check user permissions.")
else:
self.overwrite_warn = True
return

filename = str(filename)
self.moment.write(filename)

# Let the user know where we saved the file.
self.hub.broadcast(SnackbarMessage(
f"Moment map saved to {os.path.abspath(filename)}", sender=self, color="success"))
60 changes: 0 additions & 60 deletions jdaviz/configs/cubeviz/plugins/moment_maps/moment_maps.vue
Original file line number Diff line number Diff line change
Expand Up @@ -173,65 +173,5 @@
Cannot calculate moment: Must set reference wavelength for output in velocity units.
</span>
</v-row>

<j-plugin-section-header v-if="moment_available && export_enabled">Results</j-plugin-section-header>

<div style="display: grid; position: relative"> <!-- overlay container -->
<div style="grid-area: 1/1">
<div v-if="moment_available && export_enabled">
<v-row>
<v-text-field
v-model="filename"
label="Filename"
hint="Export the latest calculated moment map"
:rules="[() => !!filename || 'This field is required']"
persistent-hint>
</v-text-field>
</v-row>

<v-row>
<span class="v-messages v-messages__message text--secondary" style="color: red !important">
DeprecationWarning: Save as FITS functionality has moved to the Export plugin as of v3.9 and will be removed from Moment Maps plugin in a future release.
</span>
</v-row>

<v-row justify="end">
<j-tooltip tipid='plugin-moment-save-fits'>
<v-btn color="primary" text @click="save_as_fits">Save as FITS</v-btn>

</j-tooltip>
</v-row>

</div>
</div>

<v-overlay
absolute
opacity=1.0
:value="overwrite_warn && export_enabled"
:zIndex=3
style="grid-area: 1/1;
margin-left: -24px;
margin-right: -24px">

<v-card color="transparent" elevation=0 >
<v-card-text width="100%">
<div class="white--text">
A file with this name is already on disk. Overwrite?
</div>
</v-card-text>

<v-card-actions>
<v-row justify="end">
<v-btn tile small color="primary" class="mr-2" @click="overwrite_warn=false">Cancel</v-btn>
<v-btn tile small color="accent" class="mr-4" @click="overwrite_fits" >Overwrite</v-btn>
</v-row>
</v-card-actions>
</v-card>

</v-overlay>


</div>
</j-tray-plugin>
</template>
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
import os
import warnings
from pathlib import Path

import numpy as np
import pytest
from astropy import units as u
from astropy.io import fits
from astropy.nddata import CCDData
from astropy.tests.helper import assert_quantity_allclose
from astropy.wcs import WCS
Expand Down Expand Up @@ -144,11 +142,6 @@ def test_moment_calculation(cubeviz_helper, spectrum1d_cube,
"World 13h39m59.9731s +27d00m00.3600s (ICRS)",
"204.9998877673 27.0001000000 (deg)")

assert mm._obj.filename == 'moment0_test_FLUX.fits' # Auto-populated on calculate.
mm._obj.filename = str(tmp_path / mm._obj.filename) # But we want it in tmp_path for testing.
mm._obj.vue_save_as_fits()
assert os.path.isfile(mm._obj.filename)

mm._obj.n_moment = 1
mm._obj.output_unit_selected = "Spectral Unit"
assert mm._obj.results_label == 'moment 1'
Expand Down Expand Up @@ -258,42 +251,22 @@ def test_write_momentmap(cubeviz_helper, spectrum1d_cube, tmp_path):

# Simulate an existing file on disk to check for overwrite warning
test_file = tmp_path / "test_file.fits"
test_file_str = str(test_file)
existing_sentinel_text = "This is a simulated, existing file on disk"
test_file.write_text(existing_sentinel_text)

with warnings.catch_warnings():
warnings.filterwarnings("ignore", message="No observer defined on WCS.*")
cubeviz_helper.load_data(spectrum1d_cube, data_label='test')
plugin = cubeviz_helper.plugins['Moment Maps']
moment = plugin.calculate_moment()

# By default, we shouldn't be warning the user of anything
assert plugin._obj.overwrite_warn is False
plugin.calculate_moment()

# Attempt to write the moment map to the same file we created earlier.
plugin._obj.filename = test_file_str
plugin._obj.vue_save_as_fits()

# We should get an overwrite warning
assert plugin._obj.overwrite_warn is True
# and shouldn't have written anything (the file should be intact)
assert test_file.read_text() == existing_sentinel_text

# Force overwrite the existing file
plugin._obj.vue_overwrite_fits()

# Read back in the file and check that it is equal to the one we calculated
with fits.open(test_file_str) as pf:
assert_allclose(pf[0].data, moment.data)
w = WCS(pf[0].header)
sky = w.pixel_to_world(0, 0)
assert_allclose(sky.ra.deg, 204.9998877673)
assert_allclose(sky.dec.deg, 27.0001)
label = plugin._obj.add_results.label
export_plugin = cubeviz_helper.plugins['Export']._obj

plugin._obj.filename = "fake_path/test_file.fits"
with pytest.raises(ValueError, match="Invalid path"):
plugin._obj.vue_save_as_fits()
assert label in export_plugin.data_collection.labels


@pytest.mark.remote_data
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
import os
from functools import cached_property
from pathlib import Path

import numpy as np
import astropy
Expand Down Expand Up @@ -105,7 +103,6 @@ class SpectralExtraction(PluginTemplateMixin, ApertureSubsetSelectMixin,
function_selected = Unicode('Sum').tag(sync=True)
filename = Unicode().tag(sync=True)
extraction_available = Bool(False).tag(sync=True)
overwrite_warn = Bool(False).tag(sync=True)

results_units = Unicode().tag(sync=True)
spectrum_y_units = Unicode().tag(sync=True)
Expand Down Expand Up @@ -672,50 +669,6 @@ def vue_spectral_extraction(self, *args, **kwargs):
def vue_create_bg_spec(self, *args, **kwargs):
self.extract_bg_spectrum(add_data=True)

def vue_save_as_fits(self, *args):
self._save_extracted_spec_to_fits()

def vue_overwrite_fits(self, *args):
"""Attempt to force writing the spectral extraction if the user
confirms the desire to overwrite."""
self.overwrite_warn = False
self._save_extracted_spec_to_fits(overwrite=True)

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

if not self.export_enabled:
# this should never be triggered since this is intended for UI-disabling and the
# UI section is hidden, but would prevent any JS-hacking
raise ValueError(f"Writing out extracted {self.resulting_product_name} to file is currently disabled") # noqa

# Make sure file does not end up in weird places in standalone mode.
path = os.path.dirname(self.filename)
if path and not os.path.exists(path):
raise ValueError(f"Invalid path={path}")
elif (not path or path.startswith("..")) and os.environ.get("JDAVIZ_START_DIR", ""): # noqa: E501 # pragma: no cover
filename = Path(os.environ["JDAVIZ_START_DIR"]) / self.filename
else:
filename = Path(self.filename).resolve()

if filename.exists():
if overwrite:
# Try to delete the file
filename.unlink()
if filename.exists():
# Warn the user if the file still exists
raise FileExistsError(f"Unable to delete {filename}. Check user permissions.")
else:
self.overwrite_warn = True
return

filename = str(filename)
self.extracted_spec.write(filename)

# Let the user know where we saved the file.
self.hub.broadcast(SnackbarMessage(
f"Extracted {self.resulting_product_name} saved to {os.path.abspath(filename)}",
sender=self, color="success"))

@observe('aperture_selected', 'function_selected')
def _set_default_results_label(self, event={}):
if not hasattr(self, 'aperture'):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -271,65 +271,5 @@
</v-alert>
</plugin-add-results>

<j-plugin-section-header v-if="extraction_available && export_enabled">Results</j-plugin-section-header>

<div style="display: grid; position: relative"> <!-- overlay container -->
<div style="grid-area: 1/1">
<div v-if="extraction_available && export_enabled">

<v-row>
<v-text-field
v-model="filename"
label="Filename"
hint="Export the latest extracted spectrum."
:rules="[() => !!filename || 'This field is required']"
persistent-hint>
</v-text-field>
</v-row>

<v-row>
<span class="v-messages v-messages__message text--secondary" style="color: red !important">
DeprecationWarning: Save as FITS functionality has moved to the Export plugin as of v3.9 and will be removed from here in a future release.
</span>
</v-row>

<v-row justify="end">
<j-tooltip tipid='plugin-extract-save-fits'>
<v-btn color="primary" text @click="save_as_fits">Save as FITS</v-btn>

</j-tooltip>
</v-row>

</div>
</div>

<v-overlay
absolute
opacity=1.0
:value="overwrite_warn && export_enabled"
:zIndex=3
style="grid-area: 1/1;
margin-left: -24px;
margin-right: -24px">

<v-card color="transparent" elevation=0 >
<v-card-text width="100%">
<div class="white--text">
A file with this name is already on disk. Overwrite?
</div>
</v-card-text>

<v-card-actions>
<v-row justify="end">
<v-btn tile small color="primary" class="mr-2" @click="overwrite_warn=false">Cancel</v-btn>
<v-btn tile small color="accent" class="mr-4" @click="overwrite_fits" >Overwrite</v-btn>
</v-row>
</v-card-actions>
</v-card>

</v-overlay>
</div>
</div>

</j-tray-plugin>
</template>
Loading

0 comments on commit 1a8c878

Please sign in to comment.