From 32f6618841edd4dcdaee3b2c522657c9aab3f2d1 Mon Sep 17 00:00:00 2001
From: "Pey Lian Lim (Github)" <2090236+pllim@users.noreply.github.com>
Date: Mon, 24 Oct 2022 22:21:22 -0400
Subject: [PATCH] Catalog Search: Hide markers only when clearing by default

---
 CHANGES.rst                                   |  3 ++
 jdaviz/configs/imviz/helper.py                |  4 +++
 .../imviz/plugins/catalogs/catalogs.py        | 36 +++++++++++++------
 jdaviz/configs/imviz/tests/test_catalogs.py   |  8 +++++
 4 files changed, 40 insertions(+), 11 deletions(-)

diff --git a/CHANGES.rst b/CHANGES.rst
index e668143949..5ab9191739 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -44,6 +44,9 @@ Cubeviz
 Imviz
 ^^^^^
 
+- Clearing markers in Catalog Search will only hide them, which improves
+  "Clear" performance. [#1774]
+
 Mosviz
 ^^^^^^
 
diff --git a/jdaviz/configs/imviz/helper.py b/jdaviz/configs/imviz/helper.py
index f70c3f4981..7cc22ad751 100644
--- a/jdaviz/configs/imviz/helper.py
+++ b/jdaviz/configs/imviz/helper.py
@@ -421,6 +421,10 @@ def layer_is_image_data(layer):
     return isinstance(layer, BaseData) and layer.ndim == 2
 
 
+def layer_is_table_data(layer):
+    return isinstance(layer, BaseData) and layer.ndim == 1
+
+
 def get_top_layer_index(viewer):
     """Get index of the top visible image layer in Imviz.
     This is because when blinked, first layer might not be top visible layer.
diff --git a/jdaviz/configs/imviz/plugins/catalogs/catalogs.py b/jdaviz/configs/imviz/plugins/catalogs/catalogs.py
index f37c821d2d..ce7766f07a 100644
--- a/jdaviz/configs/imviz/plugins/catalogs/catalogs.py
+++ b/jdaviz/configs/imviz/plugins/catalogs/catalogs.py
@@ -1,12 +1,10 @@
-import numpy as np
-import numpy.ma as ma
 import os
 
-from traitlets import List, Unicode, Bool, Int, observe
-
+import numpy as np
+import numpy.ma as ma
 from astropy.table import QTable
 from astropy.coordinates import SkyCoord
-from astroquery.sdss import SDSS
+from traitlets import List, Unicode, Bool, Int, observe
 
 from jdaviz.configs.default.plugins.data_tools.file_chooser import FileChooser
 from jdaviz.core.registries import tray_registry
@@ -49,6 +47,7 @@ def __init__(self, *args, **kwargs):
         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'
@@ -127,6 +126,8 @@ def search(self):
 
         # conducts search based on SDSS
         if self.catalog_selected == "SDSS":
+            from astroquery.sdss import SDSS
+
             # queries the region (based on the provided center point and radius)
             # finds all the sources in that region
             query_region_result = SDSS.query_region(skycoord_center, radius=zoom_radius,
@@ -186,7 +187,7 @@ def search(self):
 
         # markers are added to the viewer based on the table
         viewer.marker = {'color': 'red', 'alpha': 0.8, 'markersize': 5, 'fill': False}
-        viewer.add_markers(table=catalog_results, use_skycoord=True, marker_name='catalog_results')
+        viewer.add_markers(table=catalog_results, use_skycoord=True, marker_name=self._marker_name)
 
         return skycoord_table
 
@@ -194,16 +195,29 @@ def vue_do_search(self, *args, **kwargs):
         # calls self.search() which handles all of the searching logic
         self.search()
 
-    def clear(self):
-        if self.results_available:
+    def clear(self, hide_only=True):
+        # gets the current viewer
+        viewer = self.viewer.selected_obj
+
+        if not hide_only and self._marker_name in self.app.data_collection.labels:
             # resetting values
             self.results_available = False
             self.number_of_results = 0
-            # gets the current viewer
-            viewer = self.viewer.selected_obj
 
             # all markers are removed from the viewer
-            viewer.reset_markers()
+            viewer.remove_markers(marker_name=self._marker_name)
+
+        elif self.results_available:
+            from jdaviz.configs.imviz.helper import layer_is_table_data
+
+            # resetting values
+            self.results_available = False
+            self.number_of_results = 0
+
+            # markers still there, just hidden
+            for lyr in viewer.layers:
+                if layer_is_table_data(lyr.layer) and lyr.layer.label == self._marker_name:
+                    lyr.visible = False
 
     def vue_do_clear(self, *args, **kwargs):
         self.clear()
diff --git a/jdaviz/configs/imviz/tests/test_catalogs.py b/jdaviz/configs/imviz/tests/test_catalogs.py
index 8bdbe9447e..61b2bdcb00 100644
--- a/jdaviz/configs/imviz/tests/test_catalogs.py
+++ b/jdaviz/configs/imviz/tests/test_catalogs.py
@@ -167,3 +167,11 @@ def test_offline_ecsv_catalog(imviz_helper, image_2d_wcs, tmp_path):
     assert len(out_tbl) == n_entries
     assert catalogs_plugin._obj.number_of_results == n_entries
     assert len(imviz_helper.app.data_collection) == 2  # image + markers
+
+    catalogs_plugin._obj.clear()
+    assert not catalogs_plugin._obj.results_available
+    assert len(imviz_helper.app.data_collection) == 2  # markers still there, just hidden
+
+    catalogs_plugin._obj.clear(hide_only=False)
+    assert not catalogs_plugin._obj.results_available
+    assert len(imviz_helper.app.data_collection) == 1  # markers gone for good