From 52d620417b9980a770c1df14f70e5faf77c383ba Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Tue, 15 Aug 2023 13:01:41 -0400 Subject: [PATCH 01/18] fix error message formatting --- jdaviz/configs/imviz/plugins/footprints/preset_regions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jdaviz/configs/imviz/plugins/footprints/preset_regions.py b/jdaviz/configs/imviz/plugins/footprints/preset_regions.py index 7fb5a25c5a..d6d6270e8b 100644 --- a/jdaviz/configs/imviz/plugins/footprints/preset_regions.py +++ b/jdaviz/configs/imviz/plugins/footprints/preset_regions.py @@ -93,7 +93,7 @@ def jwst_footprint(instrument, ra, dec, pa, v2_offset=0.0, v3_offset=0.0, apertu raise ImportError('jwst_footprint requires pysiaf to be installed') if instrument not in _instruments: - raise ValueError(f"instrument must be one of {[', '].join(_instruments.keys())}") + raise ValueError(f"instrument must be one of {', '.join(_instruments.keys())}") siaf_interface = pysiaf.Siaf(_instruments.get(instrument)) From bbd862e42ca10b57f6983e92cfbe793ff0c15ca1 Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Tue, 15 Aug 2023 13:02:05 -0400 Subject: [PATCH 02/18] WIP: generalize file import component and adopt in footprints plugin --- jdaviz/app.py | 1 + jdaviz/components/plugin_file_import.vue | 36 ++++++++++ .../imviz/plugins/catalogs/catalogs.py | 41 +++-------- .../imviz/plugins/catalogs/catalogs.vue | 71 +++++++------------ .../imviz/plugins/footprints/footprints.py | 38 ++++++++-- .../imviz/plugins/footprints/footprints.vue | 32 ++++++++- jdaviz/core/template_mixin.py | 64 +++++++++++++++++ 7 files changed, 199 insertions(+), 84 deletions(-) create mode 100644 jdaviz/components/plugin_file_import.vue diff --git a/jdaviz/app.py b/jdaviz/app.py index e4b5153eda..6795687a15 100644 --- a/jdaviz/app.py +++ b/jdaviz/app.py @@ -140,6 +140,7 @@ def to_unit(self, data, cid, values, original_units, target_units): 'plugin-editable-select': 'components/plugin_editable_select.vue', 'plugin-add-results': 'components/plugin_add_results.vue', 'plugin-auto-label': 'components/plugin_auto_label.vue', + 'plugin-file-import': 'components/plugin_file_import.vue', 'glue-state-sync-wrapper': 'components/glue_state_sync_wrapper.vue'} _verbosity_levels = ('debug', 'info', 'warning', 'error') diff --git a/jdaviz/components/plugin_file_import.vue b/jdaviz/components/plugin_file_import.vue new file mode 100644 index 0000000000..055c4fc906 --- /dev/null +++ b/jdaviz/components/plugin_file_import.vue @@ -0,0 +1,36 @@ + + + diff --git a/jdaviz/configs/imviz/plugins/catalogs/catalogs.py b/jdaviz/configs/imviz/plugins/catalogs/catalogs.py index c1f72431d8..7513a216ac 100644 --- a/jdaviz/configs/imviz/plugins/catalogs/catalogs.py +++ b/jdaviz/configs/imviz/plugins/catalogs/catalogs.py @@ -11,13 +11,13 @@ from jdaviz.core.events import SnackbarMessage from jdaviz.core.registries import tray_registry from jdaviz.core.template_mixin import (PluginTemplateMixin, ViewerSelectMixin, - SelectPluginComponent) + SelectPluginComponent, FileImportMixin) __all__ = ['Catalogs'] @tray_registry('imviz-catalogs', label="Catalog Search") -class Catalogs(PluginTemplateMixin, ViewerSelectMixin): +class Catalogs(PluginTemplateMixin, ViewerSelectMixin, FileImportMixin): """ See the :ref:`Catalog Search Plugin Documentation ` for more details. @@ -30,8 +30,6 @@ class Catalogs(PluginTemplateMixin, ViewerSelectMixin): template_file = __file__, "catalogs.vue" catalog_items = List([]).tag(sync=True) catalog_selected = Unicode("").tag(sync=True) - from_file = Unicode().tag(sync=True) - from_file_message = Unicode().tag(sync=True) results_available = Bool(False).tag(sync=True) number_of_results = Int(0).tag(sync=True) @@ -43,38 +41,22 @@ def __init__(self, *args, **kwargs): selected='catalog_selected', manual_options=['SDSS', 'From File...']) - # file chooser for From File - start_path = os.environ.get('JDAVIZ_START_DIR', os.path.curdir) - self._file_upload = FileChooser(start_path) - self.components = {'g-file-import': self._file_upload} - self._file_upload.observe(self._on_file_path_changed, names='file_path') - self._cached_table_from_file = {} self._marker_name = 'catalog_results' - def _on_file_path_changed(self, event): - self.from_file_message = 'Checking if file is valid' - path = event['new'] - if (path is not None - and not os.path.exists(path) - or not os.path.isfile(path)): - self.from_file_message = 'File path does not exist' - return + # set the custom file parser for importing catalogs + self.file_import._file_parser = self._file_parser + @staticmethod + def _file_parser(path): try: table = QTable.read(path) except Exception: - self.from_file_message = 'Could not parse file with astropy.table.QTable.read' - return + return 'Could not parse file with astropy.table.QTable.read', {} if 'sky_centroid' not in table.colnames: - self.from_file_message = 'Table does not contain required sky_centroid column' - return + return 'Table does not contain required sky_centroid column', {} - # since we loaded the file already to check if its valid, we might as well cache the table - # so we don't have to re-load it when clicking search. We'll only keep the latest entry - # though, but store in a dict so we can catch if the file path was changed from the API - self._cached_table_from_file = {path: table} - self.from_file_message = '' + return '', {path: table} @observe('from_file') def _from_file_changed(self, event): @@ -87,9 +69,6 @@ def _from_file_changed(self, event): # (so will change from 'From File...' to the first entry in the dropdown) self.catalog.select_default() - def vue_set_file_from_dialog(self, *args, **kwargs): - self.from_file = self._file_upload.file_path - def search(self): """ Search the catalog, display markers on the viewer, and return a table of results (or None @@ -165,7 +144,7 @@ def search(self): elif self.catalog_selected == 'From File...': # all exceptions when going through the UI should have prevented setting this path # but this exceptions might be raised here if setting from_file from the UI - table = self._cached_table_from_file.get(self.from_file, QTable.read(self.from_file)) + table = self.file_import.selected_obj self.app._catalog_source_table = table skycoord_table = table['sky_centroid'] diff --git a/jdaviz/configs/imviz/plugins/catalogs/catalogs.vue b/jdaviz/configs/imviz/plugins/catalogs/catalogs.vue index 3300ab9f72..ee57b7feae 100644 --- a/jdaviz/configs/imviz/plugins/catalogs/catalogs.vue +++ b/jdaviz/configs/imviz/plugins/catalogs/catalogs.vue @@ -1,19 +1,18 @@ + + - + diff --git a/jdaviz/configs/imviz/plugins/footprints/footprints.py b/jdaviz/configs/imviz/plugins/footprints/footprints.py index 2a9f289645..1b53f0e8a4 100644 --- a/jdaviz/configs/imviz/plugins/footprints/footprints.py +++ b/jdaviz/configs/imviz/plugins/footprints/footprints.py @@ -8,7 +8,8 @@ from jdaviz.core.marks import FootprintOverlay from jdaviz.core.registries import tray_registry from jdaviz.core.template_mixin import (PluginTemplateMixin, ViewerSelectMixin, - SelectPluginComponent, EditableSelectPluginComponent) + SelectPluginComponent, EditableSelectPluginComponent, + FileImportMixin) from jdaviz.core.user_api import PluginUserApi from jdaviz.configs.imviz.plugins.footprints import preset_regions @@ -18,7 +19,7 @@ @tray_registry('imviz-footprints', label="Footprints") -class Footprints(PluginTemplateMixin, ViewerSelectMixin): +class Footprints(PluginTemplateMixin, ViewerSelectMixin, FileImportMixin): """ See the :ref:`Footprints Plugin Documentation ` for more details. @@ -114,13 +115,13 @@ def __init__(self, *args, **kwargs): on_rename=self._on_overlay_rename, on_remove=self._on_overlay_remove) - # FUTURE IMPROVEMENT: could add 'From File...' entry here that works similar to that in - # the catalogs plugin, loads in a region file, and replaces any input UI elements with just - # a reference to the filename or some text self.preset = SelectPluginComponent(self, items='preset_items', selected='preset_selected', - manual_options=preset_regions._instruments.keys()) + manual_options=list(preset_regions._instruments.keys())+['From File...']) # noqa + + # set the custom file parser for importing catalogs + self.file_import._file_parser = self._file_parser # disable if pixel-linked AND only a single item in the data collection self.hub.subscribe(self, LinkUpdatedMessage, handler=self._on_link_type_updated) @@ -153,6 +154,13 @@ def marks(self): if hasattr(viewer, 'figure')} for overlay in self.overlay.choices} + @staticmethod + def _file_parser(path): + if path.endswith('.png'): + return '', {path: None} + + return 'Could not parse region from file', {} + def _on_link_type_updated(self, msg=None): self.is_pixel_linked = (getattr(self.app, '_link_type', None) == 'pixels' and len(self.app.data_collection) > 1) @@ -322,6 +330,19 @@ def _change_overlay(self, *args): fp[key] = getattr(self, attr) self._ignore_traitlet_change = False + @observe('from_file') + def _from_file_changed(self, event): + # NOTE: duplicated logic from catalogs.... move into the component if possible + import os + if len(event['new']): + if not os.path.exists(event['new']): + raise ValueError(f"{event['new']} does not exist") + self.preset.selected = 'From File...' + else: + # NOTE: select_default will change the value even if the current value is valid + # (so will change from 'From File...' to the first entry in the dropdown) + self.preset.select_default() + def _mark_visible(self, viewer_id, overlay=None): if not self.is_active: return False @@ -374,7 +395,10 @@ def overlay_regions(self): callable_kwargs = {k: getattr(self, k) for k in ('ra', 'dec', 'pa', 'v2_offset', 'v3_offset')} - regs = preset_regions.jwst_footprint(self.preset_selected, **callable_kwargs) + if self.preset_selected in preset_regions._instruments: + regs = preset_regions.jwst_footprint(self.preset_selected, **callable_kwargs) + else: + regs = [] return regs @observe('preset_selected', 'ra', 'dec', 'pa', 'v2_offset', 'v3_offset') diff --git a/jdaviz/configs/imviz/plugins/footprints/footprints.vue b/jdaviz/configs/imviz/plugins/footprints/footprints.vue index 8d9b648325..5266dd244b 100644 --- a/jdaviz/configs/imviz/plugins/footprints/footprints.vue +++ b/jdaviz/configs/imviz/plugins/footprints/footprints.vue @@ -85,11 +85,36 @@ :items="preset_items.map(i => i.label)" v-model="preset_selected" label="Preset" - hint="Select the preset instrument footprint." + hint="Preset instrument or import from a file." persistent-hint > + + + + {{from_file.split("/").slice(-1)[0]}} + + + + + + Center RA/Dec @@ -183,4 +208,9 @@ padding-left: 16px; border: 2px solid rgba(0,0,0,0.54); } + + .v-chip__content { + width: 100% + } + diff --git a/jdaviz/core/template_mixin.py b/jdaviz/core/template_mixin.py index cb42a69590..64cdff6587 100644 --- a/jdaviz/core/template_mixin.py +++ b/jdaviz/core/template_mixin.py @@ -5,6 +5,7 @@ import bqplot from contextlib import contextmanager import numpy as np +import os import threading import time @@ -45,6 +46,7 @@ 'ViewerSelect', 'ViewerSelectMixin', 'LayerSelect', 'LayerSelectMixin', 'DatasetSelect', 'DatasetSelectMixin', + 'FileImportMixin', 'Table', 'TableMixin', 'Plot', 'PlotMixin', 'AutoTextField', 'AutoTextFieldMixin', @@ -2326,6 +2328,68 @@ def __init__(self, *args, **kwargs): 'add_to_viewer_items', 'add_to_viewer_selected') +class FileImport(BasePluginComponent): + """ + NOT CURRENTLY ABLE TO USE MULTIPLE INSTANCES - USE FileImportMixin INSTEAD + """ + def __init__(self, plugin, from_file, from_file_message, *args, **kwargs): + from jdaviz.configs.default.plugins.data_tools.file_chooser import FileChooser + + start_path = os.environ.get('JDAVIZ_START_DIR', os.path.curdir) + self._file_chooser = FileChooser(start_path) + + plugin.components = {'g-file-import': self._file_chooser} + + self._file_chooser.observe(self._on_file_path_changed, names='file_path') + + super().__init__(plugin, from_file=from_file, from_file_message=from_file_message) + + def _default_file_parser(path): + return '', {} + + self._file_parser = kwargs.pop('file_parser', _default_file_parser) + + @property + def selected_obj(self): + return self._cached_file.get(self.from_file, self._file_parser(self.from_file)[1]) + + def _on_file_path_changed(self, event): + self.from_file_message = 'Checking if file is valid' + path = event['new'] + if (path is not None + and not os.path.exists(path) + or not os.path.isfile(path)): + self.from_file_message = 'File path does not exist' + return + + self.from_file_message, self._cached_file = self._file_parser(path) + + +class FileImportMixin(VuetifyTemplate, HubListener): + """ + + + + + """ + from_file = Unicode().tag(sync=True) + from_file_message = Unicode().tag(sync=True) + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.file_import = FileImport(self, 'from_file', 'from_file_message') + + def vue_file_import_accept(self, *args, **kwargs): + self.from_file = self.file_import._file_chooser.file_path + + class PlotOptionsSyncState(BasePluginComponent): """ Plugin component for syncing with glue state objects. From b8e79cdb50be1a82adfa2d4b4fd401d67ce5690c Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Tue, 15 Aug 2023 14:12:38 -0400 Subject: [PATCH 03/18] put implementation in select component --- jdaviz/app.py | 2 +- jdaviz/components/plugin_file_import.vue | 36 ---- .../components/plugin_file_import_select.vue | 71 ++++++++ .../imviz/plugins/catalogs/catalogs.py | 36 ++-- .../imviz/plugins/catalogs/catalogs.vue | 56 ++----- .../imviz/plugins/footprints/footprints.py | 29 +--- .../imviz/plugins/footprints/footprints.vue | 55 ++----- jdaviz/core/template_mixin.py | 154 +++++++++++------- 8 files changed, 210 insertions(+), 229 deletions(-) delete mode 100644 jdaviz/components/plugin_file_import.vue create mode 100644 jdaviz/components/plugin_file_import_select.vue diff --git a/jdaviz/app.py b/jdaviz/app.py index 6795687a15..b7318cd8e8 100644 --- a/jdaviz/app.py +++ b/jdaviz/app.py @@ -140,7 +140,7 @@ def to_unit(self, data, cid, values, original_units, target_units): 'plugin-editable-select': 'components/plugin_editable_select.vue', 'plugin-add-results': 'components/plugin_add_results.vue', 'plugin-auto-label': 'components/plugin_auto_label.vue', - 'plugin-file-import': 'components/plugin_file_import.vue', + 'plugin-file-import-select': 'components/plugin_file_import_select.vue', 'glue-state-sync-wrapper': 'components/glue_state_sync_wrapper.vue'} _verbosity_levels = ('debug', 'info', 'warning', 'error') diff --git a/jdaviz/components/plugin_file_import.vue b/jdaviz/components/plugin_file_import.vue deleted file mode 100644 index 055c4fc906..0000000000 --- a/jdaviz/components/plugin_file_import.vue +++ /dev/null @@ -1,36 +0,0 @@ - - - diff --git a/jdaviz/components/plugin_file_import_select.vue b/jdaviz/components/plugin_file_import_select.vue new file mode 100644 index 0000000000..db82a3bb10 --- /dev/null +++ b/jdaviz/components/plugin_file_import_select.vue @@ -0,0 +1,71 @@ + + + + + diff --git a/jdaviz/configs/imviz/plugins/catalogs/catalogs.py b/jdaviz/configs/imviz/plugins/catalogs/catalogs.py index 7513a216ac..7f18d423bd 100644 --- a/jdaviz/configs/imviz/plugins/catalogs/catalogs.py +++ b/jdaviz/configs/imviz/plugins/catalogs/catalogs.py @@ -1,23 +1,20 @@ -import os - import numpy as np import numpy.ma as ma from astropy import units as u from astropy.table import QTable from astropy.coordinates import SkyCoord -from traitlets import List, Unicode, Bool, Int, observe +from traitlets import List, Unicode, Bool, Int -from jdaviz.configs.default.plugins.data_tools.file_chooser import FileChooser from jdaviz.core.events import SnackbarMessage from jdaviz.core.registries import tray_registry from jdaviz.core.template_mixin import (PluginTemplateMixin, ViewerSelectMixin, - SelectPluginComponent, FileImportMixin) + FileImportSelectPluginComponent, HasFileImportSelect) __all__ = ['Catalogs'] @tray_registry('imviz-catalogs', label="Catalog Search") -class Catalogs(PluginTemplateMixin, ViewerSelectMixin, FileImportMixin): +class Catalogs(PluginTemplateMixin, ViewerSelectMixin, HasFileImportSelect): """ See the :ref:`Catalog Search Plugin Documentation ` for more details. @@ -36,15 +33,15 @@ class Catalogs(PluginTemplateMixin, ViewerSelectMixin, FileImportMixin): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - self.catalog = SelectPluginComponent(self, - items='catalog_items', - selected='catalog_selected', - manual_options=['SDSS', 'From File...']) - - self._marker_name = 'catalog_results' + self.catalog = FileImportSelectPluginComponent(self, + items='catalog_items', + selected='catalog_selected', + manual_options=['SDSS', 'From File...']) # set the custom file parser for importing catalogs - self.file_import._file_parser = self._file_parser + self.catalog._file_parser = self._file_parser + + self._marker_name = 'catalog_results' @staticmethod def _file_parser(path): @@ -58,17 +55,6 @@ def _file_parser(path): return '', {path: table} - @observe('from_file') - def _from_file_changed(self, event): - if len(event['new']): - if not os.path.exists(event['new']): - raise ValueError(f"{event['new']} does not exist") - self.catalog.selected = 'From File...' - else: - # NOTE: select_default will change the value even if the current value is valid - # (so will change from 'From File...' to the first entry in the dropdown) - self.catalog.select_default() - def search(self): """ Search the catalog, display markers on the viewer, and return a table of results (or None @@ -144,7 +130,7 @@ def search(self): elif self.catalog_selected == 'From File...': # all exceptions when going through the UI should have prevented setting this path # but this exceptions might be raised here if setting from_file from the UI - table = self.file_import.selected_obj + table = self.catalog.selected_obj self.app._catalog_source_table = table skycoord_table = table['sky_centroid'] diff --git a/jdaviz/configs/imviz/plugins/catalogs/catalogs.vue b/jdaviz/configs/imviz/plugins/catalogs/catalogs.vue index ee57b7feae..ff747dd39a 100644 --- a/jdaviz/configs/imviz/plugins/catalogs/catalogs.vue +++ b/jdaviz/configs/imviz/plugins/catalogs/catalogs.vue @@ -12,42 +12,20 @@ hint="Select a viewer to search." /> - - - - - - {{from_file.split("/").slice(-1)[0]}} - - - - - - - + + + @@ -65,9 +43,3 @@ - - diff --git a/jdaviz/configs/imviz/plugins/footprints/footprints.py b/jdaviz/configs/imviz/plugins/footprints/footprints.py index 1b53f0e8a4..e1dff9fa5e 100644 --- a/jdaviz/configs/imviz/plugins/footprints/footprints.py +++ b/jdaviz/configs/imviz/plugins/footprints/footprints.py @@ -8,8 +8,8 @@ from jdaviz.core.marks import FootprintOverlay from jdaviz.core.registries import tray_registry from jdaviz.core.template_mixin import (PluginTemplateMixin, ViewerSelectMixin, - SelectPluginComponent, EditableSelectPluginComponent, - FileImportMixin) + EditableSelectPluginComponent, + FileImportSelectPluginComponent, HasFileImportSelect) from jdaviz.core.user_api import PluginUserApi from jdaviz.configs.imviz.plugins.footprints import preset_regions @@ -19,7 +19,7 @@ @tray_registry('imviz-footprints', label="Footprints") -class Footprints(PluginTemplateMixin, ViewerSelectMixin, FileImportMixin): +class Footprints(PluginTemplateMixin, ViewerSelectMixin, HasFileImportSelect): """ See the :ref:`Footprints Plugin Documentation ` for more details. @@ -115,13 +115,13 @@ def __init__(self, *args, **kwargs): on_rename=self._on_overlay_rename, on_remove=self._on_overlay_remove) - self.preset = SelectPluginComponent(self, - items='preset_items', - selected='preset_selected', - manual_options=list(preset_regions._instruments.keys())+['From File...']) # noqa + self.preset = FileImportSelectPluginComponent(self, + items='preset_items', + selected='preset_selected', + manual_options=list(preset_regions._instruments.keys())+['From File...']) # noqa # set the custom file parser for importing catalogs - self.file_import._file_parser = self._file_parser + self.preset._file_parser = self._file_parser # disable if pixel-linked AND only a single item in the data collection self.hub.subscribe(self, LinkUpdatedMessage, handler=self._on_link_type_updated) @@ -330,19 +330,6 @@ def _change_overlay(self, *args): fp[key] = getattr(self, attr) self._ignore_traitlet_change = False - @observe('from_file') - def _from_file_changed(self, event): - # NOTE: duplicated logic from catalogs.... move into the component if possible - import os - if len(event['new']): - if not os.path.exists(event['new']): - raise ValueError(f"{event['new']} does not exist") - self.preset.selected = 'From File...' - else: - # NOTE: select_default will change the value even if the current value is valid - # (so will change from 'From File...' to the first entry in the dropdown) - self.preset.select_default() - def _mark_visible(self, viewer_id, overlay=None): if not self.is_active: return False diff --git a/jdaviz/configs/imviz/plugins/footprints/footprints.vue b/jdaviz/configs/imviz/plugins/footprints/footprints.vue index 5266dd244b..8be4630caf 100644 --- a/jdaviz/configs/imviz/plugins/footprints/footprints.vue +++ b/jdaviz/configs/imviz/plugins/footprints/footprints.vue @@ -78,42 +78,20 @@ Footprint Definition - - - - - - {{from_file.split("/").slice(-1)[0]}} - - - - - - - + + + Center RA/Dec @@ -208,9 +186,4 @@ padding-left: 16px; border: 2px solid rgba(0,0,0,0.54); } - - .v-chip__content { - width: 100% - } - diff --git a/jdaviz/core/template_mixin.py b/jdaviz/core/template_mixin.py index 64cdff6587..50fcb2dbbc 100644 --- a/jdaviz/core/template_mixin.py +++ b/jdaviz/core/template_mixin.py @@ -46,7 +46,7 @@ 'ViewerSelect', 'ViewerSelectMixin', 'LayerSelect', 'LayerSelectMixin', 'DatasetSelect', 'DatasetSelectMixin', - 'FileImportMixin', + 'FileImportSelectPluginComponent', 'HasFileImportSelect', 'Table', 'TableMixin', 'Plot', 'PlotMixin', 'AutoTextField', 'AutoTextFieldMixin', @@ -788,6 +788,96 @@ def _selected_changed(self, event): self.selected = self.labels[ind] +class FileImportSelectPluginComponent(SelectPluginComponent): + """ + IMPORTANT: Always accompany with HasFileImportSelect + IMPORTANT: currently assumed only one instance per-plugin + """ + def __init__(self, plugin, **kwargs): + """ + """ + if "From File..." not in kwargs['manual_options']: + kwargs['manual_options'] += ['From File...'] + + if not isinstance(plugin, HasFileImportSelect): + raise NotImplementedError("plugin must inherit from HasFileImportSelect") + + super().__init__(plugin, + from_file='from_file', from_file_message='from_file_message', + **kwargs) + + self.plugin._file_chooser.observe(self._on_file_path_changed, names='file_path') + # reference back here so the plugin can reset to default + self.plugin._file_chooser._select_component = self + + def _default_file_parser(path): + return '', {} + + self._file_parser = kwargs.pop('file_parser', _default_file_parser) + self.add_observe('from_file', self._from_file_changed) + + @property + def selected_obj(self): + if self.selected == 'From File...': + return self._cached_file.get(self.from_file, self._file_parser(self.from_file)[1]) + return super().selected_obj + + def _from_file_changed(self, event): + if len(event['new']): + if not os.path.exists(event['new']): + raise ValueError(f"{event['new']} does not exist") + self.selected = 'From File...' + else: + # NOTE: select_default will change the value even if the current value is valid + # (so will change from 'From File...' to the first entry in the dropdown) + self.select_default() + + def _on_file_path_changed(self, event): + self.from_file_message = 'Checking if file is valid' + path = event['new'] + if (path is not None + and not os.path.exists(path) + or not os.path.isfile(path)): + self.from_file_message = 'File path does not exist' + return + + self.from_file_message, self._cached_file = self._file_parser(path) + + +class HasFileImportSelect(VuetifyTemplate, HubListener): + """ + + + + + """ + from_file = Unicode().tag(sync=True) + from_file_message = Unicode().tag(sync=True) + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + + from jdaviz.configs.default.plugins.data_tools.file_chooser import FileChooser + + start_path = os.environ.get('JDAVIZ_START_DIR', os.path.curdir) + self._file_chooser = FileChooser(start_path) + self.components = {'g-file-import': self._file_chooser} + + def vue_file_import_accept(self, *args, **kwargs): + self.from_file = self._file_chooser.file_path + + def vue_file_import_cancel(self, *args, **kwargs): + self._file_chooser._select_component.select_default() + self.from_file = '' + + class EditableSelectPluginComponent(SelectPluginComponent): """ Plugin select with support for renaming, adding, and deleting items (by the user). @@ -2328,68 +2418,6 @@ def __init__(self, *args, **kwargs): 'add_to_viewer_items', 'add_to_viewer_selected') -class FileImport(BasePluginComponent): - """ - NOT CURRENTLY ABLE TO USE MULTIPLE INSTANCES - USE FileImportMixin INSTEAD - """ - def __init__(self, plugin, from_file, from_file_message, *args, **kwargs): - from jdaviz.configs.default.plugins.data_tools.file_chooser import FileChooser - - start_path = os.environ.get('JDAVIZ_START_DIR', os.path.curdir) - self._file_chooser = FileChooser(start_path) - - plugin.components = {'g-file-import': self._file_chooser} - - self._file_chooser.observe(self._on_file_path_changed, names='file_path') - - super().__init__(plugin, from_file=from_file, from_file_message=from_file_message) - - def _default_file_parser(path): - return '', {} - - self._file_parser = kwargs.pop('file_parser', _default_file_parser) - - @property - def selected_obj(self): - return self._cached_file.get(self.from_file, self._file_parser(self.from_file)[1]) - - def _on_file_path_changed(self, event): - self.from_file_message = 'Checking if file is valid' - path = event['new'] - if (path is not None - and not os.path.exists(path) - or not os.path.isfile(path)): - self.from_file_message = 'File path does not exist' - return - - self.from_file_message, self._cached_file = self._file_parser(path) - - -class FileImportMixin(VuetifyTemplate, HubListener): - """ - - - - - """ - from_file = Unicode().tag(sync=True) - from_file_message = Unicode().tag(sync=True) - - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - self.file_import = FileImport(self, 'from_file', 'from_file_message') - - def vue_file_import_accept(self, *args, **kwargs): - self.from_file = self.file_import._file_chooser.file_path - - class PlotOptionsSyncState(BasePluginComponent): """ Plugin component for syncing with glue state objects. From 60472193d10adec1d1bc35be0e217c72b6922746 Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Wed, 16 Aug 2023 12:46:52 -0400 Subject: [PATCH 04/18] when clicking cancel, revert to previous selection (not always default) --- jdaviz/core/template_mixin.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/jdaviz/core/template_mixin.py b/jdaviz/core/template_mixin.py index 50fcb2dbbc..7f9a6563d0 100644 --- a/jdaviz/core/template_mixin.py +++ b/jdaviz/core/template_mixin.py @@ -521,6 +521,7 @@ def __init__(self, *args, **kwargs): filters = kwargs.pop('filters', [])[:] # [:] needed to force copy from kwarg default super().__init__(*args, **kwargs) + self._selected_previous = None self._cached_properties = ["selected_obj", "selected_item"] self._default_mode = default_mode @@ -611,6 +612,15 @@ def select_next(self): self.selected = cycle[(curr_ind + 1) % len(cycle)] return self.selected + def select_previous(self): + """ + Apply and return the previous selection (or default option if no previous selection) + """ + if self._selected_previous is None: + return self.select_default() + self.selected = self._selected_previous + return self.selected + @property def default_text(self): return self._default_text @@ -732,6 +742,7 @@ def _multiselect_changed(self, event): self._apply_default_selection() def _selected_changed(self, event): + self._selected_previous = event['old'] self._clear_cache() if self.is_multiselect: if not isinstance(event['new'], list): @@ -874,7 +885,7 @@ def vue_file_import_accept(self, *args, **kwargs): self.from_file = self._file_chooser.file_path def vue_file_import_cancel(self, *args, **kwargs): - self._file_chooser._select_component.select_default() + self._file_chooser._select_component.select_previous() self.from_file = '' From 1078bb309a5edd68a7d5d4f1c6f30c3795d0a67c Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Wed, 16 Aug 2023 13:01:32 -0400 Subject: [PATCH 05/18] improve public API for selecting file --- jdaviz/core/template_mixin.py | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/jdaviz/core/template_mixin.py b/jdaviz/core/template_mixin.py index 7f9a6563d0..02bf37ff5b 100644 --- a/jdaviz/core/template_mixin.py +++ b/jdaviz/core/template_mixin.py @@ -835,13 +835,24 @@ def selected_obj(self): def _from_file_changed(self, event): if len(event['new']): - if not os.path.exists(event['new']): - raise ValueError(f"{event['new']} does not exist") + if event['new'] != self.plugin._file_chooser.file_path: + # then need to run the parser or check for valid path + if not os.path.exists(event['new']): + if self.selected == 'From File...': + self.select_previous() + raise ValueError(f"{event['new']} is not a valid file path") + + # run through the parsers and check the validity + self._on_file_path_changed(event) + if self.from_file_message: + if self.selected == 'From File...': + self.select_previous() + raise ValueError(self.from_file_message) + self.selected = 'From File...' - else: - # NOTE: select_default will change the value even if the current value is valid - # (so will change from 'From File...' to the first entry in the dropdown) - self.select_default() + + elif self.selected == 'From File...': + self.select_previous() def _on_file_path_changed(self, event): self.from_file_message = 'Checking if file is valid' @@ -854,6 +865,12 @@ def _on_file_path_changed(self, event): self.from_file_message, self._cached_file = self._file_parser(path) + def import_file(self, path): + """ + Select 'From File...' and set the path + """ + self.from_file = path + class HasFileImportSelect(VuetifyTemplate, HubListener): """ From c8f9bb604845396f7978e5798b183956447052a1 Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Wed, 16 Aug 2023 13:22:29 -0400 Subject: [PATCH 06/18] support loading region from file in footprints plugin * currently without any repositioning support (region is static) --- .../imviz/plugins/footprints/footprints.py | 25 +++- .../imviz/plugins/footprints/footprints.vue | 130 +++++++++--------- jdaviz/core/template_mixin.py | 2 + 3 files changed, 86 insertions(+), 71 deletions(-) diff --git a/jdaviz/configs/imviz/plugins/footprints/footprints.py b/jdaviz/configs/imviz/plugins/footprints/footprints.py index e1dff9fa5e..f0b2c61b26 100644 --- a/jdaviz/configs/imviz/plugins/footprints/footprints.py +++ b/jdaviz/configs/imviz/plugins/footprints/footprints.py @@ -156,10 +156,12 @@ def marks(self): @staticmethod def _file_parser(path): - if path.endswith('.png'): - return '', {path: None} - - return 'Could not parse region from file', {} + try: + region = regions.Regions.read(path) + except Exception: + return 'Could not parse region from file', {} + else: + return '', {path: region} def _on_link_type_updated(self, msg=None): self.is_pixel_linked = (getattr(self.app, '_link_type', None) == 'pixels' and @@ -305,6 +307,11 @@ def _change_overlay(self, *args): # create new entry with defaults (any defaults not provided here will be carried over # from the previous selection based on current traitlet values) self._overlays[self.overlay_selected] = {'color': '#c75109'} + if self.preset_selected == 'From File...': + # don't carry over the imported file/region to the next selection + # TODO: but the new one isn't being generated immediately... + self._overlays[self.overlay_selected]['from_file'] = '' + self._overlays[self.overlay_selected]['preset'] = self.preset.choices[0] if len(self._overlays) == 1 and len(self.viewer.selected): # default to the center of the current zoom limits of the first selected viewer self.center_on_viewer(self.viewer.selected[0]) @@ -315,7 +322,8 @@ def _change_overlay(self, *args): # traitlets simultaneously (and since we're only updating traitlets to a previously-set # overlay, we shouldn't have to update anything with the marks themselves) self._ignore_traitlet_change = True - for attr in ('preset_selected', 'visible', 'color', 'fill_opacity', 'viewer_selected', + for attr in ('from_file', 'preset_selected', + 'visible', 'color', 'fill_opacity', 'viewer_selected', 'ra', 'dec', 'pa', 'v2_offset', 'v3_offset'): key = attr.split('_selected')[0] @@ -329,6 +337,7 @@ def _change_overlay(self, *args): # dict above. fp[key] = getattr(self, attr) self._ignore_traitlet_change = False + self._preset_args_changed() def _mark_visible(self, viewer_id, overlay=None): if not self.is_active: @@ -382,13 +391,15 @@ def overlay_regions(self): callable_kwargs = {k: getattr(self, k) for k in ('ra', 'dec', 'pa', 'v2_offset', 'v3_offset')} - if self.preset_selected in preset_regions._instruments: + if self.preset_selected == 'From File...': + regs = self.preset.selected_obj + elif self.preset_selected in preset_regions._instruments: regs = preset_regions.jwst_footprint(self.preset_selected, **callable_kwargs) else: regs = [] return regs - @observe('preset_selected', 'ra', 'dec', 'pa', 'v2_offset', 'v3_offset') + @observe('preset_selected', 'from_file', 'ra', 'dec', 'pa', 'v2_offset', 'v3_offset') def _preset_args_changed(self, msg={}): if self._ignore_traitlet_change: return diff --git a/jdaviz/configs/imviz/plugins/footprints/footprints.vue b/jdaviz/configs/imviz/plugins/footprints/footprints.vue index 8be4630caf..4af72d2407 100644 --- a/jdaviz/configs/imviz/plugins/footprints/footprints.vue +++ b/jdaviz/configs/imviz/plugins/footprints/footprints.vue @@ -93,74 +93,76 @@ - - Center RA/Dec - - - {{viewer_ref}} - - - +
+ + Center RA/Dec + + + {{viewer_ref}} + + + - - - + + + - - - + + + - - - + + + - - - - - - - + + + + + + + +
diff --git a/jdaviz/core/template_mixin.py b/jdaviz/core/template_mixin.py index 02bf37ff5b..e43b1812df 100644 --- a/jdaviz/core/template_mixin.py +++ b/jdaviz/core/template_mixin.py @@ -807,6 +807,8 @@ class FileImportSelectPluginComponent(SelectPluginComponent): def __init__(self, plugin, **kwargs): """ """ + self._cached_file = {} + if "From File..." not in kwargs['manual_options']: kwargs['manual_options'] += ['From File...'] From 035d7eaa79dd2e938489cf6f69083995ab760c3d Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Wed, 16 Aug 2023 13:30:10 -0400 Subject: [PATCH 07/18] update changelog entry --- CHANGES.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index f58b0a2ab7..fde0d357a3 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -22,7 +22,8 @@ Imviz - vmin/vmax step size in the plot options plugin is now dynamic based on the full range of the image. [#2388] -- Footprints plugin for plotting overlays of instrument footprints in the image viewer. [#2341] +- Footprints plugin for plotting overlays of instrument footprints or custom regions in the image + viewer. [#2341, #2377] Mosviz ^^^^^^ From 683b318d1d7c91f2aca203e4eb40bc2c87493650 Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Wed, 16 Aug 2023 14:07:38 -0400 Subject: [PATCH 08/18] support importing from API --- .../imviz/plugins/footprints/footprints.py | 23 ++++++++++++++++++- jdaviz/core/template_mixin.py | 21 ++++++++++++++--- 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/jdaviz/configs/imviz/plugins/footprints/footprints.py b/jdaviz/configs/imviz/plugins/footprints/footprints.py index f0b2c61b26..ee27106196 100644 --- a/jdaviz/configs/imviz/plugins/footprints/footprints.py +++ b/jdaviz/configs/imviz/plugins/footprints/footprints.py @@ -46,6 +46,7 @@ class Footprints(PluginTemplateMixin, ViewerSelectMixin, HasFileImportSelect): opacity of the filled region of the currently selected overlay * ``preset`` (:class:`~jdaviz.core.template_mixin.SelectPluginComponent`): selected overlay preset + * :meth:``import_region`` * :meth:``center_on_viewer`` * ``ra`` central right ascension for the footprint overlay @@ -135,7 +136,8 @@ def user_api(self): return PluginUserApi(self, expose=('overlay', 'rename_overlay', 'add_overlay', 'remove_overlay', 'viewer', 'visible', 'color', 'fill_opacity', - 'preset', 'center_on_viewer', 'ra', 'dec', 'pa', + 'preset', 'import_region', + 'center_on_viewer', 'ra', 'dec', 'pa', 'v2_offset', 'v3_offset', 'overlay_regions')) return PluginUserApi(self) @@ -156,6 +158,10 @@ def marks(self): @staticmethod def _file_parser(path): + if isinstance(path, regions.Regions): + from_file_string = f'API: {path.__class__.__name__} object' + return '', {from_file_string: path} + try: region = regions.Regions.read(path) except Exception: @@ -382,6 +388,21 @@ def _display_args_changed(self, msg={}): mark.colors = [self.color] mark.fill_opacities = [self.fill_opacity] + def import_region(self, region): + """ + Import an astropy regions object (or file). + + Parameters + ---------- + region : str or regions.Regions object + """ + if isinstance(region, regions.Regions): + self.preset.import_obj(region) + elif isinstance(region, str): + self.preset.import_file(region) + else: + raise ValueError("region must be a regions.Regions object or file path") + @property def overlay_regions(self): """ diff --git a/jdaviz/core/template_mixin.py b/jdaviz/core/template_mixin.py index e43b1812df..110e413a6a 100644 --- a/jdaviz/core/template_mixin.py +++ b/jdaviz/core/template_mixin.py @@ -807,7 +807,7 @@ class FileImportSelectPluginComponent(SelectPluginComponent): def __init__(self, plugin, **kwargs): """ """ - self._cached_file = {} + self._cached_obj = {} if "From File..." not in kwargs['manual_options']: kwargs['manual_options'] += ['From File...'] @@ -832,10 +832,13 @@ def _default_file_parser(path): @property def selected_obj(self): if self.selected == 'From File...': - return self._cached_file.get(self.from_file, self._file_parser(self.from_file)[1]) + return self._cached_obj.get(self.from_file, self._file_parser(self.from_file)[1]) return super().selected_obj def _from_file_changed(self, event): + if event['new'].startswith('API:'): + # object imported from the API: parsing is already handled + return if len(event['new']): if event['new'] != self.plugin._file_chooser.file_path: # then need to run the parser or check for valid path @@ -865,14 +868,26 @@ def _on_file_path_changed(self, event): self.from_file_message = 'File path does not exist' return - self.from_file_message, self._cached_file = self._file_parser(path) + self.from_file_message, self._cached_obj = self._file_parser(path) def import_file(self, path): """ Select 'From File...' and set the path """ + # NOTE: this will trigger self._from_file_changed which in turn will + # pass through the parser, raise an error if necessary, and set + # self.selected accordingly self.from_file = path + def import_obj(self, obj): + """ + """ + msg, self._cached_obj = self._file_parser(obj) + if msg: + raise ValueError(msg) + self.selected = 'From File...' + self.from_file = list(self._cached_obj.keys())[0] + class HasFileImportSelect(VuetifyTemplate, HubListener): """ From 4fa9bf01a006e3b7213d70d17d44a0ac13397063 Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Thu, 17 Aug 2023 09:18:38 -0400 Subject: [PATCH 09/18] update tests/docs --- docs/imviz/plugins.rst | 5 ++- .../imviz/plugins/catalogs/catalogs.py | 15 +++++++++ .../imviz/plugins/footprints/footprints.py | 4 +-- .../plugins/footprints/preset_regions.py | 2 +- jdaviz/configs/imviz/tests/test_catalogs.py | 14 ++++----- jdaviz/configs/imviz/tests/test_footprints.py | 26 ++++++++++++++-- jdaviz/core/template_mixin.py | 31 ++++++++++--------- 7 files changed, 68 insertions(+), 29 deletions(-) diff --git a/docs/imviz/plugins.rst b/docs/imviz/plugins.rst index a0c7de9e2a..4e910e7a14 100644 --- a/docs/imviz/plugins.rst +++ b/docs/imviz/plugins.rst @@ -317,7 +317,7 @@ Footprints This plugin supports loading and overplotting instrument footprint overlays on the image viewers. Any number of overlays can be plotted simultaneously from any number of the available -preset instruments. +preset instruments or by loading an astropy regions object from a file. The top dropdown allows renaming, adding, and removing footprint overlays. To modify the display and input parameters for a given overlay, select it in the dropdown, and modify the choices @@ -325,6 +325,9 @@ in the plugin to change its color, opacity, visibilities in any image viewer in select between various preset instruments and change the input options (position on the sky, position angle, offsets, etc). +To import a file, choose "From File..." from the presets dropdown and select a valid file (must +be able to be parsed by `regions.Regions.read`). + .. _rotate-canvas: diff --git a/jdaviz/configs/imviz/plugins/catalogs/catalogs.py b/jdaviz/configs/imviz/plugins/catalogs/catalogs.py index 7f18d423bd..8c6b619c17 100644 --- a/jdaviz/configs/imviz/plugins/catalogs/catalogs.py +++ b/jdaviz/configs/imviz/plugins/catalogs/catalogs.py @@ -173,6 +173,21 @@ def search(self): return skycoord_table + def import_catalog(self, catalog): + """ + Import a catalog from a file path. + + Parameters + ---------- + catalog : str + Path to a file that can be parsed by astropy QTable + """ + # TODO: self.catalog.import_obj for a QTable directly (see footprints implementation) + if isinstance(catalog, str): + self.catalog.import_file(catalog) + else: # pragma: no cover + raise ValueError("catalog must be a string (file path)") + def vue_do_search(self, *args, **kwargs): # calls self.search() which handles all of the searching logic self.search() diff --git a/jdaviz/configs/imviz/plugins/footprints/footprints.py b/jdaviz/configs/imviz/plugins/footprints/footprints.py index ee27106196..d9c2c11896 100644 --- a/jdaviz/configs/imviz/plugins/footprints/footprints.py +++ b/jdaviz/configs/imviz/plugins/footprints/footprints.py @@ -398,10 +398,10 @@ def import_region(self, region): """ if isinstance(region, regions.Regions): self.preset.import_obj(region) - elif isinstance(region, str): + elif isinstance(region, str): # TODO: support path objects? self.preset.import_file(region) else: - raise ValueError("region must be a regions.Regions object or file path") + raise ValueError("region must be a regions.Regions object or string (file path)") @property def overlay_regions(self): diff --git a/jdaviz/configs/imviz/plugins/footprints/preset_regions.py b/jdaviz/configs/imviz/plugins/footprints/preset_regions.py index d6d6270e8b..60f87bc9a7 100644 --- a/jdaviz/configs/imviz/plugins/footprints/preset_regions.py +++ b/jdaviz/configs/imviz/plugins/footprints/preset_regions.py @@ -92,7 +92,7 @@ def jwst_footprint(instrument, ra, dec, pa, v2_offset=0.0, v3_offset=0.0, apertu if not _has_pysiaf: raise ImportError('jwst_footprint requires pysiaf to be installed') - if instrument not in _instruments: + if instrument not in _instruments: # pragma: no cover raise ValueError(f"instrument must be one of {', '.join(_instruments.keys())}") siaf_interface = pysiaf.Siaf(_instruments.get(instrument)) diff --git a/jdaviz/configs/imviz/tests/test_catalogs.py b/jdaviz/configs/imviz/tests/test_catalogs.py index 3927ac90ea..7465ea0d0e 100644 --- a/jdaviz/configs/imviz/tests/test_catalogs.py +++ b/jdaviz/configs/imviz/tests/test_catalogs.py @@ -130,13 +130,13 @@ def test_from_file_parsing(imviz_helper, tmp_path): catalogs_plugin = imviz_helper.app.get_tray_item_from_name('imviz-catalogs') # _on_file_path_changed is fired when changing the selection in the file dialog - catalogs_plugin._on_file_path_changed({'new': './invalid_path'}) + catalogs_plugin.catalog._on_file_path_changed({'new': './invalid_path'}) assert catalogs_plugin.from_file_message == 'File path does not exist' - # observe('from_file') is fired when setting from_file from the API (or after clicking - # select in the file dialog) - with pytest.raises(ValueError, match='./invalid_path does not exist'): - catalogs_plugin.from_file = './invalid_path' + # observe('from_file') is fired when setting from_file from the API or via import_file + # (or after clicking select in the file dialog) + with pytest.raises(ValueError, match='./invalid_path is not a valid file path'): + catalogs_plugin.import_catalog('./invalid_path') # setting to a blank string from the API resets the catalog selection to the # default/first entry @@ -145,13 +145,13 @@ def test_from_file_parsing(imviz_helper, tmp_path): not_table_file = tmp_path / 'not_table.tst' not_table_file.touch() - catalogs_plugin._on_file_path_changed({'new': not_table_file}) + catalogs_plugin.catalog._on_file_path_changed({'new': not_table_file}) assert catalogs_plugin.from_file_message == 'Could not parse file with astropy.table.QTable.read' # noqa qtable = QTable({'not_sky_centroid': [1, 2, 3]}) not_valid_table = tmp_path / 'not_valid_table.ecsv' qtable.write(not_valid_table, overwrite=True) - catalogs_plugin._on_file_path_changed({'new': not_valid_table}) + catalogs_plugin.catalog._on_file_path_changed({'new': not_valid_table}) assert catalogs_plugin.from_file_message == 'Table does not contain required sky_centroid column' # noqa diff --git a/jdaviz/configs/imviz/tests/test_footprints.py b/jdaviz/configs/imviz/tests/test_footprints.py index 20947d6e04..439a907761 100644 --- a/jdaviz/configs/imviz/tests/test_footprints.py +++ b/jdaviz/configs/imviz/tests/test_footprints.py @@ -12,7 +12,7 @@ def _get_markers_from_viewer(viewer): return [m for m in viewer.figure.marks if isinstance(m, FootprintOverlay)] -def test_user_api(imviz_helper, image_2d_wcs): +def test_user_api(imviz_helper, image_2d_wcs, tmp_path): arr = np.ones((10, 10)) ndd = NDData(arr, wcs=image_2d_wcs) # load the image twice to test linking @@ -26,7 +26,7 @@ def test_user_api(imviz_helper, image_2d_wcs): assert plugin._obj.is_pixel_linked is False # test that each of the supported instruments/presets work - for preset in plugin.preset.choices: + for preset in (preset for preset in plugin.preset.choices if preset != 'From File...'): plugin.preset = preset viewer_marks = _get_markers_from_viewer(imviz_helper.default_viewer) @@ -78,9 +78,29 @@ def test_user_api(imviz_helper, image_2d_wcs): plugin.viewer.select_all() assert viewer_marks[0].visible is True - # test centering logic + # test centering logic (for now just that it doesn't fail) plugin.center_on_viewer() + # test from file/API ability + reg = plugin.overlay_regions + plugin.import_region(reg) + assert plugin.preset.selected == 'From File...' + assert len(viewer_marks) == len(reg) + # clearing the file should default to the PREVIOUS preset (last from the for-loop above) + plugin._obj.vue_file_import_cancel() + assert plugin.preset.selected == preset + + tmp_file = str(tmp_path / 'test_region.reg') + reg.write(tmp_file, format='ds9') + plugin.import_region(tmp_file) + assert plugin.preset.selected == 'From File...' + assert plugin.preset.from_file == tmp_file + + with pytest.raises(ValueError): + plugin.import_region('./invalid_path.reg') + assert plugin.preset.selected == preset + # with the plugin no longer active, marks should not be visible assert plugin._obj.is_active is False + viewer_marks = _get_markers_from_viewer(imviz_helper.default_viewer) assert viewer_marks[0].visible is False diff --git a/jdaviz/core/template_mixin.py b/jdaviz/core/template_mixin.py index 110e413a6a..e7e3edd13e 100644 --- a/jdaviz/core/template_mixin.py +++ b/jdaviz/core/template_mixin.py @@ -803,6 +803,19 @@ class FileImportSelectPluginComponent(SelectPluginComponent): """ IMPORTANT: Always accompany with HasFileImportSelect IMPORTANT: currently assumed only one instance per-plugin + + Example template (label and hint are optional):: + + + + """ def __init__(self, plugin, **kwargs): """ @@ -812,7 +825,7 @@ def __init__(self, plugin, **kwargs): if "From File..." not in kwargs['manual_options']: kwargs['manual_options'] += ['From File...'] - if not isinstance(plugin, HasFileImportSelect): + if not isinstance(plugin, HasFileImportSelect): # pragma: no cover raise NotImplementedError("plugin must inherit from HasFileImportSelect") super().__init__(plugin, @@ -824,7 +837,8 @@ def __init__(self, plugin, **kwargs): self.plugin._file_chooser._select_component = self def _default_file_parser(path): - return '', {} + # by default, just return the file path itself (and allow all files) + return '', {path: path} self._file_parser = kwargs.pop('file_parser', _default_file_parser) self.add_observe('from_file', self._from_file_changed) @@ -890,19 +904,6 @@ def import_obj(self, obj): class HasFileImportSelect(VuetifyTemplate, HubListener): - """ - - - - - """ from_file = Unicode().tag(sync=True) from_file_message = Unicode().tag(sync=True) From b73db109946b45e7bd2b26673d3a989ae62db656 Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Fri, 18 Aug 2023 10:25:05 -0400 Subject: [PATCH 10/18] fix support for multiple file/objects across multiple viewers --- jdaviz/configs/imviz/plugins/footprints/footprints.py | 7 ++++++- jdaviz/core/template_mixin.py | 5 +++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/jdaviz/configs/imviz/plugins/footprints/footprints.py b/jdaviz/configs/imviz/plugins/footprints/footprints.py index d9c2c11896..37f86e3ce6 100644 --- a/jdaviz/configs/imviz/plugins/footprints/footprints.py +++ b/jdaviz/configs/imviz/plugins/footprints/footprints.py @@ -413,7 +413,12 @@ def overlay_regions(self): for k in ('ra', 'dec', 'pa', 'v2_offset', 'v3_offset')} if self.preset_selected == 'From File...': - regs = self.preset.selected_obj + # we need to cache these locally in order to support multiple files/regions between + # different overlay entries all selecting From File... + overlay = self._overlays.get(self.overlay_selected, {}) + if 'regions' not in overlay and isinstance(self.preset.selected_obj, regions.Regions): + overlay['regions'] = self.preset.selected_obj + regs = overlay.get('regions') elif self.preset_selected in preset_regions._instruments: regs = preset_regions.jwst_footprint(self.preset_selected, **callable_kwargs) else: diff --git a/jdaviz/core/template_mixin.py b/jdaviz/core/template_mixin.py index e7e3edd13e..071b501060 100644 --- a/jdaviz/core/template_mixin.py +++ b/jdaviz/core/template_mixin.py @@ -886,7 +886,7 @@ def _on_file_path_changed(self, event): def import_file(self, path): """ - Select 'From File...' and set the path + Select 'From File...' and set the path. """ # NOTE: this will trigger self._from_file_changed which in turn will # pass through the parser, raise an error if necessary, and set @@ -895,12 +895,13 @@ def import_file(self, path): def import_obj(self, obj): """ + Import a supported object directly from the API. """ msg, self._cached_obj = self._file_parser(obj) if msg: raise ValueError(msg) - self.selected = 'From File...' self.from_file = list(self._cached_obj.keys())[0] + self.selected = 'From File...' class HasFileImportSelect(VuetifyTemplate, HubListener): From 8e37c83720a7d711f11c266bf0e4771dd7129483 Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Fri, 18 Aug 2023 11:11:40 -0400 Subject: [PATCH 11/18] ensure region(s) are sky, not pixel --- .../imviz/plugins/footprints/footprints.py | 22 +++++++++++++------ jdaviz/configs/imviz/tests/test_footprints.py | 18 +++++++++++++++ 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/jdaviz/configs/imviz/plugins/footprints/footprints.py b/jdaviz/configs/imviz/plugins/footprints/footprints.py index 37f86e3ce6..eff7efab30 100644 --- a/jdaviz/configs/imviz/plugins/footprints/footprints.py +++ b/jdaviz/configs/imviz/plugins/footprints/footprints.py @@ -1,5 +1,6 @@ from traitlets import Bool, List, Unicode, observe import regions +import numpy as np from glue.core.message import DataCollectionAddMessage, DataCollectionDeleteMessage @@ -158,7 +159,14 @@ def marks(self): @staticmethod def _file_parser(path): - if isinstance(path, regions.Regions): + def _ensure_sky(region): + if isinstance(region, regions.Regions): + return np.all([_ensure_sky(reg) for reg in region.regions]) + return hasattr(region, 'to_pixel') + + if isinstance(path, (regions.Region, regions.Regions)): + if not _ensure_sky(path): + return 'Region is not a SkyRegion', {} from_file_string = f'API: {path.__class__.__name__} object' return '', {from_file_string: path} @@ -166,8 +174,9 @@ def _file_parser(path): region = regions.Regions.read(path) except Exception: return 'Could not parse region from file', {} - else: - return '', {path: region} + if not _ensure_sky(region): + return 'Region is not a SkyRegion', {} + return '', {path: region} def _on_link_type_updated(self, msg=None): self.is_pixel_linked = (getattr(self.app, '_link_type', None) == 'pixels' and @@ -315,7 +324,6 @@ def _change_overlay(self, *args): self._overlays[self.overlay_selected] = {'color': '#c75109'} if self.preset_selected == 'From File...': # don't carry over the imported file/region to the next selection - # TODO: but the new one isn't being generated immediately... self._overlays[self.overlay_selected]['from_file'] = '' self._overlays[self.overlay_selected]['preset'] = self.preset.choices[0] if len(self._overlays) == 1 and len(self.viewer.selected): @@ -396,12 +404,12 @@ def import_region(self, region): ---------- region : str or regions.Regions object """ - if isinstance(region, regions.Regions): + if isinstance(region, (regions.Region, regions.Regions)): self.preset.import_obj(region) elif isinstance(region, str): # TODO: support path objects? self.preset.import_file(region) else: - raise ValueError("region must be a regions.Regions object or string (file path)") + raise TypeError("region must be a regions.Regions object or string (file path)") @property def overlay_regions(self): @@ -421,7 +429,7 @@ def overlay_regions(self): regs = overlay.get('regions') elif self.preset_selected in preset_regions._instruments: regs = preset_regions.jwst_footprint(self.preset_selected, **callable_kwargs) - else: + else: # pragma: no cover regs = [] return regs diff --git a/jdaviz/configs/imviz/tests/test_footprints.py b/jdaviz/configs/imviz/tests/test_footprints.py index 439a907761..78e428c802 100644 --- a/jdaviz/configs/imviz/tests/test_footprints.py +++ b/jdaviz/configs/imviz/tests/test_footprints.py @@ -1,6 +1,9 @@ import numpy as np import pytest +import astropy.units as u from astropy.nddata import NDData +from astropy.coordinates import SkyCoord +from regions import PixCoord, CirclePixelRegion, CircleSkyRegion from jdaviz.core.marks import FootprintOverlay from jdaviz.configs.imviz.plugins.footprints.preset_regions import _all_apertures @@ -96,8 +99,23 @@ def test_user_api(imviz_helper, image_2d_wcs, tmp_path): assert plugin.preset.selected == 'From File...' assert plugin.preset.from_file == tmp_file + # test single region (footprints contain multiple regions) + valid_region_sky = CircleSkyRegion(center=SkyCoord(42, 43, unit='deg', frame='fk5'), + radius=3 * u.deg) + plugin.import_region(valid_region_sky) + with pytest.raises(ValueError): plugin.import_region('./invalid_path.reg') + with pytest.raises(TypeError): + plugin.import_region(5) + + invalid_region = CirclePixelRegion(PixCoord(x=8, y=7), radius=3.5) + with pytest.raises(ValueError): + plugin.import_region(invalid_region) + tmp_invalid_file = str(tmp_path / 'test_invalid_region.reg') + invalid_region.write(tmp_invalid_file, format='ds9') + with pytest.raises(ValueError): + plugin.import_region(tmp_invalid_file) assert plugin.preset.selected == preset # with the plugin no longer active, marks should not be visible From 9cc94515871d240dc3a4953508dd4a8e8d7621f9 Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Mon, 21 Aug 2023 15:43:41 -0400 Subject: [PATCH 12/18] fix traceback for parsing regions when file dialog open --- jdaviz/configs/imviz/plugins/footprints/footprints.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jdaviz/configs/imviz/plugins/footprints/footprints.py b/jdaviz/configs/imviz/plugins/footprints/footprints.py index eff7efab30..baaa221894 100644 --- a/jdaviz/configs/imviz/plugins/footprints/footprints.py +++ b/jdaviz/configs/imviz/plugins/footprints/footprints.py @@ -426,7 +426,7 @@ def overlay_regions(self): overlay = self._overlays.get(self.overlay_selected, {}) if 'regions' not in overlay and isinstance(self.preset.selected_obj, regions.Regions): overlay['regions'] = self.preset.selected_obj - regs = overlay.get('regions') + regs = overlay.get('regions', []) elif self.preset_selected in preset_regions._instruments: regs = preset_regions.jwst_footprint(self.preset_selected, **callable_kwargs) else: # pragma: no cover From 1b4d251d84fa0e90705d35420dd2014f276c39fb Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Tue, 22 Aug 2023 08:05:17 -0400 Subject: [PATCH 13/18] set min-width of plugin tray to 250px * already set to have a min-width of 25% the app-width, but for small screens/browsers, this will impose a pixel minimum to prevent UI over-crowding (and in this case, allow us to assume the hint in the "preset" dropdown will remain on a single line). --- jdaviz/app.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jdaviz/app.vue b/jdaviz/app.vue index 6b5a10ad0f..73b6ddc75d 100644 --- a/jdaviz/app.vue +++ b/jdaviz/app.vue @@ -102,7 +102,7 @@ - + Date: Thu, 24 Aug 2023 15:27:32 -0400 Subject: [PATCH 14/18] support for non-polygon region objects Co-authored-by: P. L. Lim <2090236+pllim@users.noreply.github.com> --- .../imviz/plugins/footprints/footprints.py | 36 +++++++++++++------ .../plugins/footprints/preset_regions.py | 2 +- jdaviz/configs/imviz/tests/test_footprints.py | 7 +++- jdaviz/core/marks.py | 2 +- 4 files changed, 34 insertions(+), 13 deletions(-) diff --git a/jdaviz/configs/imviz/plugins/footprints/footprints.py b/jdaviz/configs/imviz/plugins/footprints/footprints.py index baaa221894..ced466cb8a 100644 --- a/jdaviz/configs/imviz/plugins/footprints/footprints.py +++ b/jdaviz/configs/imviz/plugins/footprints/footprints.py @@ -7,6 +7,7 @@ from jdaviz.core.custom_traitlets import FloatHandleEmpty from jdaviz.core.events import LinkUpdatedMessage from jdaviz.core.marks import FootprintOverlay +from jdaviz.core.region_translators import regions2roi from jdaviz.core.registries import tray_registry from jdaviz.core.template_mixin import (PluginTemplateMixin, ViewerSelectMixin, EditableSelectPluginComponent, @@ -460,7 +461,6 @@ def _preset_args_changed(self, msg={}): if wcs is None: continue existing_overlays = self._get_marks(viewer, self.overlay_selected) - regs = [r for r in regs if isinstance(r, regions.PolygonSkyRegion)] update_existing = len(existing_overlays) == len(regs) if not update_existing and len(existing_overlays): # clear any existing marks (length has changed, perhaps a new preset) @@ -471,21 +471,38 @@ def _preset_args_changed(self, msg={}): # https://github.com/spacetelescope/jwst_novt/blob/main/jwst_novt/interact/display.py new_marks = [] for i, reg in enumerate(regs): + if not isinstance(reg, regions.Region) and not hasattr(reg, 'to_pixel'): # pragma: nocover + # NOTE: this is pre-checked for API/file selection in the file-parser + # and built-in presets should be designed to never hit this error + # in the future we may support pixel regions as well, but need to decide how + # to properly handle those scenarios for both WCS and pixel-linking + raise NotImplementedError("regions must all be SkyRegions") + pixel_region = reg.to_pixel(wcs) - if not isinstance(reg, regions.PolygonSkyRegion): # pragma: nocover - # if we ever want to support plotting centers as well, - # see jwst_novt/interact/display.py - continue - x_coords = pixel_region.vertices.x - y_coords = pixel_region.vertices.y + if isinstance(pixel_region, regions.PolygonPixelRegion): + x_coords = pixel_region.vertices.x + y_coords = pixel_region.vertices.y + + # bqplot marker does not respect image pixel sizes, so need to render as polygon. + elif isinstance(pixel_region, regions.RectanglePixelRegion): + pixel_region = pixel_region.to_polygon() + x_coords = pixel_region.vertices.x + y_coords = pixel_region.vertices.y + elif isinstance(pixel_region, (regions.CirclePixelRegion, + regions.EllipsePixelRegion, + regions.CircleAnnulusPixelRegion)): + roi = regions2roi(pixel_region) + x_coords, y_coords = roi.to_polygon() + else: # pragma: no cover + raise NotImplementedError("could not parse coordinates from regions - please report this issue") # noqa + if update_existing: mark = existing_overlays[i] with mark.hold_sync(): mark.x = x_coords mark.y = y_coords else: - # instrument aperture regions mark = FootprintOverlay( viewer, self.overlay_selected, @@ -493,8 +510,7 @@ def _preset_args_changed(self, msg={}): y=y_coords, colors=[self.color], fill_opacities=[self.fill_opacity], - visible=visible - ) + visible=visible) new_marks.append(mark) if not update_existing and len(new_marks): diff --git a/jdaviz/configs/imviz/plugins/footprints/preset_regions.py b/jdaviz/configs/imviz/plugins/footprints/preset_regions.py index 60f87bc9a7..e885abd395 100644 --- a/jdaviz/configs/imviz/plugins/footprints/preset_regions.py +++ b/jdaviz/configs/imviz/plugins/footprints/preset_regions.py @@ -52,7 +52,7 @@ "NRCB1_FULL", "NRCB2_FULL", "NRCB3_FULL", - "NRCB4_FULL",], + "NRCB4_FULL"], 'NIRCam:long': ["NRCA5_FULL", "NRCB5_FULL"], 'NIRISS': ['NIS_AMIFULL'], 'MIRI': ['MIRIM_FULL'], diff --git a/jdaviz/configs/imviz/tests/test_footprints.py b/jdaviz/configs/imviz/tests/test_footprints.py index 78e428c802..a313bce293 100644 --- a/jdaviz/configs/imviz/tests/test_footprints.py +++ b/jdaviz/configs/imviz/tests/test_footprints.py @@ -3,7 +3,7 @@ import astropy.units as u from astropy.nddata import NDData from astropy.coordinates import SkyCoord -from regions import PixCoord, CirclePixelRegion, CircleSkyRegion +from regions import PixCoord, CirclePixelRegion, CircleSkyRegion, RectangleSkyRegion from jdaviz.core.marks import FootprintOverlay from jdaviz.configs.imviz.plugins.footprints.preset_regions import _all_apertures @@ -104,6 +104,11 @@ def test_user_api(imviz_helper, image_2d_wcs, tmp_path): radius=3 * u.deg) plugin.import_region(valid_region_sky) + # test RectangleSkyRegion -> RectanglePixelRegion + valid_region_sky = RectangleSkyRegion(center=SkyCoord(42, 43, unit='deg', frame='fk5'), + height=3 * u.deg, width=2 * u.deg) + plugin.import_region(valid_region_sky) + with pytest.raises(ValueError): plugin.import_region('./invalid_path.reg') with pytest.raises(TypeError): diff --git a/jdaviz/core/marks.py b/jdaviz/core/marks.py index f5a4bd4b6d..5ec1bc866b 100644 --- a/jdaviz/core/marks.py +++ b/jdaviz/core/marks.py @@ -59,7 +59,7 @@ def _update_counts(self, *args): self.right.text = [f'{oob_right} \u25b6' if oob_right > 0 else ''] -class PluginMark(): +class PluginMark: def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) self.xunit = None From bfffc4a91d93ab08d81582bf67c5a3130e489893 Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Thu, 24 Aug 2023 15:44:11 -0400 Subject: [PATCH 15/18] reset internal cache when API/file input is changed --- jdaviz/configs/imviz/plugins/footprints/footprints.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/jdaviz/configs/imviz/plugins/footprints/footprints.py b/jdaviz/configs/imviz/plugins/footprints/footprints.py index ced466cb8a..d7a80ed459 100644 --- a/jdaviz/configs/imviz/plugins/footprints/footprints.py +++ b/jdaviz/configs/imviz/plugins/footprints/footprints.py @@ -411,6 +411,9 @@ def import_region(self, region): self.preset.import_file(region) else: raise TypeError("region must be a regions.Regions object or string (file path)") + # _preset_args_changed was probably already triggered by from_file traitlet changing, but + # that may have been before the file was fully parsed and available from preset.selected_obj + self._preset_args_changed() @property def overlay_regions(self): @@ -449,6 +452,10 @@ def _preset_args_changed(self, msg={}): if len(name): self._overlays[self.overlay_selected][name] = msg.get('new') + if name == 'from_file' and 'regions' in self._overlays[self.overlay_selected]: + # then the file may have been changed from the API, so we need to clear the cache + # the cache will then be re-populated on the call to self.overlay_regions below + del self._overlays[self.overlay_selected]['regions'] regs = self.overlay_regions @@ -471,7 +478,8 @@ def _preset_args_changed(self, msg={}): # https://github.com/spacetelescope/jwst_novt/blob/main/jwst_novt/interact/display.py new_marks = [] for i, reg in enumerate(regs): - if not isinstance(reg, regions.Region) and not hasattr(reg, 'to_pixel'): # pragma: nocover + if (not isinstance(reg, regions.Region) + and not hasattr(reg, 'to_pixel')): # pragma: nocover # NOTE: this is pre-checked for API/file selection in the file-parser # and built-in presets should be designed to never hit this error # in the future we may support pixel regions as well, but need to decide how From 89a5113e1843ca5c92851912f714cd94001581fb Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Fri, 25 Aug 2023 08:44:24 -0400 Subject: [PATCH 16/18] allow file/object import without pysiaf * instead of disabling the plugin, options are removed from the preset dropdown and a warning is shown. * to avoid "From File..." from being the default, a "None" entry is added if pysiaf is not installed * this may largely be reverted in the future Co-authored-by: P. L. Lim <2090236+pllim@users.noreply.github.com> --- docs/imviz/plugins.rst | 3 +- .../imviz/plugins/footprints/footprints.py | 32 +++++++++---------- .../imviz/plugins/footprints/footprints.vue | 6 +++- 3 files changed, 22 insertions(+), 19 deletions(-) diff --git a/docs/imviz/plugins.rst b/docs/imviz/plugins.rst index 4e910e7a14..6abb46b1cd 100644 --- a/docs/imviz/plugins.rst +++ b/docs/imviz/plugins.rst @@ -317,7 +317,8 @@ Footprints This plugin supports loading and overplotting instrument footprint overlays on the image viewers. Any number of overlays can be plotted simultaneously from any number of the available -preset instruments or by loading an astropy regions object from a file. +preset instruments (requires pysiaf to be installed) or by loading an astropy regions object from +a file. The top dropdown allows renaming, adding, and removing footprint overlays. To modify the display and input parameters for a given overlay, select it in the dropdown, and modify the choices diff --git a/jdaviz/configs/imviz/plugins/footprints/footprints.py b/jdaviz/configs/imviz/plugins/footprints/footprints.py index d7a80ed459..327e28303d 100644 --- a/jdaviz/configs/imviz/plugins/footprints/footprints.py +++ b/jdaviz/configs/imviz/plugins/footprints/footprints.py @@ -83,6 +83,7 @@ class Footprints(PluginTemplateMixin, ViewerSelectMixin, HasFileImportSelect): fill_opacity = FloatHandleEmpty(0.1).tag(sync=True) # PRESET OVERLAYS AND OPTIONS + has_pysiaf = Bool(preset_regions._has_pysiaf).tag(sync=True) preset_items = List().tag(sync=True) preset_selected = Unicode().tag(sync=True) @@ -94,12 +95,6 @@ class Footprints(PluginTemplateMixin, ViewerSelectMixin, HasFileImportSelect): # TODO: dithering/mosaic options? def __init__(self, *args, **kwargs): - if not preset_regions._has_pysiaf: # pragma: nocover - # NOTE: if we want to keep this as a soft-dependency and implement other - # footprint/region options later, we could just disable the JWST presets - # instead of the entire plugin - self.disabled_msg = 'this plugin requires pysiaf to be installed' - self._ignore_traitlet_change = False self._overlays = {} @@ -118,10 +113,15 @@ def __init__(self, *args, **kwargs): on_rename=self._on_overlay_rename, on_remove=self._on_overlay_remove) + if self.has_pysiaf: + preset_options = list(preset_regions._instruments.keys()) + else: + preset_options = ['None'] + preset_options.append('From File...') self.preset = FileImportSelectPluginComponent(self, items='preset_items', selected='preset_selected', - manual_options=list(preset_regions._instruments.keys())+['From File...']) # noqa + manual_options=preset_options) # set the custom file parser for importing catalogs self.preset._file_parser = self._file_parser @@ -134,15 +134,13 @@ def __init__(self, *args, **kwargs): @property def user_api(self): - if preset_regions._has_pysiaf: - return PluginUserApi(self, expose=('overlay', - 'rename_overlay', 'add_overlay', 'remove_overlay', - 'viewer', 'visible', 'color', 'fill_opacity', - 'preset', 'import_region', - 'center_on_viewer', 'ra', 'dec', 'pa', - 'v2_offset', 'v3_offset', - 'overlay_regions')) - return PluginUserApi(self) + return PluginUserApi(self, expose=('overlay', + 'rename_overlay', 'add_overlay', 'remove_overlay', + 'viewer', 'visible', 'color', 'fill_opacity', + 'preset', 'import_region', + 'center_on_viewer', 'ra', 'dec', 'pa', + 'v2_offset', 'v3_offset', + 'overlay_regions')) def _get_marks(self, viewer, overlay=None): matches = [mark for mark in viewer.figure.marks @@ -431,7 +429,7 @@ def overlay_regions(self): if 'regions' not in overlay and isinstance(self.preset.selected_obj, regions.Regions): overlay['regions'] = self.preset.selected_obj regs = overlay.get('regions', []) - elif self.preset_selected in preset_regions._instruments: + elif self.has_pysiaf and self.preset_selected in preset_regions._instruments: regs = preset_regions.jwst_footprint(self.preset_selected, **callable_kwargs) else: # pragma: no cover regs = [] diff --git a/jdaviz/configs/imviz/plugins/footprints/footprints.vue b/jdaviz/configs/imviz/plugins/footprints/footprints.vue index 4af72d2407..2807630cd8 100644 --- a/jdaviz/configs/imviz/plugins/footprints/footprints.vue +++ b/jdaviz/configs/imviz/plugins/footprints/footprints.vue @@ -78,6 +78,10 @@ Footprint Definition + + To use JWST footprints, install pysiaf and restart jdaviz + + -
+
Center RA/Dec From eafe011375357c3a8705d1925e52dfff360cd6d3 Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Fri, 25 Aug 2023 14:02:46 -0400 Subject: [PATCH 17/18] Apply suggestions from code review Co-authored-by: P. L. Lim <2090236+pllim@users.noreply.github.com> --- docs/imviz/plugins.rst | 2 +- jdaviz/configs/imviz/plugins/footprints/footprints.py | 6 +++--- jdaviz/core/template_mixin.py | 3 +-- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/docs/imviz/plugins.rst b/docs/imviz/plugins.rst index 6abb46b1cd..4d5d1b249d 100644 --- a/docs/imviz/plugins.rst +++ b/docs/imviz/plugins.rst @@ -317,7 +317,7 @@ Footprints This plugin supports loading and overplotting instrument footprint overlays on the image viewers. Any number of overlays can be plotted simultaneously from any number of the available -preset instruments (requires pysiaf to be installed) or by loading an astropy regions object from +preset instruments (requires pysiaf to be installed) or by loading an Astropy regions object from a file. The top dropdown allows renaming, adding, and removing footprint overlays. To modify the display diff --git a/jdaviz/configs/imviz/plugins/footprints/footprints.py b/jdaviz/configs/imviz/plugins/footprints/footprints.py index 327e28303d..24acd85663 100644 --- a/jdaviz/configs/imviz/plugins/footprints/footprints.py +++ b/jdaviz/configs/imviz/plugins/footprints/footprints.py @@ -1,6 +1,6 @@ from traitlets import Bool, List, Unicode, observe -import regions import numpy as np +import regions from glue.core.message import DataCollectionAddMessage, DataCollectionDeleteMessage @@ -397,7 +397,7 @@ def _display_args_changed(self, msg={}): def import_region(self, region): """ - Import an astropy regions object (or file). + Import an Astropy regions object (or file). Parameters ---------- @@ -477,7 +477,7 @@ def _preset_args_changed(self, msg={}): new_marks = [] for i, reg in enumerate(regs): if (not isinstance(reg, regions.Region) - and not hasattr(reg, 'to_pixel')): # pragma: nocover + or not hasattr(reg, 'to_pixel')): # pragma: no cover # NOTE: this is pre-checked for API/file selection in the file-parser # and built-in presets should be designed to never hit this error # in the future we may support pixel regions as well, but need to decide how diff --git a/jdaviz/core/template_mixin.py b/jdaviz/core/template_mixin.py index 071b501060..47a462d613 100644 --- a/jdaviz/core/template_mixin.py +++ b/jdaviz/core/template_mixin.py @@ -818,8 +818,6 @@ class FileImportSelectPluginComponent(SelectPluginComponent): """ def __init__(self, plugin, **kwargs): - """ - """ self._cached_obj = {} if "From File..." not in kwargs['manual_options']: @@ -911,6 +909,7 @@ class HasFileImportSelect(VuetifyTemplate, HubListener): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) + # imported here to avoid circular import from jdaviz.configs.default.plugins.data_tools.file_chooser import FileChooser start_path = os.environ.get('JDAVIZ_START_DIR', os.path.curdir) From 4eb05d7ad20f5cc84545dd61fbcccc12a966c0ab Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Mon, 28 Aug 2023 08:28:23 -0400 Subject: [PATCH 18/18] fix importing region before ever opening plugin --- .../configs/imviz/plugins/footprints/footprints.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/jdaviz/configs/imviz/plugins/footprints/footprints.py b/jdaviz/configs/imviz/plugins/footprints/footprints.py index 24acd85663..29bf25236d 100644 --- a/jdaviz/configs/imviz/plugins/footprints/footprints.py +++ b/jdaviz/configs/imviz/plugins/footprints/footprints.py @@ -189,6 +189,13 @@ def vue_link_by_wcs(self, *args): # when pixel-linked is reintroduced. self.app._jdaviz_helper.plugins['Links Control'].link_type = 'WCS' + def _ensure_first_overlay(self): + if not len(self._overlays): + # create the first default overlay + self._change_overlay() + # update the marks + self._preset_args_changed() + def rename_overlay(self, old_lbl, new_lbl): """ Rename an existing overlay instance @@ -272,11 +279,7 @@ def _on_is_active_changed(self, *args): return if self.is_active: - if not len(self._overlays): - # create the first default overlay - self._change_overlay() - # update the marks - self._preset_args_changed() + self._ensure_first_overlay() for overlay, viewer_marks in self.marks.items(): for viewer_id, marks in viewer_marks.items(): @@ -403,6 +406,7 @@ def import_region(self, region): ---------- region : str or regions.Regions object """ + self._ensure_first_overlay() if isinstance(region, (regions.Region, regions.Regions)): self.preset.import_obj(region) elif isinstance(region, str): # TODO: support path objects?