From 1a8c878261b388d546efca183ce607e824b35dba Mon Sep 17 00:00:00 2001 From: Hatice Karatay <66814693+haticekaratay@users.noreply.github.com> Date: Mon, 28 Oct 2024 13:30:53 -0400 Subject: [PATCH] Remove UI elements deprecated since 3.9 (#3256) * 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 --- CHANGES.rst | 1 + jdaviz/app.vue | 2 +- jdaviz/components/tooltip.vue | 2 - .../plugins/moment_maps/moment_maps.py | 43 ------------- .../plugins/moment_maps/moment_maps.vue | 60 ------------------- .../moment_maps/tests/test_moment_maps.py | 35 ++--------- .../spectral_extraction.py | 47 --------------- .../spectral_extraction.vue | 60 ------------------- .../tests/test_spectral_extraction.py | 34 ++--------- .../default/plugins/collapse/collapse.py | 46 -------------- .../default/plugins/collapse/collapse.vue | 60 ------------------- .../plugins/collapse/tests/test_collapse.py | 35 ++--------- 12 files changed, 16 insertions(+), 409 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 212c65fd9c..6031124d6e 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -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 ^^^^^ diff --git a/jdaviz/app.vue b/jdaviz/app.vue index 6ca646dc6f..5f67b89839 100644 --- a/jdaviz/app.vue +++ b/jdaviz/app.vue @@ -114,7 +114,7 @@
- {{ trayItem.label == 'Orientation' ? 'Orientation (prev. Links Control)' : trayItem.label }} + {{ trayItem.label }} diff --git a/jdaviz/components/tooltip.vue b/jdaviz/components/tooltip.vue index 341f97fa3b..a31a05a8ae 100644 --- a/jdaviz/components/tooltip.vue +++ b/jdaviz/components/tooltip.vue @@ -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', diff --git a/jdaviz/configs/cubeviz/plugins/moment_maps/moment_maps.py b/jdaviz/configs/cubeviz/plugins/moment_maps/moment_maps.py index ca4a099d1c..3aad940765 100644 --- a/jdaviz/configs/cubeviz/plugins/moment_maps/moment_maps.py +++ b/jdaviz/configs/cubeviz/plugins/moment_maps/moment_maps.py @@ -1,6 +1,3 @@ -import os -from pathlib import Path - import numpy as np import specutils from astropy import units as u @@ -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) @@ -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")) diff --git a/jdaviz/configs/cubeviz/plugins/moment_maps/moment_maps.vue b/jdaviz/configs/cubeviz/plugins/moment_maps/moment_maps.vue index 5cf4b7ea7b..3ffc5e73af 100644 --- a/jdaviz/configs/cubeviz/plugins/moment_maps/moment_maps.vue +++ b/jdaviz/configs/cubeviz/plugins/moment_maps/moment_maps.vue @@ -173,65 +173,5 @@ Cannot calculate moment: Must set reference wavelength for output in velocity units. - - Results - -
-
-
- - - - - - - - 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. - - - - - - Save as FITS - - - - -
-
- - - - - -
- A file with this name is already on disk. Overwrite? -
- - - - - Cancel - Overwrite - - - - - - - -
diff --git a/jdaviz/configs/cubeviz/plugins/moment_maps/tests/test_moment_maps.py b/jdaviz/configs/cubeviz/plugins/moment_maps/tests/test_moment_maps.py index 4448193356..3aab8e711c 100644 --- a/jdaviz/configs/cubeviz/plugins/moment_maps/tests/test_moment_maps.py +++ b/jdaviz/configs/cubeviz/plugins/moment_maps/tests/test_moment_maps.py @@ -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 @@ -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' @@ -258,7 +251,6 @@ 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) @@ -266,34 +258,15 @@ def test_write_momentmap(cubeviz_helper, spectrum1d_cube, tmp_path): 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 diff --git a/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.py b/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.py index f45d8db878..5d009cb76b 100644 --- a/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.py +++ b/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.py @@ -1,6 +1,4 @@ -import os from functools import cached_property -from pathlib import Path import numpy as np import astropy @@ -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) @@ -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'): diff --git a/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.vue b/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.vue index e30cb7d28d..2f0dbd12a6 100644 --- a/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.vue +++ b/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.vue @@ -271,65 +271,5 @@ - Results - -
-
-
- - - - - - - - - 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. - - - - - - Save as FITS - - - - -
-
- - - - - -
- A file with this name is already on disk. Overwrite? -
- - - - - Cancel - Overwrite - - - - - -
-
- diff --git a/jdaviz/configs/cubeviz/plugins/spectral_extraction/tests/test_spectral_extraction.py b/jdaviz/configs/cubeviz/plugins/spectral_extraction/tests/test_spectral_extraction.py index 9d50043f4b..e1f741e39f 100644 --- a/jdaviz/configs/cubeviz/plugins/spectral_extraction/tests/test_spectral_extraction.py +++ b/jdaviz/configs/cubeviz/plugins/spectral_extraction/tests/test_spectral_extraction.py @@ -144,7 +144,7 @@ def test_subset( assert_array_equal(collapsed_spec_2.uncertainty.array, expected_uncert) -def test_save_collapsed_to_fits(cubeviz_helper, spectrum1d_cube_with_uncerts, tmp_path): +def test_extracted_file_in_export_plugin(cubeviz_helper, spectrum1d_cube_with_uncerts, tmp_path): cubeviz_helper.load_data(spectrum1d_cube_with_uncerts) @@ -165,34 +165,10 @@ def test_save_collapsed_to_fits(cubeviz_helper, spectrum1d_cube_with_uncerts, tm assert extract_plugin._obj.filename == fname extract_plugin._obj.filename = str(fname_path) - # save output file with default name, make sure it exists - extract_plugin._obj.vue_save_as_fits() - assert fname_path.is_file() - - # read file back in, make sure it matches - dat = Spectrum1D.read(fname_path) - assert_array_equal(dat.data, extract_plugin._obj.extracted_spec.data) - assert dat.unit == extract_plugin._obj.extracted_spec.unit - - # make sure correct error message is raised when export_enabled is False - # this won't appear in UI, but just to be safe. - extract_plugin._obj.export_enabled = False - with pytest.raises( - ValueError, match="Writing out extracted spectrum to file is currently disabled"): - extract_plugin._obj.vue_save_as_fits() - extract_plugin._obj.export_enabled = True # set back to True - - # check that trying to overwrite without overwrite=True sets overwrite_warn to True, to - # display popup in UI - assert extract_plugin._obj.overwrite_warn is False - extract_plugin._obj.vue_save_as_fits() - assert extract_plugin._obj.overwrite_warn - - # check that writing out to a non existent directory fails as expected - extract_plugin._obj.filename = '/this/path/doesnt/exist.fits' - with pytest.raises(ValueError, match="Invalid path=/this/path/doesnt"): - extract_plugin._obj.vue_save_as_fits() - extract_plugin._obj.filename == fname # set back to original filename + label = extract_plugin._obj.add_results.label + export_plugin = cubeviz_helper.plugins['Export']._obj + + assert label in export_plugin.data_collection.labels def test_aperture_markers(cubeviz_helper, spectrum1d_cube): diff --git a/jdaviz/configs/default/plugins/collapse/collapse.py b/jdaviz/configs/default/plugins/collapse/collapse.py index 671051a51b..ad6382c0b1 100644 --- a/jdaviz/configs/default/plugins/collapse/collapse.py +++ b/jdaviz/configs/default/plugins/collapse/collapse.py @@ -1,5 +1,3 @@ -import os -from pathlib import Path import warnings from astropy.nddata import CCDData @@ -43,7 +41,6 @@ class Collapse(PluginTemplateMixin, DatasetSelectMixin, SpectralSubsetSelectMixi function_selected = Unicode('Sum').tag(sync=True) filename = Unicode().tag(sync=True) collapsed_spec_available = Bool(False).tag(sync=True) - overwrite_warn = Bool(False).tag(sync=True) # export_enabled controls whether saving to a file is enabled via the UI. This # is a temporary measure to allow server-installations to disable saving server-side until # saving client-side is supported @@ -143,46 +140,3 @@ def collapse(self, add_data=True): def vue_collapse(self, *args, **kwargs): self.collapse(add_data=True) - - def vue_save_as_fits(self, *args): - self._save_collapsed_spec_to_fits() - - def vue_overwrite_fits(self, *args): - """Attempt to force writing the file if the user confirms the desire to overwrite.""" - self.overwrite_warn = False - self._save_collapsed_spec_to_fits(overwrite=True) - - def _save_collapsed_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("Writing out collapsed cube 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.collapsed_spec.write(filename) - - # Let the user know where we saved the file. - self.hub.broadcast(SnackbarMessage( - f"Collapsed cube saved to {os.path.abspath(filename)}", - sender=self, color="success")) diff --git a/jdaviz/configs/default/plugins/collapse/collapse.vue b/jdaviz/configs/default/plugins/collapse/collapse.vue index 196cf52862..8a2e4f9a49 100644 --- a/jdaviz/configs/default/plugins/collapse/collapse.vue +++ b/jdaviz/configs/default/plugins/collapse/collapse.vue @@ -59,65 +59,5 @@ :api_hints_enabled="api_hints_enabled" @click:action="collapse" > - - Results - -
-
-
- - - - - - - - - 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. - - - - - - Save as FITS - - - - -
-
- - - - - -
- A file with this name is already on disk. Overwrite? -
- - - - - Cancel - Overwrite - - - - - - -
diff --git a/jdaviz/configs/default/plugins/collapse/tests/test_collapse.py b/jdaviz/configs/default/plugins/collapse/tests/test_collapse.py index d140afaeed..eec8067aa1 100644 --- a/jdaviz/configs/default/plugins/collapse/tests/test_collapse.py +++ b/jdaviz/configs/default/plugins/collapse/tests/test_collapse.py @@ -1,6 +1,5 @@ import numpy as np import pytest -from astropy.nddata import CCDData from astropy import units as u from specutils import Spectrum1D @@ -38,7 +37,7 @@ def test_linking_after_collapse(cubeviz_helper, spectral_cube_wcs): assert dc.external_links[3].cids2[0] == dc[-1].pixel_component_ids[0] -def test_save_collapsed_to_fits(cubeviz_helper, spectral_cube_wcs, tmp_path): +def test_collapsed_to_extract_plugin(cubeviz_helper, spectral_cube_wcs, tmp_path): cubeviz_helper.load_data(Spectrum1D(flux=np.ones((3, 4, 5)) * u.nJy, wcs=spectral_cube_wcs)) @@ -58,31 +57,7 @@ def test_save_collapsed_to_fits(cubeviz_helper, spectral_cube_wcs, tmp_path): assert collapse_plugin._obj.filename == fname collapse_plugin._obj.filename = str(tmp_path / fname) - # save output file with default name, make sure it exists - collapse_plugin._obj.vue_save_as_fits() - assert (tmp_path / fname).is_file() - - # read file back in, make sure it matches - dat = CCDData.read(collapse_plugin._obj.filename) - np.testing.assert_array_equal(dat.data, collapse_plugin._obj.collapsed_spec.data) - assert dat.unit == collapse_plugin._obj.collapsed_spec.unit - - # make sure correct error message is raised when export_enabled is False - # this won't appear in UI, but just to be safe. - collapse_plugin._obj.export_enabled = False - msg = "Writing out collapsed cube to file is currently disabled" - with pytest.raises(ValueError, match=msg): - collapse_plugin._obj.vue_save_as_fits() - collapse_plugin._obj.export_enabled = True # set back to True - - # check that trying to overwrite without overwrite=True sets overwrite_warn to True, to - # display popup in UI - assert collapse_plugin._obj.overwrite_warn is False - collapse_plugin._obj.vue_save_as_fits() - assert collapse_plugin._obj.overwrite_warn - - # check that writing out to a non existent directory fails as expected - collapse_plugin._obj.filename = '/this/path/doesnt/exist.fits' - with pytest.raises(ValueError, match="Invalid path=/this/path/doesnt"): - collapse_plugin._obj.vue_save_as_fits() - collapse_plugin._obj.filename == fname # set back to original filename + label = collapse_plugin._obj.add_results.label + export_plugin = cubeviz_helper.plugins['Export']._obj + + assert label in export_plugin.data_collection.labels