From 6325ec45bc7b198667f77209abde39f461baf540 Mon Sep 17 00:00:00 2001 From: "P. L. Lim" <2090236+pllim@users.noreply.github.com> Date: Mon, 15 Jul 2024 18:15:44 -0400 Subject: [PATCH 1/6] Markers: Save to CSV --- CHANGES.rst | 2 ++ .../default/plugins/markers/markers.py | 28 ++++++++++--------- .../markers/tests/test_markers_plugin.py | 19 +++++++++++++ .../imviz/plugins/coords_info/coords_info.py | 4 +++ jdaviz/core/template_mixin.py | 2 +- 5 files changed, 41 insertions(+), 14 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index fb73de5a42..3b06b9f769 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -86,6 +86,8 @@ Specviz2d Bug Fixes --------- +- Markers table can now export to CSV but its columns had to be changed to accomodate this fix. [#3089] + Cubeviz ^^^^^^^ diff --git a/jdaviz/configs/default/plugins/markers/markers.py b/jdaviz/configs/default/plugins/markers/markers.py index f3b1520e38..bf29ff9711 100644 --- a/jdaviz/configs/default/plugins/markers/markers.py +++ b/jdaviz/configs/default/plugins/markers/markers.py @@ -33,9 +33,11 @@ class Markers(PluginTemplateMixin, ViewerSelectMixin, TableMixin): _default_table_values = {'spectral_axis': np.nan, 'spectral_axis:unit': '', 'slice': np.nan, - 'pixel': (np.nan, np.nan), + 'pixel_x': np.nan, + 'pixel_y': np.nan, 'pixel:unreliable': None, - 'world': (np.nan, np.nan), + 'world_ra': np.nan, + 'world_dec': np.nan, 'world:unreliable': None, 'value': np.nan, 'value:unit': '', @@ -50,12 +52,12 @@ def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) if self.config == 'cubeviz': headers = ['spectral_axis', 'spectral_axis:unit', - 'slice', 'pixel', - 'world', 'value', 'value:unit', 'viewer'] + 'slice', 'pixel_x', 'pixel_y', + 'world_ra', 'world_dec', 'value', 'value:unit', 'viewer'] elif self.config == 'imviz': - headers = ['pixel', 'pixel:unreliable', - 'world', 'world:unreliable', + headers = ['pixel_x', 'pixel_y', 'pixel:unreliable', + 'world_ra', 'world_dec', 'world:unreliable', 'value', 'value:unit', 'value:unreliable', 'viewer'] @@ -65,10 +67,10 @@ def __init__(self, *args, **kwargs): elif self.config == 'specviz2d': # TODO: add "index" if/when specviz2d supports plotting spectral_axis headers = ['spectral_axis', 'spectral_axis:unit', - 'pixel', 'value', 'value:unit', 'viewer'] + 'pixel_x', 'pixel_y', 'value', 'value:unit', 'viewer'] elif self.config == 'mosviz': headers = ['spectral_axis', 'spectral_axis:unit', - 'pixel', 'world', 'index', 'value', 'value:unit', + 'pixel_x', 'pixel_y', 'world_ra', 'world_dec', 'index', 'value', 'value:unit', 'viewer'] else: # allow downstream configs to override headers @@ -107,7 +109,7 @@ def _on_viewer_added(self, msg): def _recompute_mark_positions(self, viewer): if self.table is None or self.table._qtable is None: return - if 'world' not in self.table.headers_avail: + if 'world_ra' not in self.table.headers_avail: return viewer_id = viewer.reference if viewer.reference is not None else viewer.reference_id @@ -124,8 +126,8 @@ def _recompute_mark_positions(self, viewer): viewer_mark.x, viewer_mark.y = [], [] return - orig_world_x = np.asarray(self.table._qtable['world'][:, 0][in_viewer]) - orig_world_y = np.asarray(self.table._qtable['world'][:, 1][in_viewer]) + orig_world_x = np.asarray(self.table._qtable['world_ra'][in_viewer]) + orig_world_y = np.asarray(self.table._qtable['world_dec'][in_viewer]) if self.app._link_type.lower() == 'wcs': # convert from the sky coordinates in the table to pixels via the WCS of the current @@ -162,8 +164,8 @@ def _recompute_mark_positions(self, viewer): # TODO: should we rescale these since pixel coordinates when linked by WCS are always # on the range 0-1 because of the orientation layer? Or hide the pixel option in the # cycler when WCS-linked? - pixel_x = np.asarray(self.table._qtable['pixel'][:, 0]) - pixel_y = np.asarray(self.table._qtable['pixel'][:, 1]) + pixel_x = np.asarray(self.table._qtable['pixel_x']) + pixel_y = np.asarray(self.table._qtable['pixel_y']) new_x = np.append(new_x, pixel_x[pixel_only_inds]) new_y = np.append(new_y, pixel_y[pixel_only_inds]) diff --git a/jdaviz/configs/default/plugins/markers/tests/test_markers_plugin.py b/jdaviz/configs/default/plugins/markers/tests/test_markers_plugin.py index f70a6b6463..b6adbeeddc 100644 --- a/jdaviz/configs/default/plugins/markers/tests/test_markers_plugin.py +++ b/jdaviz/configs/default/plugins/markers/tests/test_markers_plugin.py @@ -55,8 +55,12 @@ def test_markers_cubeviz(tmp_path, cubeviz_helper, spectrum1d_cube): 'spectral_axis:unit': 'm', 'world': (204.99988776727642, 27.000099999955538), + 'world_ra': 204.99988776727642, + 'world_dec': 27.000099999955538, 'world:unreliable': False, 'pixel': (0, 0), + 'pixel_x': 0, + 'pixel_y': 0, 'pixel:unreliable': False, 'value': 8.0, 'value:unit': 'Jy', @@ -139,6 +143,13 @@ def test_markers_cubeviz(tmp_path, cubeviz_helper, spectrum1d_cube): exp.export() assert os.path.isfile(filename) + # Also exports to CSV + filename_2 = str(tmp_path / "cubeviz_export.csv") + exp.plugin_table_format.selected = 'csv' + exp.filename.value = filename_2 + exp.export() + assert os.path.isfile(filename_2) + # clearing table clears markers mp.clear_table() assert mp.export_table() is None @@ -171,6 +182,8 @@ def test_markers_layer_cycle(self): 'axes_y:unit': 'pix', 'data_label': 'no_wcs[SCI,1]', 'pixel': (5.0, 5.0), + 'pixel_x': 5.0, + 'pixel_y': 5.0, 'pixel:unreliable': False, 'value': 55.0, 'value:unit': '', @@ -195,6 +208,8 @@ def test_markers_layer_cycle(self): 'axes_y:unit': 'pix', 'data_label': '', 'pixel': (5.0, 5.0), + 'pixel_x': 5.0, + 'pixel_y': 5.0, 'pixel:unreliable': False} mp._obj._on_viewer_key_event(self.viewer, {'event': 'keydown', @@ -217,8 +232,12 @@ def test_markers_layer_cycle(self): 'data_label': 'has_wcs[SCI,1]', 'world': (337.5187947653852, -20.831944164705973), + 'world_ra': 337.5187947653852, + 'world_dec': -20.831944164705973, 'world:unreliable': False, 'pixel': (5.0, 5.0), + 'pixel_x': 5.0, + 'pixel_y': 5.0, 'pixel:unreliable': False, 'value': 55.0, 'value:unit': '', diff --git a/jdaviz/configs/imviz/plugins/coords_info/coords_info.py b/jdaviz/configs/imviz/plugins/coords_info/coords_info.py index 87674a5253..00d54f4043 100644 --- a/jdaviz/configs/imviz/plugins/coords_info/coords_info.py +++ b/jdaviz/configs/imviz/plugins/coords_info/coords_info.py @@ -393,6 +393,8 @@ def _image_viewer_update(self, viewer, x, y): self.row3_unreliable = unreliable_world # TODO: use sky directly, but need to figure out how to have a compatible "blank" entry self._dict['world'] = (sky.ra.value, sky.dec.value) + self._dict['world_ra'] = self._dict['world'][0] + self._dict['world_dec'] = self._dict['world'][1] self._dict['world:unreliable'] = unreliable_world elif isinstance(viewer, MosvizProfile2DView) and hasattr(getattr(image, 'coords', None), 'pixel_to_world'): @@ -424,6 +426,8 @@ def _image_viewer_update(self, viewer, x, y): self.row1a_text = (fmt.format(x, y)) self.row1_unreliable = unreliable_pixel self._dict['pixel'] = (float(x), float(y)) + self._dict['pixel_x'] = self._dict['pixel'][0] + self._dict['pixel_y'] = self._dict['pixel'][1] self._dict['pixel:unreliable'] = unreliable_pixel # Extract data values at this position. diff --git a/jdaviz/core/template_mixin.py b/jdaviz/core/template_mixin.py index 07dae242d8..6d6c385a1b 100644 --- a/jdaviz/core/template_mixin.py +++ b/jdaviz/core/template_mixin.py @@ -4524,7 +4524,7 @@ def float_precision(column, item): # stored in astropy table as a float so we can also store nans, # but should display in the UI without any decimals return f"{item:.0f}" - elif column in ('pixel', ): + elif column in ('pixel', 'pixel_x', 'pixel_y'): return f"{item:0.3f}" elif column in ('xcenter', 'ycenter'): return f"{item:0.1f}" From 849759a84316a8c2a4cfa464bb16c74fb93a3447 Mon Sep 17 00:00:00 2001 From: "P. L. Lim" <2090236+pllim@users.noreply.github.com> Date: Mon, 15 Jul 2024 18:42:24 -0400 Subject: [PATCH 2/6] PEP 8 --- jdaviz/configs/default/plugins/markers/markers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/jdaviz/configs/default/plugins/markers/markers.py b/jdaviz/configs/default/plugins/markers/markers.py index bf29ff9711..d55dc45a62 100644 --- a/jdaviz/configs/default/plugins/markers/markers.py +++ b/jdaviz/configs/default/plugins/markers/markers.py @@ -70,8 +70,8 @@ def __init__(self, *args, **kwargs): 'pixel_x', 'pixel_y', 'value', 'value:unit', 'viewer'] elif self.config == 'mosviz': headers = ['spectral_axis', 'spectral_axis:unit', - 'pixel_x', 'pixel_y', 'world_ra', 'world_dec', 'index', 'value', 'value:unit', - 'viewer'] + 'pixel_x', 'pixel_y', 'world_ra', 'world_dec', 'index', + 'value', 'value:unit', 'viewer'] else: # allow downstream configs to override headers headers = kwargs.get('headers', []) From f6d95677aded1865f76fb4410a9a189ab079da6d Mon Sep 17 00:00:00 2001 From: "P. L. Lim" <2090236+pllim@users.noreply.github.com> Date: Mon, 15 Jul 2024 20:00:58 -0400 Subject: [PATCH 3/6] Mark block as uncovered for coverage --- jdaviz/configs/default/plugins/markers/markers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jdaviz/configs/default/plugins/markers/markers.py b/jdaviz/configs/default/plugins/markers/markers.py index d55dc45a62..7789030270 100644 --- a/jdaviz/configs/default/plugins/markers/markers.py +++ b/jdaviz/configs/default/plugins/markers/markers.py @@ -223,7 +223,7 @@ def _on_viewer_key_event(self, viewer, data): try: self.table.add_item({k: v for k, v in row_info.items() if k in self.table.headers_avail}) - except ValueError as err: + except ValueError as err: # pragma: no cover raise ValueError(f'failed to add {row_info} to table: {repr(err)}') x, y = row_info['axes_x'], row_info['axes_y'] From 7c59e9782ccef297570a33783b0cf6d800e0a470 Mon Sep 17 00:00:00 2001 From: "P. L. Lim" <2090236+pllim@users.noreply.github.com> Date: Tue, 16 Jul 2024 09:48:10 -0400 Subject: [PATCH 4/6] Remove unused entries from _dict --- .../plugins/markers/tests/test_markers_plugin.py | 8 -------- .../configs/imviz/plugins/coords_info/coords_info.py | 11 ++++------- 2 files changed, 4 insertions(+), 15 deletions(-) diff --git a/jdaviz/configs/default/plugins/markers/tests/test_markers_plugin.py b/jdaviz/configs/default/plugins/markers/tests/test_markers_plugin.py index b6adbeeddc..8d4e0af44e 100644 --- a/jdaviz/configs/default/plugins/markers/tests/test_markers_plugin.py +++ b/jdaviz/configs/default/plugins/markers/tests/test_markers_plugin.py @@ -53,12 +53,9 @@ def test_markers_cubeviz(tmp_path, cubeviz_helper, spectrum1d_cube): 'slice': 1.0, 'spectral_axis': 4.62360027696835e-07, 'spectral_axis:unit': 'm', - 'world': (204.99988776727642, - 27.000099999955538), 'world_ra': 204.99988776727642, 'world_dec': 27.000099999955538, 'world:unreliable': False, - 'pixel': (0, 0), 'pixel_x': 0, 'pixel_y': 0, 'pixel:unreliable': False, @@ -181,7 +178,6 @@ def test_markers_layer_cycle(self): 'axes_y': 5, 'axes_y:unit': 'pix', 'data_label': 'no_wcs[SCI,1]', - 'pixel': (5.0, 5.0), 'pixel_x': 5.0, 'pixel_y': 5.0, 'pixel:unreliable': False, @@ -207,7 +203,6 @@ def test_markers_layer_cycle(self): 'axes_y': 5, 'axes_y:unit': 'pix', 'data_label': '', - 'pixel': (5.0, 5.0), 'pixel_x': 5.0, 'pixel_y': 5.0, 'pixel:unreliable': False} @@ -230,12 +225,9 @@ def test_markers_layer_cycle(self): 'axes_y': 5, 'axes_y:unit': 'pix', 'data_label': 'has_wcs[SCI,1]', - 'world': (337.5187947653852, - -20.831944164705973), 'world_ra': 337.5187947653852, 'world_dec': -20.831944164705973, 'world:unreliable': False, - 'pixel': (5.0, 5.0), 'pixel_x': 5.0, 'pixel_y': 5.0, 'pixel:unreliable': False, diff --git a/jdaviz/configs/imviz/plugins/coords_info/coords_info.py b/jdaviz/configs/imviz/plugins/coords_info/coords_info.py index 00d54f4043..169e0c058c 100644 --- a/jdaviz/configs/imviz/plugins/coords_info/coords_info.py +++ b/jdaviz/configs/imviz/plugins/coords_info/coords_info.py @@ -391,10 +391,8 @@ def _image_viewer_update(self, viewer, x, y): self.row3_title = '' self.row3_text = f'{world_ra_deg} {world_dec_deg} (deg)' self.row3_unreliable = unreliable_world - # TODO: use sky directly, but need to figure out how to have a compatible "blank" entry - self._dict['world'] = (sky.ra.value, sky.dec.value) - self._dict['world_ra'] = self._dict['world'][0] - self._dict['world_dec'] = self._dict['world'][1] + self._dict['world_ra'] = sky.ra.value + self._dict['world_dec'] = sky.dec.value self._dict['world:unreliable'] = unreliable_world elif isinstance(viewer, MosvizProfile2DView) and hasattr(getattr(image, 'coords', None), 'pixel_to_world'): @@ -425,9 +423,8 @@ def _image_viewer_update(self, viewer, x, y): self.row1a_title = 'Pixel' self.row1a_text = (fmt.format(x, y)) self.row1_unreliable = unreliable_pixel - self._dict['pixel'] = (float(x), float(y)) - self._dict['pixel_x'] = self._dict['pixel'][0] - self._dict['pixel_y'] = self._dict['pixel'][1] + self._dict['pixel_x'] = float(x) + self._dict['pixel_y'] = float(y) self._dict['pixel:unreliable'] = unreliable_pixel # Extract data values at this position. From cc7caf915cb0fe8710030fcb4a6f5fbd14fc8857 Mon Sep 17 00:00:00 2001 From: "P. L. Lim" <2090236+pllim@users.noreply.github.com> Date: Thu, 18 Jul 2024 16:27:42 -0400 Subject: [PATCH 5/6] Update CHANGES.rst [ci skip] [rtd skip] Co-authored-by: Kyle Conroy --- CHANGES.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index 3b06b9f769..c258ece857 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -86,7 +86,7 @@ Specviz2d Bug Fixes --------- -- Markers table can now export to CSV but its columns had to be changed to accomodate this fix. [#3089] +- Markers table can now export to CSV but its columns had to be changed to accomodate this fix: world and pixel (previously containing SkyCoordinate tuples) are now each two separate columns for world_ra/world_dec and pixel_x/pixel_y, respectively. [#3089] Cubeviz ^^^^^^^ From 65d76a80d40e93531c5a81b8432aa780ad0a9e41 Mon Sep 17 00:00:00 2001 From: "P. L. Lim" <2090236+pllim@users.noreply.github.com> Date: Thu, 18 Jul 2024 16:28:45 -0400 Subject: [PATCH 6/6] Update CHANGES.rst --- CHANGES.rst | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index c258ece857..56cd1c90c6 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -86,7 +86,9 @@ Specviz2d Bug Fixes --------- -- Markers table can now export to CSV but its columns had to be changed to accomodate this fix: world and pixel (previously containing SkyCoordinate tuples) are now each two separate columns for world_ra/world_dec and pixel_x/pixel_y, respectively. [#3089] +- Markers table can now export to CSV but its columns had to be changed to accomodate this fix: + world and pixel (previously containing SkyCoord and pixel location tuples, respectively) are now + each two separate columns for world_ra/world_dec and pixel_x/pixel_y, respectively. [#3089] Cubeviz ^^^^^^^