Improve get_subsets performance for single subset case (#3363)

* Don't loop through all subsets in get_subset if one is requested, act on lowercase labels * Don't mess with lowercasing labels * get rid of debugging prints * Add changelog * Move name check to start, sort printed list in error * Update jdaviz/app.py Co-authored-by: P. L. Lim <[email protected]> * Update jdaviz/app.py Co-authored-by: P. L. Lim <[email protected]> --------- Co-authored-by: P. L. Lim <[email protected]>
spacetelescope · Dec 17, 2024 · 297f8db · 297f8db
1 parent ef6db40
commit 297f8db
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 10 deletions.
diff --git a/CHANGES.rst b/CHANGES.rst
@@ -4,19 +4,19 @@
 New Features
 ------------
 
-* New design for viewer legend and data-menu. [#3220, #3254, #3263, #3264, #3271, #3272, #3274, #3289, #3310]
+- New design for viewer legend and data-menu. [#3220, #3254, #3263, #3264, #3271, #3272, #3274, #3289, #3310]
 
-* Improve performance while importing multiple regions. [#3321]
+- Improve performance while importing multiple regions. [#3321]
 
 Cubeviz
 ^^^^^^^
 
 Imviz
 ^^^^^
 
-* Orientation plugin API now exposes create_north_up_east_left and create_north_up_east_right methods. [#3308]
+- Orientation plugin API now exposes create_north_up_east_left and create_north_up_east_right methods. [#3308]
 
-* Add Roman WFI and CGI footprints to the Footprints plugin. [#3322]
+- Add Roman WFI and CGI footprints to the Footprints plugin. [#3322]
 
 - Catalog Search plugin now exposes a maximum sources limit for all catalogs and resolves an edge case
   when loading a catalog from a file that only contains one source. [#3337]
@@ -86,9 +86,12 @@ Specviz2d
 
 Other Changes and Additions
 ---------------------------
+
 - Added a short description of each plugin in the side menu, visible before the plugin is opened. Removes redundant descriptions above link
   out to documentation when plugin is opened. Enable search on plugin description in addition to title. [#3268]
 
+- Improved performance of ``app.get_subsets`` for the single-subset case. [#3363]
+
 4.0.1 (2024-12-16)
 ==================
 

diff --git a/jdaviz/app.py b/jdaviz/app.py
@@ -955,6 +955,16 @@ def get_subsets(self, subset_name=None, spectral_only=False,
 
         all_subsets = {}
 
+        # If we only want one subset, no need to loop through them all
+        if subset_name not in (None, ""):
+            if isinstance(subset_name, str):
+                subsets = [subset for subset in subsets if subset.label == subset_name]
+                if subsets == []:
+                    all_labels = sorted(sg.label for sg in dc.subset_groups)
+                    raise ValueError(f"{subset_name} not in {all_labels}")
+            else:
+                raise ValueError("subset_name must be a string.")
+
         for subset in subsets:
 
             label = subset.label
@@ -1020,13 +1030,8 @@ def get_subsets(self, subset_name=None, spectral_only=False,
                 else:
                     all_subsets[label] = subset_region
 
-        # can this be done at the top to avoid traversing all subsets if only
-        # one is requested?
-        all_subset_names = [subset.label for subset in dc.subset_groups]
-        if subset_name and subset_name in all_subset_names:
+        if subset_name:
             return all_subsets[subset_name]
-        elif subset_name:
-            raise ValueError(f"{subset_name} not in {all_subset_names}")
         else:
             return all_subsets