From 23b67cf172db655144be3af8d7ce2295c39c46b8 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Tue, 28 May 2024 16:19:17 -0400 Subject: [PATCH 01/11] show default filepath in Export plugin --- CHANGES.rst | 3 +++ jdaviz/configs/default/plugins/export/export.py | 16 +++++++++++++++- jdaviz/configs/default/plugins/export/export.vue | 12 ++++++++++++ 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index ba3cd98f7c..ae45138f42 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -16,6 +16,9 @@ New Features - The colormap menu for image layers now shows in-line previews of the colormaps. [#2900] +- Display default filepath in Export plugin, re-enable API exporting, enable relative and absolute + path exports from the UI. [#2896] + Cubeviz ^^^^^^^ diff --git a/jdaviz/configs/default/plugins/export/export.py b/jdaviz/configs/default/plugins/export/export.py index 73978196d1..24fddac9ae 100644 --- a/jdaviz/configs/default/plugins/export/export.py +++ b/jdaviz/configs/default/plugins/export/export.py @@ -81,6 +81,8 @@ class Export(PluginTemplateMixin, ViewerSelectMixin, SubsetSelectMixin, filename_auto = Bool(True).tag(sync=True) filename_invalid_msg = Unicode('').tag(sync=True) + default_filepath = Unicode().tag(sync=True) + # if selected subset is spectral or composite, display message and disable export subset_invalid_msg = Unicode().tag(sync=True) data_invalid_msg = Unicode().tag(sync=True) @@ -254,6 +256,12 @@ def _set_filename_default(self, *args, **kwargs): else: self.filename_default = '' + # Call to initially set the default filepath + if hasattr(self, 'viewer_format') and self.filename_default: + self._normalize_filename(filename=self.filename_default, + filetype=self.viewer_format.selected, + default_path=True) + @observe('filename_value') def _is_filename_changed(self, event): # Clear overwrite warning when user changes filename @@ -333,7 +341,7 @@ def _set_dataset_not_supported_msg(self, msg=None): else: self.data_invalid_msg = '' - def _normalize_filename(self, filename=None, filetype=None, overwrite=False): + def _normalize_filename(self, filename=None, filetype=None, overwrite=False, default_path=False): # noqa: E501 # Make sure filename is valid and file does not end up in weird places in standalone mode. if not filename: raise ValueError("Invalid filename") @@ -349,6 +357,12 @@ def _normalize_filename(self, filename=None, filetype=None, overwrite=False): elif ((not filepath or str(filepath).startswith(".")) and os.environ.get("JDAVIZ_START_DIR", "")): # noqa: E501 # pragma: no cover filename = os.environ["JDAVIZ_START_DIR"] / filename + # Set the default filepath to inform user where file will be exported to + if default_path: + if not filepath.is_absolute(): + abs_path = filepath.resolve() + self.default_filepath = str(abs_path) + if filename.exists() and not overwrite: self.overwrite_warn = True else: diff --git a/jdaviz/configs/default/plugins/export/export.vue b/jdaviz/configs/default/plugins/export/export.vue index 16354a61bb..4fa516f152 100644 --- a/jdaviz/configs/default/plugins/export/export.vue +++ b/jdaviz/configs/default/plugins/export/export.vue @@ -225,6 +225,18 @@
+ + + + + + Date: Thu, 30 May 2024 15:17:50 -0400 Subject: [PATCH 02/11] update filepath if ., .., and/or / provided by user in filename --- jdaviz/configs/default/plugins/export/export.py | 9 +++++++++ jdaviz/configs/default/plugins/export/export.vue | 12 ++++++------ 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/jdaviz/configs/default/plugins/export/export.py b/jdaviz/configs/default/plugins/export/export.py index 24fddac9ae..31c6d5f7a8 100644 --- a/jdaviz/configs/default/plugins/export/export.py +++ b/jdaviz/configs/default/plugins/export/export.py @@ -264,6 +264,15 @@ def _set_filename_default(self, *args, **kwargs): @observe('filename_value') def _is_filename_changed(self, event): + # Update the UI Filepath if relative or absolute paths are provided + # by user via self.filename_value + if '/' in self.filename_value and \ + ('.' in self.filename_value or '..' in self.filename_value): + abs_path = Path(self.filename_value).resolve() + self.default_filepath = str(abs_path) + elif '/' in self.filename_value: + self.default_filepath = str(self.filename_value) + # Clear overwrite warning when user changes filename self.overwrite_warn = False diff --git a/jdaviz/configs/default/plugins/export/export.vue b/jdaviz/configs/default/plugins/export/export.vue index 4fa516f152..9a09340d04 100644 --- a/jdaviz/configs/default/plugins/export/export.vue +++ b/jdaviz/configs/default/plugins/export/export.vue @@ -222,28 +222,28 @@
-
-
- +
+
+ From 95031fa11c48717bbaf1eebde205eb8f1cc28603 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Tue, 11 Jun 2024 16:15:30 -0400 Subject: [PATCH 03/11] add ~ relative path, revert to PosixPath instead of string, UI/API export to filepath --- .../configs/default/plugins/export/export.py | 63 ++++++++++--------- 1 file changed, 33 insertions(+), 30 deletions(-) diff --git a/jdaviz/configs/default/plugins/export/export.py b/jdaviz/configs/default/plugins/export/export.py index 31c6d5f7a8..1e2f0330f1 100644 --- a/jdaviz/configs/default/plugins/export/export.py +++ b/jdaviz/configs/default/plugins/export/export.py @@ -179,7 +179,7 @@ def user_api(self): # TODO: backwards compat for save_figure, save_movie, # i_start, i_end, movie_fps, movie_filename # TODO: expose export method once API is finalized - + # is the above comment still needed or can it be removed? expose = ['viewer', 'viewer_format', 'dataset', 'dataset_format', 'subset', 'subset_format', @@ -266,8 +266,12 @@ def _set_filename_default(self, *args, **kwargs): def _is_filename_changed(self, event): # Update the UI Filepath if relative or absolute paths are provided # by user via self.filename_value - if '/' in self.filename_value and \ - ('.' in self.filename_value or '..' in self.filename_value): + if '~' in self.filename_value and '/' in self.filename_value: + expanded_path = os.path.expanduser(self.filename_value) + abs_path = Path(expanded_path).resolve() + self.default_filepath = str(abs_path) + elif '/' in self.filename_value and \ + ('.' in self.filename_value or '..' in self.filename_value): abs_path = Path(self.filename_value).resolve() self.default_filepath = str(abs_path) elif '/' in self.filename_value: @@ -377,7 +381,7 @@ def _normalize_filename(self, filename=None, filetype=None, overwrite=False, def else: self.overwrite_warn = False - return str(filename) + return filename @with_spinner() def export(self, filename=None, show_dialog=None, overwrite=False, @@ -532,31 +536,30 @@ def vue_overwrite_from_ui(self, *args, **kwargs): f"Exported to {filename} (overwrite)", sender=self, color="success")) def save_figure(self, viewer, filename=None, filetype="png", show_dialog=False): - if filetype == "png": - if filename is None or show_dialog: - viewer.figure.save_png(str(filename) if filename is not None else None) - else: - # support writing without save dialog - # https://github.com/bqplot/bqplot/pull/1397 - def on_img_received(data): - try: - with filename.open(mode='bw') as f: - f.write(data) - except Exception as e: - self.hub.broadcast(SnackbarMessage( - f"{self.viewer.selected} failed to export to {str(filename)}: {e}", - sender=self, color="error")) - finally: - self.hub.broadcast(SnackbarMessage( - f"{self.viewer.selected} exported to {str(filename)}", - sender=self, color="success")) - - if viewer.figure._upload_png_callback is not None: - raise ValueError("previous png export is still in progress. Wait to complete " - "before making another call to save_figure") - - viewer.figure.get_png_data(on_img_received) + # support writing without save dialog + # https://github.com/bqplot/bqplot/pull/1397 + if filename is None: + filename = self.filename_default + + def on_img_received(data): + try: + with filename.open(mode='bw') as f: + f.write(data) + except Exception as e: + self.hub.broadcast(SnackbarMessage( + f"{self.viewer.selected} failed to export to {str(filename)}: {e}", + sender=self, color="error")) + finally: + self.hub.broadcast(SnackbarMessage( + f"{self.viewer.selected} exported to {str(filename)}", + sender=self, color="success")) + + if viewer.figure._upload_png_callback is not None: + raise ValueError("previous png export is still in progress. Wait to complete " + "before making another call to save_figure") + + viewer.figure.get_png_data(on_img_received) elif filetype == "svg": viewer.figure.save_svg(str(filename) if filename is not None else None) @@ -721,11 +724,11 @@ def save_subset_as_region(self, selected_subset_label, filename): region = region[0][f'{"sky_" if link_type == "wcs" else ""}region'] - region.write(filename, overwrite=True) + region.write(str(filename), overwrite=True) def save_subset_as_table(self, filename): region = self.app.get_subsets(subset_name=self.subset.selected) - region.write(filename) + region.write(str(filename)) def vue_interrupt_recording(self, *args): # pragma: no cover self.movie_interrupt = True From 4d16d87c86d50752a6210927116bfe95d8ac37ac Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Wed, 12 Jun 2024 15:17:21 -0400 Subject: [PATCH 04/11] add some test coverage --- .../configs/default/plugins/export/export.py | 3 +- .../plugins/export/tests/test_export.py | 39 +++++++++++++++++++ 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/jdaviz/configs/default/plugins/export/export.py b/jdaviz/configs/default/plugins/export/export.py index 1e2f0330f1..f5d80084b3 100644 --- a/jdaviz/configs/default/plugins/export/export.py +++ b/jdaviz/configs/default/plugins/export/export.py @@ -556,8 +556,7 @@ def on_img_received(data): sender=self, color="success")) if viewer.figure._upload_png_callback is not None: - raise ValueError("previous png export is still in progress. Wait to complete " - "before making another call to save_figure") + raise ValueError("previous png export is still in progress. Wait to complete before making another call to save_figure") # noqa: E501 # pragma: no cover viewer.figure.get_png_data(on_img_received) diff --git a/jdaviz/configs/default/plugins/export/tests/test_export.py b/jdaviz/configs/default/plugins/export/tests/test_export.py index b340cb67e0..d8d41f117a 100644 --- a/jdaviz/configs/default/plugins/export/tests/test_export.py +++ b/jdaviz/configs/default/plugins/export/tests/test_export.py @@ -348,3 +348,42 @@ def test_ap_phot_plot_export(self, imviz_helper): # just check that it doesn't crash, since we can't download export_plugin.export() + + def test_figure_export(self, imviz_helper): + + data = NDData(np.ones((500, 500)) * u.nJy) + + imviz_helper.load_data(data) + + export_plugin = imviz_helper.plugins['Export']._obj + + export_plugin.export(filename=None) + + # attempting to save a figure back to back + try: + export_plugin.export(filename='img.png') + except ValueError as e: + assert str(e) == "previous png export is still in progress. Wait to complete before making another call to save_figure" # noqa: E501 + + def test_filepath_convention(self, imviz_helper): + + data = NDData(np.ones((500, 500)) * u.nJy) + + imviz_helper.load_data(data) + + export_plugin = imviz_helper.plugins['Export']._obj + + export_plugin.filename_value = '~/img.png' + expected_path = os.path.expanduser('~/img.png') + + assert export_plugin.default_filepath == expected_path + + export_plugin.filename_value = '../img.png' + expected_path = os.path.abspath('../img.png') + + assert export_plugin.default_filepath == expected_path + + export_plugin.filename_value = './img.png' + expected_path = os.path.abspath('./img.png') + + assert export_plugin.default_filepath == expected_path From 9fe1234debe249a7c2eda2a39134661132ce2c1c Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Wed, 12 Jun 2024 15:55:50 -0400 Subject: [PATCH 05/11] update test, reduce transparency of overwrite overlay as temporary fix --- jdaviz/configs/default/plugins/export/export.vue | 4 ++-- jdaviz/configs/default/plugins/export/tests/test_export.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/jdaviz/configs/default/plugins/export/export.vue b/jdaviz/configs/default/plugins/export/export.vue index 9a09340d04..40148856f6 100644 --- a/jdaviz/configs/default/plugins/export/export.vue +++ b/jdaviz/configs/default/plugins/export/export.vue @@ -273,9 +273,9 @@ diff --git a/jdaviz/configs/default/plugins/export/tests/test_export.py b/jdaviz/configs/default/plugins/export/tests/test_export.py index d8d41f117a..d5272dc599 100644 --- a/jdaviz/configs/default/plugins/export/tests/test_export.py +++ b/jdaviz/configs/default/plugins/export/tests/test_export.py @@ -374,7 +374,7 @@ def test_filepath_convention(self, imviz_helper): export_plugin = imviz_helper.plugins['Export']._obj export_plugin.filename_value = '~/img.png' - expected_path = os.path.expanduser('~/img.png') + expected_path = os.path.normpath(os.path.expanduser('~/img.png')) assert export_plugin.default_filepath == expected_path From c245d8d2c5782559d69c4f8f1264d3f1355ea876 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Fri, 14 Jun 2024 15:21:34 -0400 Subject: [PATCH 06/11] add / testing --- jdaviz/configs/default/plugins/export/export.py | 3 +-- jdaviz/configs/default/plugins/export/export.vue | 2 +- jdaviz/configs/default/plugins/export/tests/test_export.py | 5 ++++- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/jdaviz/configs/default/plugins/export/export.py b/jdaviz/configs/default/plugins/export/export.py index f5d80084b3..183971f437 100644 --- a/jdaviz/configs/default/plugins/export/export.py +++ b/jdaviz/configs/default/plugins/export/export.py @@ -537,8 +537,7 @@ def vue_overwrite_from_ui(self, *args, **kwargs): def save_figure(self, viewer, filename=None, filetype="png", show_dialog=False): if filetype == "png": - # support writing without save dialog - # https://github.com/bqplot/bqplot/pull/1397 + if filename is None: filename = self.filename_default diff --git a/jdaviz/configs/default/plugins/export/export.vue b/jdaviz/configs/default/plugins/export/export.vue index 40148856f6..381664cfdf 100644 --- a/jdaviz/configs/default/plugins/export/export.vue +++ b/jdaviz/configs/default/plugins/export/export.vue @@ -275,7 +275,7 @@ absolute opacity=0.5 :value="overwrite_warn" - :zIndex=0 + :zIndex=3 style="grid-area: 1/1; margin-left: -24px; margin-right: -24px"> diff --git a/jdaviz/configs/default/plugins/export/tests/test_export.py b/jdaviz/configs/default/plugins/export/tests/test_export.py index d5272dc599..388eef2838 100644 --- a/jdaviz/configs/default/plugins/export/tests/test_export.py +++ b/jdaviz/configs/default/plugins/export/tests/test_export.py @@ -359,7 +359,7 @@ def test_figure_export(self, imviz_helper): export_plugin.export(filename=None) - # attempting to save a figure back to back + # attempt to save a figure back to back try: export_plugin.export(filename='img.png') except ValueError as e: @@ -373,6 +373,9 @@ def test_filepath_convention(self, imviz_helper): export_plugin = imviz_helper.plugins['Export']._obj + export_plugin.filename_value = '/img.png' + assert export_plugin.default_filepath == export_plugin.filename_value + export_plugin.filename_value = '~/img.png' expected_path = os.path.normpath(os.path.expanduser('~/img.png')) From e5f299320ac9083d9f9f4b295bbd149d8f9c03bc Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Fri, 14 Jun 2024 15:54:05 -0400 Subject: [PATCH 07/11] address 3.10 test failures --- jdaviz/configs/default/plugins/export/tests/test_export.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jdaviz/configs/default/plugins/export/tests/test_export.py b/jdaviz/configs/default/plugins/export/tests/test_export.py index 388eef2838..49609ae41b 100644 --- a/jdaviz/configs/default/plugins/export/tests/test_export.py +++ b/jdaviz/configs/default/plugins/export/tests/test_export.py @@ -374,7 +374,7 @@ def test_filepath_convention(self, imviz_helper): export_plugin = imviz_helper.plugins['Export']._obj export_plugin.filename_value = '/img.png' - assert export_plugin.default_filepath == export_plugin.filename_value + assert os.path.normpath(export_plugin.default_filepath) == os.path.normpath(export_plugin.filename_value) # noqa: E501 export_plugin.filename_value = '~/img.png' expected_path = os.path.normpath(os.path.expanduser('~/img.png')) From b50dbcf1c1129ce585d8e8864f1a03871144b7cc Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Fri, 14 Jun 2024 16:11:35 -0400 Subject: [PATCH 08/11] address 3.10 test failures --- jdaviz/configs/default/plugins/export/tests/test_export.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jdaviz/configs/default/plugins/export/tests/test_export.py b/jdaviz/configs/default/plugins/export/tests/test_export.py index 49609ae41b..4e83d8b1b8 100644 --- a/jdaviz/configs/default/plugins/export/tests/test_export.py +++ b/jdaviz/configs/default/plugins/export/tests/test_export.py @@ -374,7 +374,7 @@ def test_filepath_convention(self, imviz_helper): export_plugin = imviz_helper.plugins['Export']._obj export_plugin.filename_value = '/img.png' - assert os.path.normpath(export_plugin.default_filepath) == os.path.normpath(export_plugin.filename_value) # noqa: E501 + assert os.path.abspath(export_plugin.default_filepath) == os.path.abspath(export_plugin.filename_value) # noqa: E501 export_plugin.filename_value = '~/img.png' expected_path = os.path.normpath(os.path.expanduser('~/img.png')) From 2461e3f1d4321896ddf1871036f4a9724361f24b Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Thu, 20 Jun 2024 16:01:28 -0400 Subject: [PATCH 09/11] ensure overwrite only occurs after export button click, standalone path shows cwd and not temp path --- jdaviz/configs/default/plugins/export/export.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/jdaviz/configs/default/plugins/export/export.py b/jdaviz/configs/default/plugins/export/export.py index 183971f437..5a805b3f9c 100644 --- a/jdaviz/configs/default/plugins/export/export.py +++ b/jdaviz/configs/default/plugins/export/export.py @@ -375,8 +375,11 @@ def _normalize_filename(self, filename=None, filetype=None, overwrite=False, def if not filepath.is_absolute(): abs_path = filepath.resolve() self.default_filepath = str(abs_path) + # set default path for standalone + if os.environ.get("JDAVIZ_START_DIR", ""): + self.default_filepath = os.environ["JDAVIZ_START_DIR"] - if filename.exists() and not overwrite: + if filename.exists() and not overwrite and not default_path: self.overwrite_warn = True else: self.overwrite_warn = False From b221862e3b0726bb843c70c4e279fd061a8d0d66 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Fri, 21 Jun 2024 16:25:58 -0400 Subject: [PATCH 10/11] update paths to use os/Path and tests to use OS independent methods for building paths --- .../configs/default/plugins/export/export.py | 13 +++-------- .../plugins/export/tests/test_export.py | 22 ++++++++----------- 2 files changed, 12 insertions(+), 23 deletions(-) diff --git a/jdaviz/configs/default/plugins/export/export.py b/jdaviz/configs/default/plugins/export/export.py index 5a805b3f9c..292c5af2b5 100644 --- a/jdaviz/configs/default/plugins/export/export.py +++ b/jdaviz/configs/default/plugins/export/export.py @@ -266,16 +266,9 @@ def _set_filename_default(self, *args, **kwargs): def _is_filename_changed(self, event): # Update the UI Filepath if relative or absolute paths are provided # by user via self.filename_value - if '~' in self.filename_value and '/' in self.filename_value: - expanded_path = os.path.expanduser(self.filename_value) - abs_path = Path(expanded_path).resolve() - self.default_filepath = str(abs_path) - elif '/' in self.filename_value and \ - ('.' in self.filename_value or '..' in self.filename_value): - abs_path = Path(self.filename_value).resolve() - self.default_filepath = str(abs_path) - elif '/' in self.filename_value: - self.default_filepath = str(self.filename_value) + expanded_path = os.path.expanduser(self.filename_value) + abs_path = Path(expanded_path).resolve() + self.default_filepath = str(abs_path) # Clear overwrite warning when user changes filename self.overwrite_warn = False diff --git a/jdaviz/configs/default/plugins/export/tests/test_export.py b/jdaviz/configs/default/plugins/export/tests/test_export.py index 4e83d8b1b8..701f07aca9 100644 --- a/jdaviz/configs/default/plugins/export/tests/test_export.py +++ b/jdaviz/configs/default/plugins/export/tests/test_export.py @@ -10,6 +10,7 @@ from glue.core.roi import CircularROI, XRangeROI from regions import Regions, CircleSkyRegion from specutils import Spectrum1D +from pathlib import Path @pytest.mark.usefixtures('_jail') @@ -366,27 +367,22 @@ def test_figure_export(self, imviz_helper): assert str(e) == "previous png export is still in progress. Wait to complete before making another call to save_figure" # noqa: E501 def test_filepath_convention(self, imviz_helper): - data = NDData(np.ones((500, 500)) * u.nJy) - imviz_helper.load_data(data) - export_plugin = imviz_helper.plugins['Export']._obj - export_plugin.filename_value = '/img.png' + # Set filename value using OS-independent Path methods + export_plugin.filename_value = str(Path('/') / 'img.png') assert os.path.abspath(export_plugin.default_filepath) == os.path.abspath(export_plugin.filename_value) # noqa: E501 - export_plugin.filename_value = '~/img.png' - expected_path = os.path.normpath(os.path.expanduser('~/img.png')) - + export_plugin.filename_value = str(Path('~') / 'img.png') + expected_path = str(Path('~').expanduser() / 'img.png') assert export_plugin.default_filepath == expected_path - export_plugin.filename_value = '../img.png' - expected_path = os.path.abspath('../img.png') - + export_plugin.filename_value = str(Path('..') / 'img.png') + expected_path = str((Path('..') / 'img.png').resolve()) assert export_plugin.default_filepath == expected_path - export_plugin.filename_value = './img.png' - expected_path = os.path.abspath('./img.png') - + export_plugin.filename_value = str(Path('.') / 'img.png') + expected_path = str((Path('.') / 'img.png').resolve()) assert export_plugin.default_filepath == expected_path From 79c885148a4a1ac461de7278e6247e8ce157cc95 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Fri, 5 Jul 2024 12:37:58 -0400 Subject: [PATCH 11/11] update change log milestone --- CHANGES.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index ae45138f42..82a92af199 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -16,9 +16,6 @@ New Features - The colormap menu for image layers now shows in-line previews of the colormaps. [#2900] -- Display default filepath in Export plugin, re-enable API exporting, enable relative and absolute - path exports from the UI. [#2896] - Cubeviz ^^^^^^^ @@ -190,6 +187,9 @@ Other Changes and Additions Bug Fixes --------- +- Display default filepath in Export plugin, re-enable API exporting, enable relative and absolute + path exports from the UI. [#2896] + Cubeviz ^^^^^^^